Make step status EXCEPTION when test runner has infrastructure failure.

Make AMP step status EXCEPTION when test runner has infrastructure failure.

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

[mikecase (-- gone --), 2015-02-24 19:35:36]

mikecase@chromium.org changed reviewers:
+ jbudorick@chromium.org

[mikecase (-- gone --), 2015-02-24 19:35:37]

This change won't do anything until this CL lands...
https://codereview.chromium.org/950893004/
... but then it should make the trigger step show up as an infra failure if
appurify devices couldn't be found.

[jbudorick, 2015-02-27 15:17:27]

https://codereview.chromium.org/954743003/diff/20001/scripts/slave/recipe_mod...
File scripts/slave/recipe_modules/amp/api.py (right):

https://codereview.chromium.org/954743003/diff/20001/scripts/slave/recipe_mod...
scripts/slave/recipe_modules/amp/api.py:16: """Call this once before any of the
other APIs in this module."""
This should be rephrased.

https://codereview.chromium.org/954743003/diff/20001/scripts/slave/recipe_mod...
File scripts/slave/recipe_modules/amp/example.py (right):

https://codereview.chromium.org/954743003/diff/20001/scripts/slave/recipe_mod...
scripts/slave/recipe_modules/amp/example.py:104: api.amp.setup()
Can't say I'm a big fan of this... looking into alternatives.

[mikecase (-- gone --), 2015-02-27 18:10:58]

https://codereview.chromium.org/954743003/diff/20001/scripts/slave/recipe_mod...
File scripts/slave/recipe_modules/amp/api.py (right):

https://codereview.chromium.org/954743003/diff/20001/scripts/slave/recipe_mod...
scripts/slave/recipe_modules/amp/api.py:129: step_result.presentation.status ==
self.m.step.SUCCESS):
Also will need to add this logic to the trigger step which I forgot to do. Am
thinking of just adding a function like handle_test_runner_step_results (with a
better name if I can think of one) in chromium_android api and just set the step
status there.

https://codereview.chromium.org/954743003/diff/20001/scripts/slave/recipe_mod...
File scripts/slave/recipe_modules/amp/example.py (right):

https://codereview.chromium.org/954743003/diff/20001/scripts/slave/recipe_mod...
scripts/slave/recipe_modules/amp/example.py:104: api.amp.setup()
On 2015/02/27 15:17:27, jbudorick wrote:
> Can't say I'm a big fan of this... looking into alternatives.
I based what I did on what recipe_modules/gpu/api.py and the gpu recipes do
(they basically have to setup some configs as well). I welcome any alternatives
and I'll look around as well.

[mikecase (-- gone --), 2015-02-27 21:11:33]

Changed things a little bit. Added a function to chromium_android/api.py which
wraps the pylib testrunner calls and handles setting the step status based on
testrunner exit code.

[jbudorick, 2015-03-02 16:26:02]

https://codereview.chromium.org/954743003/diff/60001/scripts/slave/recipe_mod...
File scripts/slave/recipe_modules/chromium_android/api.py (right):

https://codereview.chromium.org/954743003/diff/60001/scripts/slave/recipe_mod...
scripts/slave/recipe_modules/chromium_android/api.py:735: def test_runner(self,
name, args=None, **kwargs):
I like this idea, although I'm not sure if it should be in here or in its own
recipe_module. (I'll yield to luqui on that one.)

Document the arguments, though.

[mikecase (-- gone --), 2015-03-02 18:07:07]

mikecase@chromium.org changed reviewers:
+ luqui@chromium.org

[mikecase (-- gone --), 2015-03-02 18:07:07]

Adding luqui. 

This CL wraps running the test_runner.py script in recipes so that we can handle
the exit codes from that script.

[mikecase (-- gone --), 2015-03-02 18:08:44]

https://codereview.chromium.org/954743003/diff/60001/scripts/slave/recipe_mod...
File scripts/slave/recipe_modules/chromium_android/api.py (right):

https://codereview.chromium.org/954743003/diff/60001/scripts/slave/recipe_mod...
scripts/slave/recipe_modules/chromium_android/api.py:735: def test_runner(self,
name, args=None, **kwargs):
On 2015/03/02 16:26:02, jbudorick wrote:
> I like this idea, although I'm not sure if it should be in here or in its own
> recipe_module. (I'll yield to luqui on that one.)
> 
> Document the arguments, though.

Added documentation for the arguments. Added luqui to the CL. Luqui, is the
test_runner() function good here, or should it be part of a new test_runner
recipe_module?

[luqui, 2015-03-02 23:40:59]

I'm not a huge fan of .setup() either.  It is probably better than the
alternatives though given the current config system. 

I don't think test_runner needs a recipe module; feels like overkill.

https://codereview.chromium.org/954743003/diff/80001/scripts/slave/recipe_mod...
File scripts/slave/recipe_modules/chromium_android/config.py (right):

https://codereview.chromium.org/954743003/diff/80001/scripts/slave/recipe_mod...
scripts/slave/recipe_modules/chromium_android/config.py:32:
test_runner_exit_codes = Dict(value_type=int),
This doesn't feel right to me.  I would prefer to hardcode this dictionary in
the test_runner function -- keeps the implementation details from leaking all
over the place.

Unless you expect different configurations to use different dictionaries, which
I find unlikely.

[mikecase (-- gone --), 2015-03-03 00:46:47]

https://codereview.chromium.org/954743003/diff/80001/scripts/slave/recipe_mod...
File scripts/slave/recipe_modules/chromium_android/config.py (right):

https://codereview.chromium.org/954743003/diff/80001/scripts/slave/recipe_mod...
scripts/slave/recipe_modules/chromium_android/config.py:32:
test_runner_exit_codes = Dict(value_type=int),
On 2015/03/02 23:40:58, luqui wrote:
> This doesn't feel right to me.  I would prefer to hardcode this dictionary in
> the test_runner function -- keeps the implementation details from leaking all
> over the place.
> 
> Unless you expect different configurations to use different dictionaries,
which
> I find unlikely.

Done.

[luqui, 2015-03-03 20:58:24]

lgtm

[mikecase (-- gone --), 2015-03-03 21:52:30]

The CQ bit was checked by mikecase@chromium.org

[commit-bot: I haz the power, 2015-03-03 21:53:02]

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

[commit-bot: I haz the power, 2015-03-03 21:57:04]

Committed patchset #6 (id:100001) as
http://src.chromium.org/viewvc/chrome?view=rev&revision=294315