[Findit] Use heuristic analysis result for test try jobs(Findit side).

[Findit] Use heuristic analysis result for test try jobs(Findit side).

Process the result of heuristic analysis and get a list of suspected revisions and pass those revisions to try job.

BUG=596969
Committed: https://chromium.googlesource.com/infra/infra/+/9b6b8073b0d6f4c6ea26bdbdae406f5918b75c9e

[chanli, 2016-04-27 23:15:23]

Description was changed from

==========
[Findit] Use heuristic analysis result for test try jobs(Findit side).

BUG=596969
==========

to

==========
[Findit] Use heuristic analysis result for test try jobs(Findit side).

Will upload another CL for recipe side changes.

BUG=596969
==========

[chanli, 2016-04-27 23:15:38]

chanli@chromium.org changed reviewers:
+ lijeffrey@chromium.org, stgao@chromium.org

[chanli, 2016-04-27 23:15:39]

ptal

[lijeffrey, 2016-04-27 23:34:08]

lgtm with nits

https://codereview.chromium.org/1924823003/diff/1/appengine/findit/waterfall/...
File appengine/findit/waterfall/test/try_job_util_test.py (right):

https://codereview.chromium.org/1924823003/diff/1/appengine/findit/waterfall/...
appengine/findit/waterfall/test/try_job_util_test.py:442: heuristic_result))
nit: does this fit on one line?

https://codereview.chromium.org/1924823003/diff/1/appengine/findit/waterfall/...
File appengine/findit/waterfall/try_job_util.py (right):

https://codereview.chromium.org/1924823003/diff/1/appengine/findit/waterfall/...
appengine/findit/waterfall/try_job_util.py:173:
_GetSuspectsFromHeuristicResult(heuristic_result))
nit: does this fit on one line?

[chanli, 2016-04-28 17:36:04]

https://codereview.chromium.org/1924823003/diff/1/appengine/findit/waterfall/...
File appengine/findit/waterfall/test/try_job_util_test.py (right):

https://codereview.chromium.org/1924823003/diff/1/appengine/findit/waterfall/...
appengine/findit/waterfall/test/try_job_util_test.py:442: heuristic_result))
On 2016/04/27 23:34:08, lijeffrey wrote:
> nit: does this fit on one line?

Done.

https://codereview.chromium.org/1924823003/diff/1/appengine/findit/waterfall/...
File appengine/findit/waterfall/try_job_util.py (right):

https://codereview.chromium.org/1924823003/diff/1/appengine/findit/waterfall/...
appengine/findit/waterfall/try_job_util.py:173:
_GetSuspectsFromHeuristicResult(heuristic_result))
On 2016/04/27 23:34:08, lijeffrey wrote:
> nit: does this fit on one line?

Done.

[stgao, 2016-04-28 17:37:50]

https://codereview.chromium.org/1924823003/diff/1/appengine/findit/waterfall/...
File appengine/findit/waterfall/try_job_util.py (right):

https://codereview.chromium.org/1924823003/diff/1/appengine/findit/waterfall/...
appengine/findit/waterfall/try_job_util.py:140: suspected_revisions =
[c['revision'] for c in failure['suspected_cls']]
I don't think this overwrite will be correct for tests.

For different step failures, there might be different culprit CLs, right?

https://codereview.chromium.org/1924823003/diff/20001/appengine/findit/waterf...
File appengine/findit/waterfall/test/try_job_util_test.py (right):

https://codereview.chromium.org/1924823003/diff/20001/appengine/findit/waterf...
appengine/findit/waterfall/test/try_job_util_test.py:434: ],
May consider adding a separate testcase for test failrues.

[chanli, 2016-04-28 22:13:14]

https://codereview.chromium.org/1924823003/diff/1/appengine/findit/waterfall/...
File appengine/findit/waterfall/try_job_util.py (right):

https://codereview.chromium.org/1924823003/diff/1/appengine/findit/waterfall/...
appengine/findit/waterfall/try_job_util.py:140: suspected_revisions =
[c['revision'] for c in failure['suspected_cls']]
On 2016/04/28 17:37:49, stgao wrote:
> I don't think this overwrite will be correct for tests.
> 
> For different step failures, there might be different culprit CLs, right?

Right... Good catch!

https://codereview.chromium.org/1924823003/diff/20001/appengine/findit/waterf...
File appengine/findit/waterfall/test/try_job_util_test.py (right):

