Avoid crashing due to missing trace_output option in Monkey test

Avoid crashing due to missing trace_output option in Monkey test

BUG=712895
Review-Url: https://codereview.chromium.org/2872813003
Cr-Commit-Position: refs/heads/master@{#471154}
Committed: https://chromium.googlesource.com/chromium/src/+/2b6930c7e92c1c2df1576907c6b965ac72a752ce

[aluo1, 2017-05-09 00:17:54]

Description was changed from

==========
Add tracing parser for monkey subparser

BUG=712895
==========

to

==========
Add tracing parser for monkey subparser

BUG=712895
==========

[aluo1, 2017-05-09 00:17:55]

aluo@google.com changed reviewers:
+ jbudorick@chromium.org, shenghuazhang@chromium.org

[aluo1, 2017-05-09 18:08:40]

aluo@google.com changed reviewers:
+ aluo@google.com

[aluo1, 2017-05-09 18:08:40]

Please review

[jbudorick, 2017-05-09 18:18:32]

This is fine, but I'm not sure it'll do anything by itself -- you may need to
add some trace_event calls or decorators to local_device_monkey_test_run.py.

[shenghuazhang, 2017-05-09 18:44:23]

On 2017/05/09 18:18:32, jbudorick wrote:
> This is fine, but I'm not sure it'll do anything by itself -- you may need to
> add some trace_event calls or decorators to local_device_monkey_test_run.py.

Similar to local_device_instrumentation_test_run.py, usually could add
trace_event/ decorators for those steps 
1. SetUp
2. the step that device triggers tests
(https://codesearch.chromium.org/chromium/src/build/android/pylib/local/device...)
3. TearDown

Not very familiar with monkey test, but by looking into
local_device_monkey_test_run.py, I think it's fine to omit adding trace_event
for 'SetUP' and 'TearDown'? Since those are simple passing functions.

[aluo1, 2017-05-09 19:07:58]

On 2017/05/09 18:44:23, shenghuazhang wrote:
> On 2017/05/09 18:18:32, jbudorick wrote:
> > This is fine, but I'm not sure it'll do anything by itself -- you may need
to
> > add some trace_event calls or decorators to local_device_monkey_test_run.py.
> 
> Similar to local_device_instrumentation_test_run.py, usually could add
> trace_event/ decorators for those steps 
> 1. SetUp
> 2. the step that device triggers tests
>
(https://codesearch.chromium.org/chromium/src/build/android/pylib/local/device...)
> 3. TearDown
> 
> Not very familiar with monkey test, but by looking into
> local_device_monkey_test_run.py, I think it's fine to omit adding trace_event
> for 'SetUP' and 'TearDown'? Since those are simple passing functions.

Just avoiding the crash that happens if the trace_output option was not set to
the default value of false:

|00:04:05   python
/b/build/slave/test-offical-arm_64/build/src/build/android/test_runner.py monkey
--browser=chrome --event-count=50000
E    0.000s Main  Unrecognized error occurred.
Traceback (most recent call last):
  File
"/b/build/slave/test-offical-arm_64/build/src/build/android/test_runner.py",
line 917, in main
    return RunTestsCommand(args)
  File
"/b/build/slave/test-offical-arm_64/build/src/build/android/test_runner.py",
line 676, in RunTestsCommand
    return RunTestsInPlatformMode(args)
  File
"/b/build/slave/test-offical-arm_64/build/src/build/android/test_runner.py",
line 774, in RunTestsInPlatformMode
    env = environment_factory.CreateEnvironment(args, infra_error)
  File
"/b/build/slave/test-offical-arm_64/build/src/build/android/pylib/base/environment_factory.py",
line 13, in CreateEnvironment
    return local_device_environment.LocalDeviceEnvironment(args, error_func)
  File
"/b/build/slave/test-offical-arm_64/build/src/build/android/pylib/local/device/local_device_environment.py",
line 97, in __init__
    self._trace_output = args.trace_output
AttributeError: 'Namespace' object has no attribute 'trace_output'

I'm not enabling tracing for Monkey test in this cl.

[aluo, 2017-05-09 19:09:18]

Description was changed from

==========
Add tracing parser for monkey subparser

BUG=712895
==========

to

==========
Avoid crashing due to missing trace_output option in Monkey test

BUG=712895
==========

[aluo1, 2017-05-09 19:09:40]

Description was changed from

==========
Avoid crashing due to missing trace_output option in Monkey test

BUG=712895
==========

to

==========
Avoid crashing due to missing trace_output option in Monkey test

BUG=712895
==========

[aluo1, 2017-05-09 19:10:04]

On 2017/05/09 19:07:58, aluo1 wrote:
> On 2017/05/09 18:44:23, shenghuazhang wrote:
> > On 2017/05/09 18:18:32, jbudorick wrote:
> > > This is fine, but I'm not sure it'll do anything by itself -- you may need
> to
> > > add some trace_event calls or decorators to
local_device_monkey_test_run.py.
> > 
> > Similar to local_device_instrumentation_test_run.py, usually could add
> > trace_event/ decorators for those steps 
> > 1. SetUp
> > 2. the step that device triggers tests
> >
>
(https://codesearch.chromium.org/chromium/src/build/android/pylib/local/device...)
> > 3. TearDown
> > 
> > Not very familiar with monkey test, but by looking into
> > local_device_monkey_test_run.py, I think it's fine to omit adding
trace_event
> > for 'SetUP' and 'TearDown'? Since those are simple passing functions.
> 
> Just avoiding the crash that happens if the trace_output option was not set to
> the default value of false:
> 
> |00:04:05   python
> /b/build/slave/test-offical-arm_64/build/src/build/android/test_runner.py
monkey
> --browser=chrome --event-count=50000
> E    0.000s Main  Unrecognized error occurred.
> Traceback (most recent call last):
>   File
> "/b/build/slave/test-offical-arm_64/build/src/build/android/test_runner.py",
> line 917, in main
>     return RunTestsCommand(args)
>   File
> "/b/build/slave/test-offical-arm_64/build/src/build/android/test_runner.py",
> line 676, in RunTestsCommand
>     return RunTestsInPlatformMode(args)
>   File
> "/b/build/slave/test-offical-arm_64/build/src/build/android/test_runner.py",
> line 774, in RunTestsInPlatformMode
>     env = environment_factory.CreateEnvironment(args, infra_error)
>   File
>
"/b/build/slave/test-offical-arm_64/build/src/build/android/pylib/base/environment_factory.py",
> line 13, in CreateEnvironment
>     return local_device_environment.LocalDeviceEnvironment(args, error_func)
>   File
>
"/b/build/slave/test-offical-arm_64/build/src/build/android/pylib/local/device/local_device_environment.py",
> line 97, in __init__
>     self._trace_output = args.trace_output
> AttributeError: 'Namespace' object has no attribute 'trace_output'
> 
> I'm not enabling tracing for Monkey test in this cl.

I've fixed the cl description to match this cl.

[jbudorick, 2017-05-09 19:15:06]

Ah, should've read the bug. Sorry about that.

We shouldn't be adding the option if we're not adding support for tracing. This
should instead check hasattr(args, 'trace_output') in
local_device_environment.py and set self._trace_output = None if it's not
present.

[aluo1, 2017-05-09 19:30:44]

On 2017/05/09 19:15:06, jbudorick wrote:
> Ah, should've read the bug. Sorry about that.
> 
> We shouldn't be adding the option if we're not adding support for tracing.
This
> should instead check hasattr(args, 'trace_output') in
> local_device_environment.py and set self._trace_output = None if it's not
> present.

done

[jbudorick, 2017-05-09 19:33:34]

lgtm w/ nit

https://codereview.chromium.org/2872813003/diff/20001/build/android/pylib/loc...
File build/android/pylib/local/device/local_device_environment.py (right):

https://codereview.chromium.org/2872813003/diff/20001/build/android/pylib/loc...
build/android/pylib/local/device/local_device_environment.py:99:
self._trace_output = args.trace_output
nit: 2 space indent

[aluo1, 2017-05-09 19:38:05]

The CQ bit was checked by aluo@google.com to run a CQ dry run

[commit-bot: I haz the power, 2017-05-09 19:38:45]

Dry run: 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-05-09 20:10:58]

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

[commit-bot: I haz the power, 2017-05-09 20:10:59]

Dry run: Try jobs failed on following builders:
  ios-simulator on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator/bui...)

[aluo1, 2017-05-09 20:54:57]

The CQ bit was checked by aluo@google.com to run a CQ dry run

[commit-bot: I haz the power, 2017-05-09 20:56:20]

Dry run: 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-05-09 22:38:01]

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

[commit-bot: I haz the power, 2017-05-09 22:38:03]

Dry run: Try jobs failed on following builders:
  linux_chromium_chromeos_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[aluo1, 2017-05-11 21:35:44]

The CQ bit was checked by aluo@google.com to run a CQ dry run

[commit-bot: I haz the power, 2017-05-11 21:36:42]

Dry run: 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-05-11 23:48:29]

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

