Partial revert of flag simplification.

Unfortunately it turns out the Android test runner requires
the isolated script flag to be in its current form, or it
doesn't work. This means we have to keep translating the
flag name.

We can get rid of the isolated_script_test_output flag
at least.

Tbr: mbonadei@webrtc.org
Bug: chromium:1051927
Change-Id: I4fdbff980e65332b757b1c95aa6587328411c0ed
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/171809
Reviewed-by: Patrik Höglund <phoglund@webrtc.org>
Commit-Queue: Patrik Höglund <phoglund@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#30907}

audio/test/low_bandwidth_audio_test.py

@@ -56,7 +56,7 @@ def _ParseArgs():
   parser.add_argument('--adb-path', help='Path to adb binary.', default='adb')
   parser.add_argument('--num-retries', default='0',
                       help='Number of times to retry the test on Android.')
-  parser.add_argument('--isolated_script_test_perf_output', default=None,
+  parser.add_argument('--isolated-script-test-perf-output', default=None,
       help='Path to store perf results in histogram proto format.')
   parser.add_argument('--extra-test-args', default=[], action='append',
       help='Extra args to path to the test binary.')

tools_webrtc/flags_compatibility.py

@@ -0,0 +1,44 @@
+#!/usr/bin/env python
+
+# Copyright (c) 2019 The WebRTC project authors. All Rights Reserved.
+#
+# Use of this source code is governed by a BSD-style license
+# that can be found in the LICENSE file in the root of the source
+# tree. An additional intellectual property rights grant can be found
+# in the file PATENTS.  All contributing project authors may
+# be found in the AUTHORS file in the root of the source tree.
+
+import argparse
+import logging
+import subprocess
+import sys
+
+
+def main():
+  parser = argparse.ArgumentParser()
+  parser.add_argument('--isolated-script-test-perf-output')
+  args, unrecognized_args = parser.parse_known_args()
+
+  test_command = _ForcePythonInterpreter(unrecognized_args)
+  if args.isolated_script_test_perf_output:
+    test_command += ['--isolated_script_test_perf_output',
+                     args.isolated_script_test_perf_output]
+  logging.info('Running %r', test_command)
+
+  return subprocess.call(test_command)
+
+
+def _ForcePythonInterpreter(cmd):
+  """Returns the fixed command line to call the right python executable."""
+  out = cmd[:]
+  if out[0] == 'python':
+    out[0] = sys.executable
+  elif out[0].endswith('.py'):
+    out.insert(0, sys.executable)
+  return out
+
+
+if __name__ == '__main__':
+  # pylint: disable=W0101
+  logging.basicConfig(level=logging.INFO)
+  sys.exit(main())

tools_webrtc/gtest-parallel-wrapper.py

@@ -15,18 +15,25 @@ gtest-parallel, renaming options and translating environment variables into
 flags. Developers should execute gtest-parallel directly.
 
 In particular, this translates the GTEST_SHARD_INDEX and GTEST_TOTAL_SHARDS
-environment variables to the --shard_index and --shard_count flags, and
-interprets e.g. --workers=2x as 2 workers per core.
+environment variables to the --shard_index and --shard_count flags
+and interprets e.g. --workers=2x as 2 workers per core.
 
 Flags before '--' will be attempted to be understood as arguments to
 gtest-parallel. If gtest-parallel doesn't recognize the flag or the flag is
 after '--', the flag will be passed on to the test executable.
 
+--isolated-script-test-perf-output is renamed to
+--isolated_script_test_perf_output. The Android test runner needs the flag to
+be in the former form, but our tests require the latter, so this is the only
+place we can do it.
+
 If the --store-test-artifacts flag is set, an --output_dir must be also
 specified.
+
 The test artifacts will then be stored in a 'test_artifacts' subdirectory of the
 output dir, and will be compressed into a zip file once the test finishes
 executing.
+
 This is useful when running the tests in swarming, since the output directory
 is not known beforehand.
 
@@ -37,6 +44,7 @@ For example:
       --another_flag \
       --output_dir=SOME_OUTPUT_DIR \
       --store-test-artifacts
+      --isolated-script-test-perf-output=SOME_OTHER_DIR \
       -- \
       --foo=bar \
       --baz
@@ -47,11 +55,13 @@ Will be converted into:
       --shard_index 0 \
       --shard_count 1 \
       --output_dir=SOME_OUTPUT_DIR \
+      --dump_json_test_results=SOME_DIR \
       some_test \
       -- \
       --test_artifacts_dir=SOME_OUTPUT_DIR/test_artifacts \
       --some_flag=some_value \
       --another_flag \
+      --isolated_script_test_perf_output=SOME_OTHER_DIR \
       --foo=bar \
       --baz
 
@@ -146,8 +156,16 @@ def ParseArgs(argv=None):
 
   options, unrecognized_args = parser.parse_known_args(argv)
 
-  # Just pass on flags we don't recognize to the test binary.
-  executable_args = options.executable_args + unrecognized_args
+  args_to_pass = []
+  for arg in unrecognized_args:
+    if arg.startswith('--isolated-script-test-perf-output'):
+      arg_split = arg.split('=')
+      assert len(arg_split) == 2, 'You must use the = syntax for this flag.'
+      args_to_pass.append('--isolated_script_test_perf_output=' + arg_split[1])
+    else:
+      args_to_pass.append(arg)
+
+  executable_args = options.executable_args + args_to_pass
 
   if options.store_test_artifacts:
     assert options.output_dir, (

tools_webrtc/gtest_parallel_wrapper_test.py

@@ -133,7 +133,7 @@ class GtestParallelWrapperTest(unittest.TestCase):
       result = gtest_parallel_wrapper.ParseArgs([
           'some_test', '--some_flag=some_value', '--another_flag',
           '--output_dir=' + output_dir, '--store-test-artifacts',
-          '--isolated_script_test_perf_output=SOME_OTHER_DIR', '--foo=bar',
+          '--isolated-script-test-perf-output=SOME_OTHER_DIR', '--foo=bar',
           '--baz'
       ])
       expected_artifacts_dir = os.path.join(output_dir, 'test_artifacts')

tools_webrtc/mb/mb.py

@@ -855,6 +855,10 @@ class MetaBuildWrapper(object):
                   '--logcat-output-file', '${ISOLATED_OUTDIR}/logcats',
                   '--store-tombstones']
     else:
+      if test_type == 'raw':
+        cmdline.append('../../tools_webrtc/flags_compatibility.py')
+        extra_files.append('../../tools_webrtc/flags_compatibility.py')
+
       if isolate_map[target].get('use_webcam', False):
         cmdline.append('../../tools_webrtc/ensure_webcam_is_running.py')
         extra_files.append('../../tools_webrtc/ensure_webcam_is_running.py')

tools_webrtc/mb/mb_unittest.py

@@ -453,9 +453,11 @@ class UnitTest(unittest.TestCase):
     self.assertEqual(files, [
         '../../.vpython',
         '../../testing/test_env.py',
+        '../../tools_webrtc/flags_compatibility.py',
         'base_unittests',
     ])
     self.assertEqual(command, [
+        '../../tools_webrtc/flags_compatibility.py',
         '../../testing/test_env.py',
         './base_unittests',
         '--asan=0',