[Findit] don't included skipped or unknown tests in swarming tasks into failed tests.

[Findit] don't included skipped or unknown tests in swarming tasks into failed tests.

When record failed tests in detect_first_failure pipeline and check flakiness in swarming rerun, don't include skipped or unknown tests as failed.

BUG=616313
Committed: https://chromium.googlesource.com/infra/infra/+/5267507e65d388cc288f57a44f9fe042112a90d7

[chanli, 2016-06-01 18:40:06]

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

[chanli, 2016-06-01 18:40:07]

ptal

[lijeffrey, 2016-06-01 19:27:03]

https://codereview.chromium.org/2027333002/diff/1/appengine/findit/model/wf_s...
File appengine/findit/model/wf_swarming_task.py (right):

https://codereview.chromium.org/2027333002/diff/1/appengine/findit/model/wf_s...
appengine/findit/model/wf_swarming_task.py:49: 'reliable_tests': ['test3', ...]
Update the docstring too to put 'other_tests'.

side note: is 'other_tests' the same as 'unclassified_tests'?

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

https://codereview.chromium.org/2027333002/diff/1/appengine/findit/waterfall/...
appengine/findit/waterfall/detect_first_failure_pipeline.py:184: if
(any(test['status'] in ['SUCCESS', 'SKIPPED', 'UNKNOWN']
Maybe move this list to a constant called TEST_STATUSES at the top

[chanli, 2016-06-01 22:55:24]

ptal

[stgao, 2016-06-02 01:31:20]

https://codereview.chromium.org/2027333002/diff/20001/appengine/findit/waterf...
File appengine/findit/waterfall/detect_first_failure_pipeline.py (right):

https://codereview.chromium.org/2027333002/diff/20001/appengine/findit/waterf...
appengine/findit/waterfall/detect_first_failure_pipeline.py:186: # Ignore the
test if any of the attempts were 'SUCCESS'.
comment needs updating

https://codereview.chromium.org/2027333002/diff/20001/appengine/findit/waterf...
File appengine/findit/waterfall/process_swarming_task_result_pipeline.py
(right):

https://codereview.chromium.org/2027333002/diff/20001/appengine/findit/waterf...
appengine/findit/waterfall/process_swarming_task_result_pipeline.py:35: if
test_name not in targeted_tests:
If PRE_A failed on Waterfall and PRE_PRE_A failed on Swarming rerun (PRE_A is
skipped), how do we handle this case in the end?

[chanli, 2016-06-02 22:40:09]

https://codereview.chromium.org/2027333002/diff/1/appengine/findit/model/wf_s...
File appengine/findit/model/wf_swarming_task.py (right):

https://codereview.chromium.org/2027333002/diff/1/appengine/findit/model/wf_s...
appengine/findit/model/wf_swarming_task.py:49: 'reliable_tests': ['test3', ...]
On 2016/06/01 19:27:02, lijeffrey wrote:
> Update the docstring too to put 'other_tests'.
> 
> side note: is 'other_tests' the same as 'unclassified_tests'?

Done. Essentially a skipped test is a test with a failed pre_ test, so this test
will be skipped. And there is another status 'UnKNOWN'. I have changed the logic
here to consider 'skipped' tests are reliable failure.

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

https://codereview.chromium.org/2027333002/diff/1/appengine/findit/waterfall/...
appengine/findit/waterfall/detect_first_failure_pipeline.py:184: if
(any(test['status'] in ['SUCCESS', 'SKIPPED', 'UNKNOWN']
On 2016/06/01 19:27:03, lijeffrey wrote:
> Maybe move this list to a constant called TEST_STATUSES at the top

Done.

https://codereview.chromium.org/2027333002/diff/20001/appengine/findit/waterf...
File appengine/findit/waterfall/detect_first_failure_pipeline.py (right):

https://codereview.chromium.org/2027333002/diff/20001/appengine/findit/waterf...
appengine/findit/waterfall/detect_first_failure_pipeline.py:186: # Ignore the
test if any of the attempts were 'SUCCESS'.
On 2016/06/02 01:31:20, stgao wrote:
> comment needs updating

Done.

https://codereview.chromium.org/2027333002/diff/20001/appengine/findit/waterf...
File appengine/findit/waterfall/process_swarming_task_result_pipeline.py
(right):

https://codereview.chromium.org/2027333002/diff/20001/appengine/findit/waterf...
appengine/findit/waterfall/process_swarming_task_result_pipeline.py:35: if
test_name not in targeted_tests:
On 2016/06/02 01:31:20, stgao wrote:
> If PRE_A failed on Waterfall and PRE_PRE_A failed on Swarming rerun (PRE_A is
> skipped), how do we handle this case in the end?

In the end we should still say PRE_A failed.

[stgao, 2016-06-03 06:59:40]

https://codereview.chromium.org/2027333002/diff/20001/appengine/findit/waterf...
File appengine/findit/waterfall/process_swarming_task_result_pipeline.py
(right):

https://codereview.chromium.org/2027333002/diff/20001/appengine/findit/waterf...
appengine/findit/waterfall/process_swarming_task_result_pipeline.py:35: if
test_name not in targeted_tests:
On 2016/06/02 22:40:09, chanli wrote:
> On 2016/06/02 01:31:20, stgao wrote:
> > If PRE_A failed on Waterfall and PRE_PRE_A failed on Swarming rerun (PRE_A
is
> > skipped), how do we handle this case in the end?
> 
> In the end we should still say PRE_A failed.

OK. The code seems fixed for this now.

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

https://codereview.chromium.org/2027333002/diff/40001/appengine/findit/waterf...
appengine/findit/waterfall/detect_first_failure_pipeline.py:187: # Ignore the
test if any of the attempts didn't fail.
Please add explanation why skipped test should be treated as not failed.

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

https://codereview.chromium.org/2027333002/diff/40001/appengine/findit/waterf...
appengine/findit/waterfall/swarming_tasks_to_try_job_pipeline.py:32:
pre_position = test.find(_PRE_TEST_PREFIX)
What if the test is "a_PRE_b.PRE_test"?

[chanli, 2016-06-06 22:41:30]

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

https://codereview.chromium.org/2027333002/diff/40001/appengine/findit/waterf...
appengine/findit/waterfall/detect_first_failure_pipeline.py:187: # Ignore the
test if any of the attempts didn't fail.
On 2016/06/03 06:59:40, stgao wrote:
> Please add explanation why skipped test should be treated as not failed.

Done.

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

https://codereview.chromium.org/2027333002/diff/40001/appengine/findit/waterf...
appengine/findit/waterfall/swarming_tasks_to_try_job_pipeline.py:32:
pre_position = test.find(_PRE_TEST_PREFIX)
On 2016/06/03 06:59:40, stgao wrote:
> What if the test is "a_PRE_b.PRE_test"?

Done.

[stgao, 2016-06-06 23:34:32]

lgtm once comment is addressed.

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

https://codereview.chromium.org/2027333002/diff/60001/appengine/findit/waterf...
appengine/findit/waterfall/swarming_tasks_to_try_job_pipeline.py:37:
pre_position = base_test.find(_PRE_TEST_PREFIX)
This seems incorrect.

To make it simpler, we could separate the suite name from test name during
processing suite_name.test_name, and then merge them back.

[chanli, 2016-06-08 21:10:59]

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

https://codereview.chromium.org/2027333002/diff/60001/appengine/findit/waterf...
appengine/findit/waterfall/swarming_tasks_to_try_job_pipeline.py:37:
pre_position = base_test.find(_PRE_TEST_PREFIX)
On 2016/06/06 23:34:32, stgao wrote:
> This seems incorrect.
> 
> To make it simpler, we could separate the suite name from test name during
> processing suite_name.test_name, and then merge them back.

Done.

[stgao, 2016-06-17 18:40:09]

On 2016/06/08 21:10:59, chanli wrote:
>
https://codereview.chromium.org/2027333002/diff/60001/appengine/findit/waterf...
> File appengine/findit/waterfall/swarming_tasks_to_try_job_pipeline.py (right):
> 
>
https://codereview.chromium.org/2027333002/diff/60001/appengine/findit/waterf...
> appengine/findit/waterfall/swarming_tasks_to_try_job_pipeline.py:37:
> pre_position = base_test.find(_PRE_TEST_PREFIX)
> On 2016/06/06 23:34:32, stgao wrote:
> > This seems incorrect.
> > 
> > To make it simpler, we could separate the suite name from test name during
> > processing suite_name.test_name, and then merge them back.
> 
> Done.

Is there a reason we haven't committed this CL?

[chanli, 2016-06-17 20:27:50]

The CQ bit was checked by chanli@chromium.org

[chanli, 2016-06-17 20:27:50]

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

[commit-bot: I haz the power, 2016-06-17 20:28:01]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/2027333002/100001

[commit-bot: I haz the power, 2016-06-17 20:41:24]

Description was changed from

==========
[Findit] don't included skipped or unknown tests in swarming tasks into failed
tests.

When record failed tests in detect_first_failure pipeline and check flakiness in
swarming rerun, don't include skipped or unknown tests as failed.

BUG=616313
==========

to

==========
[Findit] don't included skipped or unknown tests in swarming tasks into failed
tests.

When record failed tests in detect_first_failure pipeline and check flakiness in
swarming rerun, don't include skipped or unknown tests as failed.

BUG=616313

Committed:
https://chromium.googlesource.com/infra/infra/+/5267507e65d388cc288f57a44f9fe...
==========

[commit-bot: I haz the power, 2016-06-17 20:41:25]

Committed patchset #6 (id:100001) as
https://chromium.googlesource.com/infra/infra/+/5267507e65d388cc288f57a44f9fe...