Add exit code for infra failures to testrunner script.

Add exit code for infra failures to testrunner script.

For recipes to know a failure in the testrunner was caused by
an infrastructure failure, we are adding a specific exit code.

BUG=451993
Committed: https://crrev.com/e740510219de4a3d7b0f7d1bf8a21bd86366e00a
Cr-Commit-Position: refs/heads/master@{#318341}

[mikecase (-- gone --), 2015-02-23 18:23:01]

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

[mikecase (-- gone --), 2015-02-23 22:06:10]

Addressed your feedback. Made everything more general. Any exception that
inherits from base_error.BaseError can set is_infra_error property to be
True/False (is False by default). If an exception occurs with is_infra_error set
to True, the test runner will exit with exit code constant.INFRA_EXIT_CODE.

[jbudorick, 2015-02-23 22:13:01]

https://codereview.chromium.org/950893004/diff/20001/build/android/pylib/base...
File build/android/pylib/base/base_error.py (right):

https://codereview.chromium.org/950893004/diff/20001/build/android/pylib/base...
build/android/pylib/base/base_error.py:1: # Copyright 2015 The Chromium Authors.
All rights reserved.
changed my mind, put this in pylib/utils/

https://codereview.chromium.org/950893004/diff/20001/build/android/pylib/remo...
File build/android/pylib/remote/device/remote_device_environment.py (right):

https://codereview.chromium.org/950893004/diff/20001/build/android/pylib/remo...
build/android/pylib/remote/device/remote_device_environment.py:305: raise
remote_device_helper.RemoteDeviceError('No device found.',
Either:
 - put a parameter on the first line and indent all subsequent parameter lines
to that depths, or
 - don't put a parameter on the first line and indent all parameter lines 4
spaces

do not mix the two.

https://codereview.chromium.org/950893004/diff/20001/build/android/test_runne...
File build/android/test_runner.py (right):

https://codereview.chromium.org/950893004/diff/20001/build/android/test_runne...
build/android/test_runner.py:1024: raise
What's your reasoning behind reraising non-infra errors but not infra errors?

[jbudorick, 2015-02-23 22:16:03]

https://codereview.chromium.org/950893004/diff/20001/build/android/pylib/base...
File build/android/pylib/base/base_error.py (right):

https://codereview.chromium.org/950893004/diff/20001/build/android/pylib/base...
build/android/pylib/base/base_error.py:1: # Copyright 2015 The Chromium Authors.
All rights reserved.
On 2015/02/23 22:13:00, jbudorick wrote:
> changed my mind, put this in pylib/utils/

(reasoning here: we're going to have to eventually extract parts of pylib into
their own library for telemetry etc, and currently those parts are limited to
pylib/device/, pylib/utils/, and some parts of pylib/. Since pylib/device/
depends on this, it's easier if we put it in pylib/utils/ for now.)

[mikecase (-- gone --), 2015-02-23 22:29:27]

https://codereview.chromium.org/950893004/diff/20001/build/android/pylib/base...
File build/android/pylib/base/base_error.py (right):

https://codereview.chromium.org/950893004/diff/20001/build/android/pylib/base...
build/android/pylib/base/base_error.py:1: # Copyright 2015 The Chromium Authors.
All rights reserved.
On 2015/02/23 22:16:03, jbudorick wrote:
> On 2015/02/23 22:13:00, jbudorick wrote:
> > changed my mind, put this in pylib/utils/
> 
> (reasoning here: we're going to have to eventually extract parts of pylib into
> their own library for telemetry etc, and currently those parts are limited to
> pylib/device/, pylib/utils/, and some parts of pylib/. Since pylib/device/
> depends on this, it's easier if we put it in pylib/utils/ for now.)

Done.

https://codereview.chromium.org/950893004/diff/20001/build/android/pylib/remo...
File build/android/pylib/remote/device/remote_device_environment.py (right):

https://codereview.chromium.org/950893004/diff/20001/build/android/pylib/remo...
build/android/pylib/remote/device/remote_device_environment.py:305: raise
remote_device_helper.RemoteDeviceError('No device found.',
On 2015/02/23 22:13:00, jbudorick wrote:
> Either:
>  - put a parameter on the first line and indent all subsequent parameter lines
> to that depths, or
>  - don't put a parameter on the first line and indent all parameter lines 4
> spaces
> 
> do not mix the two.

Done.

https://codereview.chromium.org/950893004/diff/20001/build/android/test_runne...
File build/android/test_runner.py (right):

https://codereview.chromium.org/950893004/diff/20001/build/android/test_runne...
build/android/test_runner.py:1024: raise
On 2015/02/23 22:13:00, jbudorick wrote:
> What's your reasoning behind reraising non-infra errors but not infra errors?

You're right. Its a lot clearer if I just handle both infra and non-infra errors
in the same way.

I did it like this originally because I didn't have to catch non-infra errors
(by default python exceptions exit with exit code 1 which is the same as
constants.ERROR_EXIT_CODE).

https://codereview.chromium.org/950893004/diff/40001/build/android/test_runne...
File build/android/test_runner.py (right):

https://codereview.chromium.org/950893004/diff/40001/build/android/test_runne...
build/android/test_runner.py:1021: logging.exception('An error occured.')
Just fyi.

logging.exception WILL print the error message of the exception, but it also
requires you pass it a message... so I just had to give it something super
generic  :/

[jbudorick, 2015-02-23 22:31:29]

https://codereview.chromium.org/950893004/diff/40001/build/android/test_runne...
File build/android/test_runner.py (right):

https://codereview.chromium.org/950893004/diff/40001/build/android/test_runne...
build/android/test_runner.py:1021: logging.exception('An error occured.')
On 2015/02/23 22:29:27, mikecase wrote:
> Just fyi.
> 
> logging.exception WILL print the error message of the exception, but it also
> requires you pass it a message... so I just had to give it something super
> generic  :/

We could split by infra / non-infra?

What does this wind up looking like?

[mikecase (-- gone --), 2015-02-23 22:48:23]

https://codereview.chromium.org/950893004/diff/40001/build/android/test_runne...
File build/android/test_runner.py (right):

https://codereview.chromium.org/950893004/diff/40001/build/android/test_runne...
build/android/test_runner.py:1021: logging.exception('An error occured.')
On 2015/02/23 22:31:29, jbudorick wrote:
> On 2015/02/23 22:29:27, mikecase wrote:
> > Just fyi.
> > 
> > logging.exception WILL print the error message of the exception, but it also
> > requires you pass it a message... so I just had to give it something super
> > generic  :/
> 
> We could split by infra / non-infra?
> 
> What does this wind up looking like?

Wrote a quick example. Output of logging.exception looks something like this....

ERROR:root:"Logging.exception message."
Traceback (most recent call last):
  File "test.py", line 23, in <module>
    level1()
  File "test.py", line 13, in level1
    level2()
  File "test.py", line 16, in level2
    level3()
  File "test.py", line 19, in level3
    raise CustomException('"Custom exception message."')
CustomException: "Custom exception message."

Looks like it will print all the information we would need. We could even change
it to logging.exception(''), but that could be confusing to read.

[mikecase (-- gone --), 2015-02-25 19:51:21]

I never really understood what you wanted me to check for this CL. You said
something like "check how it will change how sheriffs see the error". The
stacktrace and error message will get printed through stdout with the
logging.exception call. logging.exception logs with log level ERROR. I dont
think the appearance of the logs should change much. Is there anything specific
you want me to verify.

When you have a change give this (and the corresponding recipe change
https://codereview.chromium.org/954743003/ ) a look. Thanks!

[jbudorick, 2015-02-25 20:29:30]

On 2015/02/25 19:51:21, mikecase wrote:
> I never really understood what you wanted me to check for this CL. You said
> something like "check how it will change how sheriffs see the error". The
> stacktrace and error message will get printed through stdout with the
> logging.exception call. logging.exception logs with log level ERROR. I dont
> think the appearance of the logs should change much. Is there anything
specific
> you want me to verify.
> 
> When you have a change give this (and the corresponding recipe change
> https://codereview.chromium.org/954743003/ ) a look. Thanks!

I just wanted an example of an actual exception trace.

[mikecase (-- gone --), 2015-02-26 19:56:28]

On 2015/02/25 20:29:30, jbudorick wrote:
> On 2015/02/25 19:51:21, mikecase wrote:
> > I never really understood what you wanted me to check for this CL. You said
> > something like "check how it will change how sheriffs see the error". The
> > stacktrace and error message will get printed through stdout with the
> > logging.exception call. logging.exception logs with log level ERROR. I dont
> > think the appearance of the logs should change much. Is there anything
> specific
> > you want me to verify.
> > 
> > When you have a change give this (and the corresponding recipe change
> > https://codereview.chromium.org/954743003/ ) a look. Thanks!
> 
> I just wanted an example of an actual exception trace.

Just ran the test runner and caused an exception to be raised. This is what
stdout looked like.

================ WITH MY CHANGE =======================
I   11.589s Main  Triggering test run.
I   11.589s Main  Using default runner package:
org.chromium.native_test.ChromeNativeTestInstrumentationTestRunner
I   11.590s Main  Uploading
/usr/local/google/code/clankium/src/out/Debug/apks/remote_device_dummy.apk to
remote service.
I   12.011s Main  Uploading /tmp/tmpHazyjU.zip to remote service.
I   14.123s Main  Setting config: {'runner':
'org.chromium.native_test.ChromeNativeTestInstrumentationTestRunner',
'host_test': 'ui_touch_selection_unittests-debug.apk', 'sdcard_files':
'tmpj24qLK.flags.txt', 'environment_vars':
'org.chromium.native_test.ChromeNativeTestInstrumentationTestRunner.OnlyOutputFailures=None,org.chromium.native_test.ChromeNativeTestActivity.CommandLineFile=tmpj24qLK.flags.txt'}
I   14.123s Main  Generating config file for test.
I   14.223s Main  Revoking remote service access token
E   14.267s Main  An error occured.
Traceback (most recent call last):
  File "build/android/test_runner.py", line 1019, in main
    return RunTestsCommand(args, parser)
  File "build/android/test_runner.py", line 897, in RunTestsCommand
    return RunTestsInPlatformMode(args, parser)
  File "build/android/test_runner.py", line 945, in RunTestsInPlatformMode
    results = test_run.RunTests()
  File
"/usr/local/google/code/clankium/src/build/android/pylib/remote/device/remote_device_test_run.py",
line 69, in RunTests
    is_infra_error=True)
RemoteDeviceError: Fake error!



================ WITHOUT MY CHANGE =======================
I   10.670s Main  Triggering test run.
I   10.670s Main  Using default runner package:
org.chromium.native_test.ChromeNativeTestInstrumentationTestRunner
I   10.671s Main  Uploading
/usr/local/google/code/clankium/src/out/Debug/apks/remote_device_dummy.apk to
remote service.
I   11.067s Main  Uploading /tmp/tmpHraV5K.zip to remote service.
I   12.423s Main  Setting config: {'runner':
'org.chromium.native_test.ChromeNativeTestInstrumentationTestRunner',
'host_test': 'ui_touch_selection_unittests-debug.apk', 'sdcard_files':
'tmp12joG9.flags.txt', 'environment_vars':
'org.chromium.native_test.ChromeNativeTestInstrumentationTestRunner.OnlyOutputFailures=None,org.chromium.native_test.ChromeNativeTestActivity.CommandLineFile=tmp12joG9.flags.txt'}
I   12.423s Main  Generating config file for test.
I   12.531s Main  Revoking remote service access token
Traceback (most recent call last):
  File "build/android/test_runner.py", line 1020, in <module>
    sys.exit(main())
  File "build/android/test_runner.py", line 1016, in main
    return RunTestsCommand(args, parser)
  File "build/android/test_runner.py", line 896, in RunTestsCommand
    return RunTestsInPlatformMode(args, parser)
  File "build/android/test_runner.py", line 944, in RunTestsInPlatformMode
    results = test_run.RunTests()
  File
"/usr/local/google/code/clankium/src/build/android/pylib/remote/device/remote_device_test_run.py",
line 68, in RunTests
    'Fake error!')
pylib.remote.device.remote_device_helper.RemoteDeviceError: Fake error!



================ DIFFERENCE? =====================
Added
+ E   14.267s Main  An error occured.

Removed sys.exit(main()) from stacktrace
- File "build/android/test_runner.py", line 1020, in <module>
  sys.exit(main())

Slightly changed how exception is displayed.
+ RemoteDeviceError: Fake error!
- pylib.remote.device.remote_device_helper.RemoteDeviceError: Fake error!

[jbudorick, 2015-02-26 20:07:43]

lgtm w/ nits

https://codereview.chromium.org/950893004/diff/80001/build/android/test_runne...
File build/android/test_runner.py (right):

https://codereview.chromium.org/950893004/diff/80001/build/android/test_runne...
build/android/test_runner.py:1021: logging.exception('An error occured.')
nit: occured -> occurred

https://codereview.chromium.org/950893004/diff/80001/build/android/test_runne...
build/android/test_runner.py:1026: 
nit: for consistency, I would add a generic exception handler, e.g.:

  except:
    logging.exception('Unrecognized error occurred.')
    return constants.ERROR_EXIT_CODE

This will run afoul of pylint, but since this is the top level exception
handler, I think that's ok.

[mikecase (-- gone --), 2015-02-26 21:51:39]

https://codereview.chromium.org/950893004/diff/80001/build/android/test_runne...
File build/android/test_runner.py (right):

https://codereview.chromium.org/950893004/diff/80001/build/android/test_runne...
build/android/test_runner.py:1021: logging.exception('An error occured.')
On 2015/02/26 20:07:43, jbudorick wrote:
> nit: occured -> occurred

Done.

https://codereview.chromium.org/950893004/diff/80001/build/android/test_runne...
build/android/test_runner.py:1026: 
On 2015/02/26 20:07:43, jbudorick wrote:
> nit: for consistency, I would add a generic exception handler, e.g.:
> 
>   except:
>     logging.exception('Unrecognized error occurred.')
>     return constants.ERROR_EXIT_CODE
> 
> This will run afoul of pylint, but since this is the top level exception
> handler, I think that's ok.

Done.

[mikecase (-- gone --), 2015-02-26 21:52:39]

The CQ bit was checked by mikecase@chromium.org

[mikecase (-- gone --), 2015-02-26 21:52:39]

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

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

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

[commit-bot: I haz the power, 2015-02-26 23:08:31]

Committed patchset #6 (id:100001)

[commit-bot: I haz the power, 2015-02-26 23:09:21]

Patchset 6 (id:??) landed as
https://crrev.com/e740510219de4a3d7b0f7d1bf8a21bd86366e00a
Cr-Commit-Position: refs/heads/master@{#318341}