[commit-bot: I haz the power, 2017-05-11 23:48:30]

Dry run: This issue passed the CQ dry run.

[aluo1, 2017-05-12 00:44:00]

The CQ bit was checked by aluo@google.com

[aluo1, 2017-05-12 00:44:00]

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/2872813003/#ps40001
(title: "Fix indent")

[commit-bot: I haz the power, 2017-05-12 00:44:58]

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-05-12 00:52:24]

CQ is committing da patch.

Bot data: {"patchset_id": 40001, "attempt_start_ts": 1494549840591770,
"parent_rev": "16a806a082b9bdea1c85f5ad5f126d6aa329dbe0", "commit_rev":
"2b6930c7e92c1c2df1576907c6b965ac72a752ce"}

[commit-bot: I haz the power, 2017-05-12 00:52:33]

Description was changed from

==========
Avoid crashing due to missing trace_output option in Monkey test

BUG=712895
==========

to

==========
Avoid crashing due to missing trace_output option in Monkey test

BUG=712895

Review-Url: https://codereview.chromium.org/2872813003
Cr-Commit-Position: refs/heads/master@{#471154}
Committed:
https://chromium.googlesource.com/chromium/src/+/2b6930c7e92c1c2df1576907c6b9...
==========

[commit-bot: I haz the power, 2017-05-12 00:52:35]

Committed patchset #3 (id:40001) as
https://chromium.googlesource.com/chromium/src/+/2b6930c7e92c1c2df1576907c6b9...