Always identify test names that lead to failures

Always identify test names that lead to failures

BUG=chromium:699120, chromium:705791
Review-Url: https://codereview.chromium.org/2797023002
Committed: https://github.com/luci/recipes-py/commit/0586f16a4911be4855ce52621e143662e1d58062

[Paweł Hajdan Jr., 2017-04-04 20:20:50]

The CQ bit was checked by phajdan.jr@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-04-04 20:21:01]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[iannucci, 2017-04-04 20:24:50]

iannucci@chromium.org changed reviewers:
+ iannucci@chromium.org

[iannucci, 2017-04-04 20:24:51]

lgtm w/ comments

https://codereview.chromium.org/2797023002/diff/20001/recipe_engine/test_resu...
File recipe_engine/test_result.proto (right):

https://codereview.chromium.org/2797023002/diff/20001/recipe_engine/test_resu...
recipe_engine/test_result.proto:63: InternalFailure internal_failure = 3;
I suspect 'internal_failure' isn't quite right; most of the time this just means
that the recipe code needs modification. Maybe 'incompatibility_failure'?

followup: I would also add some comments to this proto to document what the
different failure types mean, exactly.

https://codereview.chromium.org/2797023002/diff/20001/recipe_modules/step/exa...
File recipe_modules/step/example.py (right):

https://codereview.chromium.org/2797023002/diff/20001/recipe_modules/step/exa...
recipe_modules/step/example.py:35: raise Exception('fuu')
did you mean to leave this in? should this be added as a different test?

[commit-bot: I haz the power, 2017-04-04 20:26:11]

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

[commit-bot: I haz the power, 2017-04-04 20:26:12]

Dry run: Try jobs failed on following builders:
  Recipes-py Presubmit on luci.infra.try (JOB_FAILED,
https://luci-milo.appspot.com/swarming/task/35537260d8623910)

[Paweł Hajdan Jr., 2017-04-04 20:28:39]

The CQ bit was checked by phajdan.jr@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-04-04 20:28:42]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Paweł Hajdan Jr., 2017-04-04 20:30:57]

The CQ bit was checked by phajdan.jr@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-04-04 20:31:01]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Paweł Hajdan Jr., 2017-04-04 20:32:04]

PTAL. Suggestions are welcome.

https://codereview.chromium.org/2797023002/diff/20001/recipe_engine/test_resu...
File recipe_engine/test_result.proto (right):

https://codereview.chromium.org/2797023002/diff/20001/recipe_engine/test_resu...
recipe_engine/test_result.proto:63: InternalFailure internal_failure = 3;
On 2017/04/04 20:24:51, iannucci wrote:
> I suspect 'internal_failure' isn't quite right; most of the time this just
means
> that the recipe code needs modification. Maybe 'incompatibility_failure'?
> 
> followup: I would also add some comments to this proto to document what the
> different failure types mean, exactly.

Added comment to the proto.

As indicated by the comment, incompatibility in user recipe code is not the only
case which can trigger this.

https://codereview.chromium.org/2797023002/diff/20001/recipe_modules/step/exa...
File recipe_modules/step/example.py (right):

https://codereview.chromium.org/2797023002/diff/20001/recipe_modules/step/exa...
recipe_modules/step/example.py:35: raise Exception('fuu')
On 2017/04/04 20:24:51, iannucci wrote:
> did you mean to leave this in? should this be added as a different test?

Ouch, indeed left by mistake. Also detected by presubmit. Removed now.

By the way, we do have test coverage for this - see changing tests.

[iannucci, 2017-04-04 20:34:02]

yep lgtm. I saw the tests later and forgot to remove the question from the
earlier comment.

[commit-bot: I haz the power, 2017-04-04 20:34:11]

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

[commit-bot: I haz the power, 2017-04-04 20:34:12]

Dry run: This issue passed the CQ dry run.

[Paweł Hajdan Jr., 2017-04-04 20:34:52]

The CQ bit was checked by phajdan.jr@chromium.org

[commit-bot: I haz the power, 2017-04-04 20:35:03]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-04-04 20:38:13]

CQ is committing da patch.

Bot data: {"patchset_id": 60001, "attempt_start_ts": 1491338092422280,
"parent_rev": "189f7dcc9e1151e6714987e26eb7de14c531d3ae", "commit_rev":
"0586f16a4911be4855ce52621e143662e1d58062"}

[commit-bot: I haz the power, 2017-04-04 20:38:19]

Description was changed from

==========
Always identify test names that lead to failures

BUG=chromium:699120, chromium:705791
==========

to

==========
Always identify test names that lead to failures

BUG=chromium:699120, chromium:705791

Review-Url: https://codereview.chromium.org/2797023002
Committed:
https://github.com/luci/recipes-py/commit/0586f16a4911be4855ce52621e143662e1d...
==========

[commit-bot: I haz the power, 2017-04-04 20:38:20]

Committed patchset #4 (id:60001) as
https://github.com/luci/recipes-py/commit/0586f16a4911be4855ce52621e143662e1d...