https://codereview.chromium.org/1924823003/diff/20001/appengine/findit/waterf...
appengine/findit/waterfall/test/try_job_util_test.py:434: ],
On 2016/04/28 17:37:49, stgao wrote:
> May consider adding a separate testcase for test failrues.

Done.

[stgao, 2016-04-28 22:25:49]

Before the change on recipe is committed, this CL should not be committed.
Otherwise, it will cause breakage on recipe side if a new version of Findit is
deployed.

https://codereview.chromium.org/1924823003/diff/40001/appengine/findit/waterf...
File appengine/findit/waterfall/try_job_util.py (right):

https://codereview.chromium.org/1924823003/diff/40001/appengine/findit/waterf...
appengine/findit/waterfall/try_job_util.py:141:
suspected_revisions.add(cl['revision'])
Have you considered organizing suspected CLs by step-test for test failures? Not
sure that would be the best for test failures.

Thus the change to the recipe will help on the decision here.

[chanli, 2016-04-28 22:43:19]

https://codereview.chromium.org/1924823003/diff/40001/appengine/findit/waterf...
File appengine/findit/waterfall/try_job_util.py (right):

https://codereview.chromium.org/1924823003/diff/40001/appengine/findit/waterf...
appengine/findit/waterfall/try_job_util.py:141:
suspected_revisions.add(cl['revision'])
On 2016/04/28 22:25:49, stgao wrote:
> Have you considered organizing suspected CLs by step-test for test failures?
Not
> sure that would be the best for test failures.
> 
> Thus the change to the recipe will help on the decision here.

I have thought about it. But I think the accuracy rate of heuristic analysis
result for test is not as good as compile, we should test all reliable tests
against each revision.

[stgao, 2016-05-04 21:40:18]

https://codereview.chromium.org/1924823003/diff/60001/appengine/findit/waterf...
File appengine/findit/waterfall/identify_try_job_culprit_pipeline.py (right):

https://codereview.chromium.org/1924823003/diff/60001/appengine/findit/waterf...
appengine/findit/waterfall/identify_try_job_culprit_pipeline.py:179: if not
test_results.get(revision):
How about let the recipe compute the culprits it finds for each test?

In that case, the recipe could change the way how it tests without impacting the
logic here. WDYT?

[chanli, 2016-05-04 23:41:37]

https://codereview.chromium.org/1924823003/diff/60001/appengine/findit/waterf...
File appengine/findit/waterfall/identify_try_job_culprit_pipeline.py (right):

https://codereview.chromium.org/1924823003/diff/60001/appengine/findit/waterf...
appengine/findit/waterfall/identify_try_job_culprit_pipeline.py:179: if not
test_results.get(revision):
On 2016/05/04 21:40:18, stgao wrote:
> How about let the recipe compute the culprits it finds for each test?
> 
> In that case, the recipe could change the way how it tests without impacting
the
> logic here. WDYT?

Done.

[lijeffrey, 2016-05-06 18:45:40]

lgtm

[chanli, 2016-05-13 00:39:57]

The CQ bit was checked by chanli@chromium.org

[commit-bot: I haz the power, 2016-05-13 00:40:02]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1924823003/80001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1924823003/80001

[commit-bot: I haz the power, 2016-05-13 00:45:20]

Description was changed from

==========
[Findit] Use heuristic analysis result for test try jobs(Findit side).

Will upload another CL for recipe side changes.

BUG=596969
==========

to

==========
[Findit] Use heuristic analysis result for test try jobs(Findit side).

Will upload another CL for recipe side changes.

BUG=596969

Committed:
https://chromium.googlesource.com/infra/infra/+/9b6b8073b0d6f4c6ea26bdbdae406...
==========

[commit-bot: I haz the power, 2016-05-13 00:45:21]

Committed patchset #5 (id:80001) as
https://chromium.googlesource.com/infra/infra/+/9b6b8073b0d6f4c6ea26bdbdae406...

[chanli1, 2017-04-07 18:36:13]

Description was changed from

==========
[Findit] Use heuristic analysis result for test try jobs(Findit side).

Will upload another CL for recipe side changes.

BUG=596969

Committed:
https://chromium.googlesource.com/infra/infra/+/9b6b8073b0d6f4c6ea26bdbdae406...
==========

to

==========
[Findit] Use heuristic analysis result for test try jobs(Findit side).

Process the result of heuristic analysis and get a list of suspected revisions
and pass those revisions to try job.

BUG=596969

Committed:
https://chromium.googlesource.com/infra/infra/+/9b6b8073b0d6f4c6ea26bdbdae406...
==========