recipe_tryjob: Allow test bypass.

recipe_tryjob: Allow test bypass. 

This allows recipe authors to cause this tryjob to turn green by adding the "Recipe-Tryjob-Bypass-Reason" git footer to their CL.

BUG=591172
Committed: https://chromium.googlesource.com/chromium/tools/build/+/f106ee6ee4e976af0be9823a1ad0c22a4c3b598f

[martiniss, 2016-05-23 23:21:54]

martiniss@chromium.org changed reviewers:
+ estaab@chromium.org, iannucci@chromium.org

[martiniss, 2016-05-23 23:21:54]

PTAL

[martiniss, 2016-05-23 23:33:29]

Description was changed from

==========
recipe_tryjob: Add the ability to ignore the result.

BUG=
==========

to

==========
recipe_tryjob: Allow test bypass. 

This allows recipe authors to cause this tryjob to turn green by adding the
"Recipe-Tryjob-Bypass-Reason" git footer to their CL.

BUG=591172
==========

[Paweł Hajdan Jr., 2016-05-25 00:51:54]

phajdan.jr@chromium.org changed reviewers:
+ phajdan.jr@chromium.org

[Paweł Hajdan Jr., 2016-05-25 00:51:55]

https://codereview.chromium.org/1971983002/diff/20001/scripts/slave/recipe_mo...
File scripts/slave/recipe_modules/recipe_tryjob/api.py (right):

https://codereview.chromium.org/1971983002/diff/20001/scripts/slave/recipe_mo...
scripts/slave/recipe_modules/recipe_tryjob/api.py:344: if
should_fail_build_mapping.get(proj):
Shouldn't we re-raise the exception here?

It's not obvious that AggregatedStepFailure would be raised if we catch
everything here, right?

Alternatively, always check failures for being non-empty, rather than only if we
catch AggregatedStepFailure.

WDYT?

[martiniss, 2016-05-25 17:36:30]

https://codereview.chromium.org/1971983002/diff/20001/scripts/slave/recipe_mo...
File scripts/slave/recipe_modules/recipe_tryjob/api.py (right):

https://codereview.chromium.org/1971983002/diff/20001/scripts/slave/recipe_mo...
scripts/slave/recipe_modules/recipe_tryjob/api.py:344: if
should_fail_build_mapping.get(proj):
On 2016/05/25 at 00:51:55, Paweł Hajdan Jr. wrote:
> Shouldn't we re-raise the exception here?
Re-raising the exception here would mean we don't run all the tests; I'd like to
run tests for every repo, so that they know exactly which ones broke. 
> 
> It's not obvious that AggregatedStepFailure would be raised if we catch
everything here, right?

It does; see the three_patches_fail_not_ok test case.

> 
> Alternatively, always check failures for being non-empty, rather than only if
we catch AggregatedStepFailure.
> 
> WDYT?

[Paweł Hajdan Jr., 2016-05-25 17:58:10]

LGTM

https://codereview.chromium.org/1971983002/diff/20001/scripts/slave/recipe_mo...
File scripts/slave/recipe_modules/recipe_tryjob/api.py (right):

https://codereview.chromium.org/1971983002/diff/20001/scripts/slave/recipe_mo...
scripts/slave/recipe_modules/recipe_tryjob/api.py:344: if
should_fail_build_mapping.get(proj):
I think I understand this now.

defer_results results in the original exception (possibly raised by
self.simulation_test) getting caught, registered, and AggregatedStepFailure
thrown later.

With get_result we're explicitly looking at that exception. Makes sense.

[martiniss, 2016-05-25 17:59:13]

On 2016/05/25 at 17:58:10, phajdan.jr wrote:
> LGTM
> 
>
https://codereview.chromium.org/1971983002/diff/20001/scripts/slave/recipe_mo...
> File scripts/slave/recipe_modules/recipe_tryjob/api.py (right):
> 
>
https://codereview.chromium.org/1971983002/diff/20001/scripts/slave/recipe_mo...
> scripts/slave/recipe_modules/recipe_tryjob/api.py:344: if
should_fail_build_mapping.get(proj):
> I think I understand this now.
> 
> defer_results results in the original exception (possibly raised by
self.simulation_test) getting caught, registered, and AggregatedStepFailure
thrown later.
> 
> With get_result we're explicitly looking at that exception. Makes sense.
Yup!

[martiniss, 2016-05-25 17:59:15]

The CQ bit was checked by martiniss@chromium.org

[commit-bot: I haz the power, 2016-05-25 17:59:21]

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

[commit-bot: I haz the power, 2016-05-25 18:03:02]

Description was changed from

==========
recipe_tryjob: Allow test bypass. 

This allows recipe authors to cause this tryjob to turn green by adding the
"Recipe-Tryjob-Bypass-Reason" git footer to their CL.

BUG=591172
==========

to

==========
recipe_tryjob: Allow test bypass. 

This allows recipe authors to cause this tryjob to turn green by adding the
"Recipe-Tryjob-Bypass-Reason" git footer to their CL.

BUG=591172

Committed:
https://chromium.googlesource.com/chromium/tools/build/+/f106ee6ee4e976af0be9...
==========

[commit-bot: I haz the power, 2016-05-25 18:03:03]

Committed patchset #2 (id:20001) as
https://chromium.googlesource.com/chromium/tools/build/+/f106ee6ee4e976af0be9...