[android] Add support for passing command-line flags directly.

[android] Add support for passing command-line flags directly.

Previously, command-line flags to be passed to the test had to be
passed via:
 - (gtests) --test-arguments <string>
 - (instrumentation tests) --device-flags{,-file} <file>

This CL changes it s.t. any flags not recognized by the test runner
are passed down to the binary. It also removes --device-flags and
deprecates --test-arguments.

BUG=700366
Review-Url: https://codereview.chromium.org/2752493002
Cr-Commit-Position: refs/heads/master@{#457513}
Committed: https://chromium.googlesource.com/chromium/src/+/7d30806cf9bcb492d60dab0b3987aba72ff59f87

[jbudorick, 2017-03-13 21:54:56]

jbudorick@chromium.org changed reviewers:
+ jam@chromium.org, mikecase@chromium.org, shenghuazhang@chromium.org

[jam, 2017-03-13 21:59:17]

(I'm not qualified to review this, but thank you)

[mikecase (-- gone --), 2017-03-13 23:59:47]

https://codereview.chromium.org/2752493002/diff/1/build/android/pylib/gtest/g...
File build/android/pylib/gtest/gtest_test_instance.py (right):

https://codereview.chromium.org/2752493002/diff/1/build/android/pylib/gtest/g...
build/android/pylib/gtest/gtest_test_instance.py:350:
self._flags.append('--gtest_also_run_disabled_tests')
nit: wrong indentation

https://codereview.chromium.org/2752493002/diff/1/build/android/pylib/gtest/g...
build/android/pylib/gtest/gtest_test_instance.py:391: def
gtest_also_run_disabled_tests(self):
Probably remove this since it is no longer used anywhere.

https://codereview.chromium.org/2752493002/diff/1/build/android/test_runner.py
File build/android/test_runner.py (right):

https://codereview.chromium.org/2752493002/diff/1/build/android/test_runner.p...
build/android/test_runner.py:870: if unknown_args:
Do we want to do this? Now if you spell an argument incorrectly it will be
passed on to the test binary? Also (this probably wont be a problem) but we have
to worry about collision between test runner arguments and test binary
arguments.

[shenghuazhang, 2017-03-14 05:45:05]

https://codereview.chromium.org/2752493002/diff/1/build/android/pylib/gtest/g...
File build/android/pylib/gtest/gtest_test_instance.py (right):

https://codereview.chromium.org/2752493002/diff/1/build/android/pylib/gtest/g...
build/android/pylib/gtest/gtest_test_instance.py:349: if args.run_disabled:
If we need to append args.run_disabled in _flags, should we also do the same to
_flags in instrumentation_test_instance.py?

https://codereview.chromium.org/2752493002/diff/1/build/android/test_runner.py
File build/android/test_runner.py (right):

https://codereview.chromium.org/2752493002/diff/1/build/android/test_runner.p...
build/android/test_runner.py:80: help='DEPRECATED. Pass such flags directly.')
This version enables deprecated argument '--test-arguments' for instrumentation
tests here, and instrumentation_test_instance.py doesn't handle it (which is
correct I think). Do we need to specify this for gtest in 'help' comment?

[jbudorick, 2017-03-15 18:28:00]

https://codereview.chromium.org/2752493002/diff/1/build/android/pylib/gtest/g...
File build/android/pylib/gtest/gtest_test_instance.py (right):

https://codereview.chromium.org/2752493002/diff/1/build/android/pylib/gtest/g...
build/android/pylib/gtest/gtest_test_instance.py:349: if args.run_disabled:
On 2017/03/14 05:45:05, shenghuazhang wrote:
> If we need to append args.run_disabled in _flags, should we also do the same
to
> _flags in instrumentation_test_instance.py?

The disable mechanism works a bit differently for instrumentation tests -- we
check the tests' annotations against the user-specified ones (or everything but
@DisabledTest and @FlakyTest by default) on the host. Device flags aren't
involved.

https://codereview.chromium.org/2752493002/diff/1/build/android/pylib/gtest/g...
build/android/pylib/gtest/gtest_test_instance.py:350:
self._flags.append('--gtest_also_run_disabled_tests')
On 2017/03/13 23:59:47, mikecase wrote:
> nit: wrong indentation

oops

https://codereview.chromium.org/2752493002/diff/1/build/android/pylib/gtest/g...
build/android/pylib/gtest/gtest_test_instance.py:391: def
gtest_also_run_disabled_tests(self):
On 2017/03/13 23:59:47, mikecase wrote:
> Probably remove this since it is no longer used anywhere.

Done.

https://codereview.chromium.org/2752493002/diff/1/build/android/test_runner.py
File build/android/test_runner.py (right):

