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

build/android/test_runner.py

 #!/usr/bin/env python
 #
 # Copyright 2013 The Chromium Authors. All rights reserved.
 # Use of this source code is governed by a BSD-style license that can be
 # found in the LICENSE file.
 
 """Runs all types of tests from one unified interface."""
 
 import argparse
 import collections
@@ skipping 24 matching lines @@
 from pylib.results import json_results
 from pylib.results import report_results
 
 from py_utils import contextlib_ext
 
 
 _DEVIL_STATIC_CONFIG_FILE = os.path.abspath(os.path.join(
     host_paths.DIR_SOURCE_ROOT, 'build', 'android', 'devil_config.json'))
 
 
-def AddTestLauncherArgs(parser):
+def AddTestLauncherOptions(parser):
   """Adds arguments mirroring //base/test/launcher.
 
   Args:
     parser: The parser to which arguments should be added.
   Returns:
     The given parser.
   """
   parser.add_argument(
       '--test-launcher-retry-limit',
       '--test_launcher_retry_limit',
       '--num_retries', '--num-retries',
       dest='num_retries', type=int, default=2,
       help='Number of retries for a test before '
            'giving up (default: %(default)s).')
   parser.add_argument(
       '--test-launcher-summary-output',
       '--json-results-file',
       dest='json_results_file', type=os.path.realpath,
       help='If set, will dump results in JSON form '
            'to specified file.')
 
   return parser
 
 
+def AddCommandLineOptions(parser):
+  """Adds arguments to support passing command-line flags to the device."""
+  parser.add_argument(
+      '--device-flags-file',
+      type=os.path.realpath,
+      help='The relative filepath to a file containing '
+           'command-line flags to set on the device')
+  parser.add_argument(
+      '-a', '--test-arguments',
+      dest='test_arguments', default='',
+      help='DEPRECATED. Pass such flags directly.')

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

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]

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.

+  parser.set_defaults(allow_unknown=True)
+
+
 def AddTracingOptions(parser):
   # TODO(shenghuazhang): Move this into AddCommonOptions once it's supported
   # for all test types.
   parser.add_argument(
       '--trace-output',
       metavar='FILENAME', type=os.path.realpath,
       help='Path to save test_runner trace data to.')
 
 
 def AddCommonOptions(parser):
@@ skipping 65 matching lines @@
            ' precedence over --debug and --release')
   parser.add_argument(
       '--repeat', '--gtest_repeat', '--gtest-repeat',
       dest='repeat', type=int, default=0,
       help='Number of times to repeat the specified set of tests.')
   parser.add_argument(
       '-v', '--verbose',
       dest='verbose_count', default=0, action='count',
       help='Verbose level (multiple times for more)')
 
-  AddTestLauncherArgs(parser)
+  AddTestLauncherOptions(parser)
 
 
 def ProcessCommonOptions(args):
   """Processes and handles all common options."""
   run_tests_helper.SetLogLevel(args.verbose_count)
   constants.SetBuildType(args.build_type)
   if args.output_directory:
     constants.SetOutputDirectory(args.output_directory)
 
   # Some things such as Forwarder require ADB to be in the environment path.
@@ skipping 109 matching lines @@
       dest='store_tombstones', action='store_true',
       help='Add tombstones in results if crash.')
   parser.add_argument(
       '-s', '--suite',
       dest='suite_name', nargs='+', metavar='SUITE_NAME', required=True,
       help='Executable name of the test suite to run.')
   parser.add_argument(
       '--test-apk-incremental-install-script',
       type=os.path.realpath,
       help='Path to install script for the test apk.')
-  parser.add_argument(
-      '-a', '--test-arguments',
-      dest='test_arguments', default='',
-      help='Additional arguments to pass to the test.')
 
   filter_group = parser.add_mutually_exclusive_group()
   filter_group.add_argument(
       '-f', '--gtest_filter', '--gtest-filter',
       dest='test_filter',
       help='googletest-style filter string.')
   filter_group.add_argument(
       '--gtest-filter-file',
       dest='test_filter_file', type=os.path.realpath,
       help='Path to file that contains googletest-style filter strings. '
@@ skipping 26 matching lines @@
   parser.add_argument(
       '--coverage-dir',
       type=os.path.realpath,
       help='Directory in which to place all generated '
            'EMMA coverage files.')
   parser.add_argument(
       '--delete-stale-data',
       action='store_true', dest='delete_stale_data',
       help='Delete stale test data on the device.')
   parser.add_argument(
-      '--device-flags',
-      dest='device_flags',
-      type=os.path.realpath,
-      help='The relative filepath to a file containing '
-           'command-line flags to set on the device')
-  parser.add_argument(
-      '--device-flags-file',
-      type=os.path.realpath,
-      help='The relative filepath to a file containing '
-           'command-line flags to set on the device')
-  parser.add_argument(
       '--disable-dalvik-asserts',
       dest='set_asserts', action='store_false', default=True,
       help='Removes the dalvik.vm.enableassertions property')
   parser.add_argument(
       '-E', '--exclude-annotation',
       dest='exclude_annotation_str',
       help='Comma-separated list of annotations. Exclude tests with these '
            'annotations.')
   parser.add_argument(
       '-f', '--test-filter', '--gtest_filter', '--gtest-filter',
@@ skipping 459 matching lines @@
   command_parsers = parser.add_subparsers(
       title='test types', dest='command')
 
   subp = command_parsers.add_parser(
       'gtest',
       help='googletest-based C++ tests')
   AddCommonOptions(subp)
   AddDeviceOptions(subp)
   AddGTestOptions(subp)
   AddTracingOptions(subp)
+  AddCommandLineOptions(subp)
 
   subp = command_parsers.add_parser(
       'instrumentation',
       help='InstrumentationTestCase-based Java tests')
   AddCommonOptions(subp)
   AddDeviceOptions(subp)
   AddInstrumentationTestOptions(subp)
   AddTracingOptions(subp)
+  AddCommandLineOptions(subp)
 
   subp = command_parsers.add_parser(
       'junit',
       help='JUnit4-based Java tests')
   AddCommonOptions(subp)
   AddJUnitTestOptions(subp)
 
   subp = command_parsers.add_parser(
       'linker',
       help='linker tests')
@@ skipping 15 matching lines @@
   AddDeviceOptions(subp)
   AddPerfTestOptions(subp)
   AddTracingOptions(subp)
 
   subp = command_parsers.add_parser(
       'python',
       help='python tests based on unittest.TestCase')
   AddCommonOptions(subp)
   AddPythonTestOptions(subp)
 
-  args = parser.parse_args()
+  args, unknown_args = parser.parse_known_args()
+  if unknown_args:

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

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.

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

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.
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

+    if hasattr(args, 'allow_unknown') and args.allow_unknown:
+      args.command_line_flags = unknown_args
+    else:
+      parser.error('unrecognized arguments: %s' % ' '.join(unknown_args))
 
   try:
     return RunTestsCommand(args)
   except base_error.BaseError as e:
     logging.exception('Error occurred.')
     if e.is_infra_error:
       return constants.INFRA_EXIT_CODE
     return constants.ERROR_EXIT_CODE
   except: # pylint: disable=W0702
     logging.exception('Unrecognized error occurred.')
     return constants.ERROR_EXIT_CODE
 
 
 if __name__ == '__main__':
   sys.exit(main())