[Findit] Change test.py recipe to speed up test try jobs(Recipe side change).

[Findit] Change test.py recipe to speed up.

There are 2 main changes in this CL:
1. Use Findit heuristic analysis results to guide test try jobs.

Run tests on the revision before the suspected revision and the suspected revision first, if test failed on suspected revision and passed on the one before it, the culprit is confirmed to be the suspected one.


2. After a test finds its culprit, don't run this test in the rest of revisions.

BUG=608415, 596969
Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=300567

[chanli, 2016-05-03 22:15:57]

Description was changed from

==========
[Findit] Change test.py recipe to speed up.

There are 2 main changes in this CL:
Use Findit heuristic analysis results to guide test try jobs.
After a test finds its culprit, don't run this test in the rest of revisions.

BUG=608415, 596969
==========

to

==========
[Findit] Change test.py recipe to speed up.

There are 2 main changes in this CL:
Use Findit heuristic analysis results to guide test try jobs.
After a test finds its culprit, don't run this test in the rest of revisions.

BUG=608415, 596969
==========

[chanli, 2016-05-03 22:16:41]

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

[chanli, 2016-05-03 22:16:42]

ptal

[lijeffrey, 2016-05-03 23:25:44]

https://codereview.chromium.org/1946463003/diff/20001/scripts/slave/recipes/f...
File scripts/slave/recipes/findit/chromium/test.py (right):