https://codereview.chromium.org/2752493002/diff/1/build/android/test_runner.p...
build/android/test_runner.py:80: help='DEPRECATED. Pass such flags directly.')
On 2017/03/14 05:45:05, shenghuazhang wrote:
> This version enables deprecated argument '--test-arguments' for
instrumentation
> tests here, and instrumentation_test_instance.py doesn't handle it (which is
> correct I think). Do we need to specify this for gtest in 'help' comment?

hrm, good point. As this is deprecated, though, I'm removing it from the help
text entirely + making instrumentation tests throw an exception if it's
specified.

https://codereview.chromium.org/2752493002/diff/1/build/android/test_runner.p...
build/android/test_runner.py:870: if unknown_args:
On 2017/03/13 23:59:47, mikecase wrote:
> Do we want to do this? Now if you spell an argument incorrectly it will be
> passed on to the test binary? Also (this probably wont be a problem) but we
have

This is somewhat of a concern, but as I understand that's also how the desktop
test binaries behave. If you don't spell your option correctly, it doesn't
really do anything.

> to worry about collision between test runner arguments and test binary
> arguments.

This is likely to be rare, but in cases where it happens, we can either:
 - pass the flag down to the device in addition to whatever logic we're doing
 - provide equivalent functionality
 - rename the test runner arg

[mikecase (-- gone --), 2017-03-16 16:52:37]

lgtm

[jbudorick, 2017-03-16 17:05:49]

The CQ bit was checked by jbudorick@chromium.org

[commit-bot: I haz the power, 2017-03-16 17:06:41]

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-03-16 17:09:49]

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

[commit-bot: I haz the power, 2017-03-16 17:09:51]

Try jobs failed on following builders:
  ios-device-xcode-clang on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device-xcode-...)
  mac_chromium_compile_dbg_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...)
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[jbudorick, 2017-03-16 17:19:47]

The CQ bit was checked by jbudorick@chromium.org

[jbudorick, 2017-03-16 17:19:47]

The patchset sent to the CQ was uploaded after l-g-t-m from
mikecase@chromium.org
Link to the patchset: https://codereview.chromium.org/2752493002/#ps40001
(title: "rebase")

[commit-bot: I haz the power, 2017-03-16 17:20:40]

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-03-16 17:58:47]

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

[commit-bot: I haz the power, 2017-03-16 17:58:49]

Try jobs failed on following builders:
  android_n5x_swarming_rel on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_n5x_...)

[jbudorick, 2017-03-16 18:05:42]

The CQ bit was checked by jbudorick@chromium.org

[jbudorick, 2017-03-16 18:05:43]

The patchset sent to the CQ was uploaded after l-g-t-m from
mikecase@chromium.org
Link to the patchset: https://codereview.chromium.org/2752493002/#ps60001
(title: "Ensure command_line_flags is always present in the Namespace.")

[commit-bot: I haz the power, 2017-03-16 18:06:16]

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-03-16 19:19:03]

CQ is committing da patch.

Bot data: {"patchset_id": 60001, "attempt_start_ts": 1489687542774530,
"parent_rev": "bb69cdf76b25cee534e6b84a5c55f4453ee3c828", "commit_rev":
"7d30806cf9bcb492d60dab0b3987aba72ff59f87"}

[commit-bot: I haz the power, 2017-03-16 19:19:45]

Description was changed from

==========
[android] Add support for passing command-line flags directly.

Previously, command-line flags to be passed to the test had to be
passed via:
 - (gtests) --test-arguments <string>
 - (instrumentation tests) --device-flags{,-file} <file>

This CL changes it s.t. any flags not recognized by the test runner
are passed down to the binary. It also removes --device-flags and
deprecates --test-arguments.

BUG=700366
==========

to

==========
[android] Add support for passing command-line flags directly.

Previously, command-line flags to be passed to the test had to be
passed via:
 - (gtests) --test-arguments <string>
 - (instrumentation tests) --device-flags{,-file} <file>

This CL changes it s.t. any flags not recognized by the test runner
are passed down to the binary. It also removes --device-flags and
deprecates --test-arguments.

BUG=700366

Review-Url: https://codereview.chromium.org/2752493002
Cr-Commit-Position: refs/heads/master@{#457513}
Committed:
https://chromium.googlesource.com/chromium/src/+/7d30806cf9bcb492d60dab0b3987...
==========

[commit-bot: I haz the power, 2017-03-16 19:19:46]

Committed patchset #4 (id:60001) as
https://chromium.googlesource.com/chromium/src/+/7d30806cf9bcb492d60dab0b3987...