Don't duplicate gtest-parallel flags in gtest-parallel-wrappers.

tools-webrtc/gtest-parallel-wrapper.py

Index: tools-webrtc/gtest-parallel-wrapper.py
diff --git a/tools-webrtc/gtest-parallel-wrapper.py b/tools-webrtc/gtest-parallel-wrapper.py
index 5009e20c9a723bf3f3eed1bf9cc7d67c35f3768f..300cabb522003cb0e0f5db040e408dab1de409fb 100755
--- a/tools-webrtc/gtest-parallel-wrapper.py
+++ b/tools-webrtc/gtest-parallel-wrapper.py
@@ -18,24 +18,35 @@ In particular, this translates the GTEST_SHARD_INDEX and GTEST_TOTAL_SHARDS
 environment variables to the --shard_index and --shard_count flags, and renames
 the --isolated-script-test-output flag to --dump_json_test_results.
 
-Note that the flags unprocessed by this script will passed as arguments to the
-test executable, i.e.
+All flags before '--' will be passed as arguments to gtest_parallel, and

[kjellander_webrtc, 2017/05/04 11:50:09]

nit: gtest_parallel -> gtest-parallel 

+(almost) all flags after '--' will be passed as arguments to the test
+executable.
+The exception is that --isolated-script-test-output and
+--isolated-script-test-chartson-output are expected to be after '--', so they
+are processed and removed from there.
+For example:
 
   gtest-parallel-wrapper.py some_test \
-      --isolated-script-test-output=some_dir \
-      --unprocessed_arg_1
+      --some_flag=some_value \
+      --another_flag \
       -- \
-      --unprocessed_arg_2
+      --isolated-script-test-output=some_dir \
+      --isolated-script-test-chartson-output=some_other_dir \

[kjellander_webrtc, 2017/05/04 11:50:10]

chartson -> chartjson

+      --foo=bar \
+      --baz
 
-will be converted into
+Will be converted into:
 
   python gtest-parallel some_test \
       --shard_count 1 \
       --shard_index 0 \
-      --dump_json_test_results some_dir \
+      --some_flag=some_value \
+      --another_flag \
+      --dump_json_test_results=some_dir \
       -- \
-      --unprocessed_arg_1
-      --unprocessed_arg_2
+      --foo=bar
+      --baz
+
 """
 
 import argparse
@@ -52,11 +63,14 @@ def CatFiles(file_list, output_file):
       os.remove(filename)
 
 
-def main():
-  # Ignore '--'. Options unprocessed by this script will be passed to the test
-  # as arguments.
-  if '--' in sys.argv:
-    del sys.argv[sys.argv.index('--')]
+def get_args_and_env():
+  if '--' not in sys.argv:
+    return sys.argv, os.environ
+
+  argv_index = sys.argv.index('--')
+
+  gtest_parallel_args = sys.argv[1:argv_index]
+  executable_args = sys.argv[argv_index + 1:]
 
   parser = argparse.ArgumentParser()
   parser.add_argument('--isolated-script-test-output', type=str, default=None)
@@ -67,11 +81,19 @@ def main():
   parser.add_argument('--isolated-script-test-chartjson-output', type=str,
                       default=None)
 
-  # TODO(ehmaldonado): Figure out a way to avoid duplicating the flags in
-  # gtest-parallel.
-  parser.add_argument('--gtest_color', type=str, default='auto')
-  parser.add_argument('--output_dir', type=str, default=None)
-  parser.add_argument('--timeout', type=int, default=None)
+  # We have to do this, since --isolated-script-test-output is passed as an
+  # argument to the executable by the swarming scripts, and we want to pass it
+  # to gtest-parallel instead.
+  options, executable_args = parser.parse_known_args(executable_args)
+
+  # --isolated-script-test-output is used to upload results to the flakiness
+  # dashboard. This translation is made because gtest-parallel expects the flag
+  # to be called --dump_json_test_results instead.
+  if options.isolated_script_test_output:
+    gtest_parallel_args += [
+        '--dump_json_test_results',
+        options.isolated_script_test_output,
+    ]
 
   # GTEST_SHARD_INDEX and GTEST_TOTAL_SHARDS must be removed from the
   # environment. Otherwise it will be picked up by the binary, causing a bug
@@ -80,62 +102,48 @@ def main():
   gtest_shard_index = test_env.pop('GTEST_SHARD_INDEX', '0')
   gtest_total_shards = test_env.pop('GTEST_TOTAL_SHARDS', '1')
 
-  webrtc_root = os.path.dirname(os.path.dirname(os.path.abspath(__file__)))
-  gtest_parallel_path = os.path.join(
-      webrtc_root, 'third_party', 'gtest-parallel', 'gtest-parallel')
-
-  options, unprocessed = parser.parse_known_args()
-  test_executable = unprocessed[0]
-  test_arguments = unprocessed[1:]
-
-  gtest_args = [
-      test_executable,
+  gtest_parallel_args += [
       '--shard_count',
       gtest_total_shards,
       '--shard_index',
       gtest_shard_index,
-      '--gtest_color',
-      options.gtest_color,
-  ]
+  ] + ['--'] + executable_args
 
-  # --isolated-script-test-output is used to upload results to the flakiness
-  # dashboard. This translation is made because gtest-parallel expects the flag
-  # to be called --dump_json_test_results instead.
-  if options.isolated_script_test_output:
-    gtest_args += [
-        '--dump_json_test_results',
-        options.isolated_script_test_output,
-    ]
+  return gtest_parallel_args, test_env
 
-  if options.output_dir:
-    gtest_args += [
-        '--output_dir',
-        options.output_dir,
-    ]
 
-  if options.timeout:
-    gtest_args += [
-        '--timeout',
-        str(options.timeout),
-    ]
+def get_output_dir(gtest_parallel_args):
+  parser = argparse.ArgumentParser()
+  parser.add_argument('--output_dir', type=str, default=None)
+  options, _ = parser.parse_known_args(gtest_parallel_args)
+  return options.output_dir
+
+
+def main():
+  webrtc_root = os.path.dirname(os.path.dirname(os.path.abspath(__file__)))
+  gtest_parallel_path = os.path.join(
+      webrtc_root, 'third_party', 'gtest-parallel', 'gtest-parallel')
+
+  gtest_parallel_args, test_env = get_args_and_env()
 
   command = [
       sys.executable,
       gtest_parallel_path,
-  ] + gtest_args + ['--'] + test_arguments
+  ] + gtest_parallel_args
 
   print 'gtest-parallel-wrapper: Executing command %s' % ' '.join(command)
   sys.stdout.flush()
 
   exit_code = subprocess.call(command, env=test_env, cwd=os.getcwd())
 
-  if options.output_dir:
+  output_dir = get_output_dir(gtest_parallel_args)
+  if output_dir:
     for test_status in 'passed', 'failed', 'interrupted':
-      logs_dir = os.path.join(options.output_dir, test_status)
+      logs_dir = os.path.join(output_dir, test_status)
       if not os.path.isdir(logs_dir):
         continue
       logs = [os.path.join(logs_dir, log) for log in os.listdir(logs_dir)]
-      log_file = os.path.join(options.output_dir, '%s-tests.log' % test_status)
+      log_file = os.path.join(output_dir, '%s-tests.log' % test_status)
       CatFiles(logs, log_file)
       os.rmdir(logs_dir)