https://codereview.chromium.org/1946463003/diff/20001/scripts/slave/recipes/f...
scripts/slave/recipes/findit/chromium/test.py:174: def
_get_shrinked_test_dict(original_test_dict, failed_tests_dict):
'shrinked' sounds a bit strange, how about rename this (and the corresponding
variables' to 'unique' intsead?

https://codereview.chromium.org/1946463003/diff/20001/scripts/slave/recipes/f...
scripts/slave/recipes/findit/chromium/test.py:266: if not no_culprit_tests:  #
All tests have found culprits
nit: comment should end with a .

https://codereview.chromium.org/1946463003/diff/20001/scripts/slave/recipes/f...
scripts/slave/recipes/findit/chromium/test.py:269: potential_green_rev =
sub_range[0]
nit: Add a comment that potential_green_rev is the revision right before the
suspected revision provided by the heuristic result.

https://codereview.chromium.org/1946463003/diff/20001/scripts/slave/recipes/f...
scripts/slave/recipes/findit/chromium/test.py:282: # Culprits for tests that
failed in potential green should be earlier.
nit: failed in the revision before suspected revisions should be earlier. etc...

https://codereview.chromium.org/1946463003/diff/20001/scripts/slave/recipes/f...
scripts/slave/recipes/findit/chromium/test.py:284: # only runs the tests that
failed in following revisions.
nit: Culprits for tests that failed in potential green should be earlier, thus
no need to run passed tests in the following revisions. Instead, only run the
tests that failed in the following revisions.

[chanli, 2016-05-04 17:14:40]

https://codereview.chromium.org/1946463003/diff/20001/scripts/slave/recipes/f...
File scripts/slave/recipes/findit/chromium/test.py (right):

https://codereview.chromium.org/1946463003/diff/20001/scripts/slave/recipes/f...
scripts/slave/recipes/findit/chromium/test.py:174: def
_get_shrinked_test_dict(original_test_dict, failed_tests_dict):
On 2016/05/03 23:25:44, lijeffrey wrote:
> 'shrinked' sounds a bit strange, how about rename this (and the corresponding
> variables' to 'unique' intsead?

It's actually not 'unique'... What I do here is just to take out tests that are
in both dicts from original_test_dict. But I agree I need to think of a better
name.

https://codereview.chromium.org/1946463003/diff/20001/scripts/slave/recipes/f...
scripts/slave/recipes/findit/chromium/test.py:266: if not no_culprit_tests:  #
All tests have found culprits
On 2016/05/03 23:25:44, lijeffrey wrote:
> nit: comment should end with a .

Done.

https://codereview.chromium.org/1946463003/diff/20001/scripts/slave/recipes/f...
scripts/slave/recipes/findit/chromium/test.py:269: potential_green_rev =
sub_range[0]
On 2016/05/03 23:25:44, lijeffrey wrote:
> nit: Add a comment that potential_green_rev is the revision right before the
> suspected revision provided by the heuristic result.

Done.

https://codereview.chromium.org/1946463003/diff/20001/scripts/slave/recipes/f...
scripts/slave/recipes/findit/chromium/test.py:282: # Culprits for tests that
failed in potential green should be earlier.
On 2016/05/03 23:25:44, lijeffrey wrote:
> nit: failed in the revision before suspected revisions should be earlier.
etc...

Modified the whole sentence.

https://codereview.chromium.org/1946463003/diff/20001/scripts/slave/recipes/f...
scripts/slave/recipes/findit/chromium/test.py:284: # only runs the tests that
failed in following revisions.
On 2016/05/03 23:25:44, lijeffrey wrote:
> nit: Culprits for tests that failed in potential green should be earlier, thus
> no need to run passed tests in the following revisions. Instead, only run the
> tests that failed in the following revisions.

Done.

[lijeffrey, 2016-05-04 19:15:19]

https://codereview.chromium.org/1946463003/diff/40001/scripts/slave/recipes/f...
File scripts/slave/recipes/findit/chromium/test.py (right):

https://codereview.chromium.org/1946463003/diff/40001/scripts/slave/recipes/f...
scripts/slave/recipes/findit/chromium/test.py:174: def
_get_subtracted_test_dict(original_test_dict, failed_tests_dict):
Naming this one's tricky, how about 'reduced' instead of 'subtracted'?

https://codereview.chromium.org/1946463003/diff/40001/scripts/slave/recipes/f...
scripts/slave/recipes/findit/chromium/test.py:285: # takes out passed tests and
only runs failed ones in following revisions.
nit: 'remove' instead of 'takes out'

[stgao, 2016-05-04 21:35:29]

https://codereview.chromium.org/1946463003/diff/40001/scripts/slave/recipes/f...
File scripts/slave/recipes/findit/chromium/test.py (right):

https://codereview.chromium.org/1946463003/diff/40001/scripts/slave/recipes/f...
scripts/slave/recipes/findit/chromium/test.py:244: sub_ranges[-1].insert(0,
remaining_revisions[index - 1])
Can we handle this separately by operating just on ``suspected_revision_index``?
That will improve the readability.

https://codereview.chromium.org/1946463003/diff/40001/scripts/slave/recipes/f...
scripts/slave/recipes/findit/chromium/test.py:263: no_culprit_tests = tests
naming nit: this name seems misleading -- these tests have no culprit, like
long-standing flaky tests.

https://codereview.chromium.org/1946463003/diff/40001/scripts/slave/recipes/f...
scripts/slave/recipes/findit/chromium/test.py:265: for sub_range in sub_ranges:
Can we have a comment on why sub-ranges with newer revisions are tested first?

https://codereview.chromium.org/1946463003/diff/40001/scripts/slave/recipes/f...
scripts/slave/recipes/findit/chromium/test.py:288: for revision in
following_revisions:
Maybe add a comment on the general idea how this works?

[chanli, 2016-05-04 22:52:43]

https://codereview.chromium.org/1946463003/diff/40001/scripts/slave/recipes/f...
File scripts/slave/recipes/findit/chromium/test.py (right):

https://codereview.chromium.org/1946463003/diff/40001/scripts/slave/recipes/f...
scripts/slave/recipes/findit/chromium/test.py:174: def
_get_subtracted_test_dict(original_test_dict, failed_tests_dict):
On 2016/05/04 19:15:19, lijeffrey wrote:
> Naming this one's tricky, how about 'reduced' instead of 'subtracted'?

Done.

https://codereview.chromium.org/1946463003/diff/40001/scripts/slave/recipes/f...
scripts/slave/recipes/findit/chromium/test.py:244: sub_ranges[-1].insert(0,
remaining_revisions[index - 1])
On 2016/05/04 21:35:28, stgao wrote:
> Can we handle this separately by operating just on
``suspected_revision_index``?
> That will improve the readability.

I don't rally understand what you're suggesting here. I changed this logic a
little, is readability better?

https://codereview.chromium.org/1946463003/diff/40001/scripts/slave/recipes/f...
scripts/slave/recipes/findit/chromium/test.py:263: no_culprit_tests = tests
On 2016/05/04 21:35:28, stgao wrote:
> naming nit: this name seems misleading -- these tests have no culprit, like
> long-standing flaky tests.

Done.

https://codereview.chromium.org/1946463003/diff/40001/scripts/slave/recipes/f...
scripts/slave/recipes/findit/chromium/test.py:265: for sub_range in sub_ranges:
On 2016/05/04 21:35:28, stgao wrote:
> Can we have a comment on why sub-ranges with newer revisions are tested first?

Done.

https://codereview.chromium.org/1946463003/diff/40001/scripts/slave/recipes/f...
scripts/slave/recipes/findit/chromium/test.py:285: # takes out passed tests and
only runs failed ones in following revisions.
On 2016/05/04 19:15:19, lijeffrey wrote:
> nit: 'remove' instead of 'takes out'

Done.

https://codereview.chromium.org/1946463003/diff/40001/scripts/slave/recipes/f...
scripts/slave/recipes/findit/chromium/test.py:288: for revision in
following_revisions:
On 2016/05/04 21:35:28, stgao wrote:
> Maybe add a comment on the general idea how this works?

Done.

[chanli, 2016-05-05 22:11:23]

Adopted Shuotao's change on compile recipe.

[lijeffrey, 2016-05-06 18:41:55]

lgtm with nits

https://codereview.chromium.org/1946463003/diff/100001/scripts/slave/recipes/...
File scripts/slave/recipes/findit/chromium/test.py (right):

https://codereview.chromium.org/1946463003/diff/100001/scripts/slave/recipes/...
scripts/slave/recipes/findit/chromium/test.py:134: failed_tests_dict =
defaultdict(list)
nit: move this closer to where it's being used

https://codereview.chromium.org/1946463003/diff/100001/scripts/slave/recipes/...
scripts/slave/recipes/findit/chromium/test.py:310: # Records down found
culprits.
nit: "down" is not necessary

[chanli, 2016-05-06 23:39:18]

https://codereview.chromium.org/1946463003/diff/100001/scripts/slave/recipes/...
File scripts/slave/recipes/findit/chromium/test.py (right):

https://codereview.chromium.org/1946463003/diff/100001/scripts/slave/recipes/...
scripts/slave/recipes/findit/chromium/test.py:134: failed_tests_dict =
defaultdict(list)
On 2016/05/06 18:41:55, lijeffrey wrote:
> nit: move this closer to where it's being used

Done.

https://codereview.chromium.org/1946463003/diff/100001/scripts/slave/recipes/...
scripts/slave/recipes/findit/chromium/test.py:310: # Records down found
culprits.
On 2016/05/06 18:41:55, lijeffrey wrote:
> nit: "down" is not necessary

Done.

[stgao, 2016-05-10 00:38:04]

lgtm

It might be good to test the recipe with a real test failure locally to ensure
it works as expected.

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

The CQ bit was checked by chanli@chromium.org

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

The patchset sent to the CQ was uploaded after l-g-t-m from
lijeffrey@chromium.org, stgao@chromium.org
Link to the patchset: https://codereview.chromium.org/1946463003/#ps160001
(title: "rebase")

[commit-bot: I haz the power, 2016-05-13 00:39:55]

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

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

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-05-13 00:41:06]

Try jobs failed on following builders:
  Build Presubmit on tryserver.infra (JOB_FAILED,
https://build.chromium.org/p/tryserver.infra/builders/Build%20Presubmit/build...)
  Build Try Recipe Test Trusty64 on tryserver.infra (JOB_FAILED,
https://build.chromium.org/p/tryserver.infra/builders/Build%20Try%20Recipe%20...)
  Build Try Recipe Test Win64 on tryserver.infra (JOB_FAILED,
https://build.chromium.org/p/tryserver.infra/builders/Build%20Try%20Recipe%20...)

[chanli, 2016-05-13 01:05:32]

The CQ bit was checked by chanli@chromium.org

[chanli, 2016-05-13 01:05:33]

The patchset sent to the CQ was uploaded after l-g-t-m from stgao@chromium.org,
lijeffrey@chromium.org
Link to the patchset: https://codereview.chromium.org/1946463003/#ps180001
(title: "rebase")

[commit-bot: I haz the power, 2016-05-13 01:05:43]

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

[commit-bot: I haz the power, 2016-05-13 01:12:53]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-05-13 01:12:54]

Try jobs failed on following builders:
  Build Presubmit on tryserver.infra (JOB_FAILED,
https://build.chromium.org/p/tryserver.infra/builders/Build%20Presubmit/build...)

[chanli, 2016-05-13 01:30:07]

The CQ bit was checked by chanli@chromium.org

[chanli, 2016-05-13 01:30:08]

The patchset sent to the CQ was uploaded after l-g-t-m from stgao@chromium.org,
lijeffrey@chromium.org
Link to the patchset: https://codereview.chromium.org/1946463003/#ps200001
(title: ".")

[commit-bot: I haz the power, 2016-05-13 01:30:12]

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

[commit-bot: I haz the power, 2016-05-13 01:34:12]

Description was changed from

==========
[Findit] Change test.py recipe to speed up.

There are 2 main changes in this CL:
Use Findit heuristic analysis results to guide test try jobs.
After a test finds its culprit, don't run this test in the rest of revisions.

BUG=608415, 596969
==========

to

==========
[Findit] Change test.py recipe to speed up.

There are 2 main changes in this CL:
Use Findit heuristic analysis results to guide test try jobs.
After a test finds its culprit, don't run this test in the rest of revisions.

BUG=608415, 596969

Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=300567
==========

[commit-bot: I haz the power, 2016-05-13 01:34:13]

Committed patchset #11 (id:200001) as
http://src.chromium.org/viewvc/chrome?view=rev&revision=300567

[chanli1, 2017-04-07 18:40:02]

Description was changed from

==========
[Findit] Change test.py recipe to speed up.

There are 2 main changes in this CL:
Use Findit heuristic analysis results to guide test try jobs.
After a test finds its culprit, don't run this test in the rest of revisions.

BUG=608415, 596969

Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=300567
==========

to

==========
[Findit] Change test.py recipe to speed up.

There are 2 main changes in this CL:
1. Use Findit heuristic analysis results to guide test try jobs.
Segments revisions_to_check by suspected_revisions.
Each sub_range will contain following elements:
a. Revision before a suspected_revision or None as a placeholder when no such
revision
b. Suspected_revision
c. Revisions between a suspected_revision and the revision before next
suspected_revision, or remaining revisions before all suspect_revisions.
For example, if revisions_to_check are [r0, r1, ..., r6] and suspected_revisions
are [r2, r5], sub_ranges will be:
[[None, r0], [r1, r2, r3], [r4, r5, r6]]

2. After a test finds its culprit, don't run this test in the rest of revisions.

BUG=608415, 596969

Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=300567
==========

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

Description was changed from

==========
[Findit] Change test.py recipe to speed up.

There are 2 main changes in this CL:
1. Use Findit heuristic analysis results to guide test try jobs.
Segments revisions_to_check by suspected_revisions.
Each sub_range will contain following elements:
a. Revision before a suspected_revision or None as a placeholder when no such
revision
b. Suspected_revision
c. Revisions between a suspected_revision and the revision before next
suspected_revision, or remaining revisions before all suspect_revisions.
For example, if revisions_to_check are [r0, r1, ..., r6] and suspected_revisions
are [r2, r5], sub_ranges will be:
[[None, r0], [r1, r2, r3], [r4, r5, r6]]

2. After a test finds its culprit, don't run this test in the rest of revisions.

BUG=608415, 596969

Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=300567
==========

to

==========
[Findit] Change test.py recipe to speed up.

There are 2 main changes in this CL:
1. Use Findit heuristic analysis results to guide test try jobs.
Segments revisions_to_check by suspected_revisions.
Each sub_range will contain following elements:
a. Revision before a suspected_revision or None as a placeholder when no such
revision
b. Suspected_revision
c. Revisions between a suspected_revision and the revision before next
suspected_revision, or remaining revisions before all suspect_revisions.
For example, if revisions_to_check are [r0, r1, ..., r6] and suspected_revisions
are [r2, r5], sub_ranges will be:
[[r4, r5, r6], [r1, r2, r3], [None, r0]]

So when test on revisions, we will test on 

2. After a test finds its culprit, don't run this test in the rest of revisions.

BUG=608415, 596969

Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=300567
==========

[chanli1, 2017-04-07 19:51:30]

Description was changed from

==========
[Findit] Change test.py recipe to speed up.

There are 2 main changes in this CL:
1. Use Findit heuristic analysis results to guide test try jobs.
Segments revisions_to_check by suspected_revisions.
Each sub_range will contain following elements:
a. Revision before a suspected_revision or None as a placeholder when no such
revision
b. Suspected_revision
c. Revisions between a suspected_revision and the revision before next
suspected_revision, or remaining revisions before all suspect_revisions.
For example, if revisions_to_check are [r0, r1, ..., r6] and suspected_revisions
are [r2, r5], sub_ranges will be:
[[r4, r5, r6], [r1, r2, r3], [None, r0]]

So when test on revisions, we will test on 

2. After a test finds its culprit, don't run this test in the rest of revisions.

BUG=608415, 596969

Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=300567
==========

to

==========
[Findit] Change test.py recipe to speed up.

There are 2 main changes in this CL:
1. Use Findit heuristic analysis results to guide test try jobs.

Run tests on the revision before the suspected revision and the suspected
revision first, if test failed on suspected revision and passed on the one
before it, the culprit is confirmed to be the suspected one.


2. After a test finds its culprit, don't run this test in the rest of revisions.

BUG=608415, 596969

Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=300567
==========