[testing/scripts] Add base adapter class that enables more code reuse for all isolated script test

BUG: 894261
Change-Id: Id707ba8a8782a23a41a7dc4680cbc2538e1d3df1
Reviewed-on: https://chromium-review.googlesource.com/c/1278156Reviewed-by: David Trainor <dtrainor@chromium.org>
Reviewed-by: Kenneth Russell <kbr@chromium.org>
Commit-Queue: Ned Nguyen <nednguyen@google.com>
Cr-Commit-Position: refs/heads/master@{#599669}

chrome/android/monochrome/BUILD.gn

@@ -31,5 +31,6 @@ group("monochrome_apk_checker") {
     "./scripts/monochrome_apk_checker.py",
     "//testing/scripts/monochrome_apk_checker_wrapper.py",
     "//testing/scripts/common.py",
+    "//testing/xvfb.py",
   ]
 }

testing/scripts/common.py

@@ -12,6 +12,17 @@ import subprocess
 import sys
 import tempfile
 import time
+import traceback
+
+# Add src/testing/ into sys.path for importing xvfb.
+sys.path.append(os.path.join(os.path.dirname(__file__), '..'))
+import xvfb
+
+# Unfortunately we need to copy these variables from ../test_env.py.
+# Importing it and using its get_sandbox_env breaks test runs on Linux
+# (it seems to unset DISPLAY).
+CHROME_SANDBOX_ENV = 'CHROME_DEVEL_SANDBOX'
+CHROME_SANDBOX_PATH = '/opt/chromium/chrome_sandbox'
 
 
 SCRIPT_DIR = os.path.abspath(os.path.dirname(__file__))
@@ -199,3 +210,157 @@ def run_integration_test(script_to_run, extra_args, log_file, output):
   }, output)
 
   return integration_test_res
+
+
+class BaseIsolatedScriptArgsAdapter(object):
+  """The base class for all script adapters that need to translate flags
+  set by isolated script test contract into the specific test script's flags.
+  """
+
+  def __init__(self):
+    self._parser = argparse.ArgumentParser()
+    self._options = None
+    self._rest_args = None
+    self._parser.add_argument(
+        '--isolated-script-test-output', type=str,
+        required=True)
+    self._parser.add_argument(
+        '--isolated-script-test-filter', type=str,
+        required=False)
+    self._parser.add_argument(
+        '--isolated-script-test-repeat', type=str,
+        required=False)
+    self._parser.add_argument(
+        '--isolated-script-test-launcher-retry-limit', type=str,
+        required=False)
+    self._parser.add_argument(
+        '--isolated-script-test-also-run-disabled-tests',
+        default=False, action='store_true', required=False)
+
+    self._parser.add_argument('--xvfb', help='start xvfb', action='store_true')
+
+    # This argument is ignored for now.
+    self._parser.add_argument(
+        '--isolated-script-test-chartjson-output', type=str)
+    # This argument is ignored for now.
+    self._parser.add_argument('--isolated-script-test-perf-output', type=str)
+
+    self.add_extra_arguments(self._parser)
+
+  def add_extra_arguments(self, parser):
+    pass
+
+  def parse_args(self, args=None):
+    self._options, self._rest_args = self._parser.parse_known_args(args)
+
+  @property
+  def parser(self):
+    return self._parser
+
+  @property
+  def options(self):
+    return self._options
+
+  @property
+  def rest_args(self):
+    return self._rest_args
+
+  def generate_test_output_args(self, output):
+    del output  # unused
+    raise RuntimeError('this method is not yet implemented')
+
+  def generate_test_filter_args(self, test_filter_str):
+    del test_filter_str  # unused
+    raise RuntimeError('this method is not yet implemented')
+
+  def generate_test_repeat_args(self, repeat_count):
+    del repeat_count  # unused
+    raise RuntimeError('this method is not yet implemented')
+
+  def generate_test_launcher_retry_limit_args(self, retry_limit):
+    del retry_limit  # unused
+    raise RuntimeError('this method is not yet implemented')
+
+  def generate_test_also_run_disabled_tests_args(self, also_run_disabled_tests):
+    del also_run_disabled_tests  # unused
+    raise RuntimeError('this method is not yet implemented')
+
+  def generate_sharding_args(self, total_shard, shard_index):
+    del total_shard, shard_index  # unused
+    raise RuntimeError('this method is not yet implemented')
+
+  def generate_isolated_script_cmd(self):
+    isolated_script_cmd = [sys.executable] + self._rest_args
+
+    isolated_script_cmd += self.generate_test_output_args(
+        self.options.isolated_script_test_output)
+
+    # Augment test filter args if needed
+    if self.options.isolated_script_test_filter:
+      isolated_script_cmd += self.generate_test_filter_args(
+          self.options.isolated_script_test_filter)
+
+    # Augment test repeat if needed
+    if self.options.isolated_script_test_repeat:
+      isolated_script_cmd += self.generate_test_repeat_args(
+          self.options.isolated_script_test_repeat)
+
+    # Augment test launcher retry limit args if needed
+    if self.options.isolated_script_test_launcher_retry_limit:
+      isolated_script_cmd += self.generate_test_launcher_retry_limit_args(
+          self.options.isolated_script_test_launcher_retry_limit)
+
+    # Augment test also run disable tests args if needed
+    if self.options.isolated_script_test_also_run_disabled_tests:
+      isolated_script_cmd += self.generate_test_also_run_disabled_tests_args(
+          self.options.isolated_script_test_also_run_disabled_tests)
+
+    # Augment shard args if needed
+    env = os.environ.copy()
+
+    total_shards = None
+    shard_index = None
+
+    if 'GTEST_TOTAL_SHARDS' in env:
+      total_shards = int(env['GTEST_TOTAL_SHARDS'])
+    if 'GTEST_SHARD_INDEX' in env:
+      shard_index = int(env['GTEST_SHARD_INDEX'])
+    if total_shards is not None and shard_index is not None:
+      isolated_script_cmd += self.generate_sharding_args(
+          total_shards, shard_index)
+
+    return isolated_script_cmd
+
+  def run_test(self):
+    self.parse_args()
+    cmd = self.generate_isolated_script_cmd()
+
+    env = os.environ.copy()
+
+    # Assume we want to set up the sandbox environment variables all the
+    # time; doing so is harmless on non-Linux platforms and is needed
+    # all the time on Linux.
+    env[CHROME_SANDBOX_ENV] = CHROME_SANDBOX_PATH
+    valid = True
+    rc = 0
+    try:
+      env['CHROME_HEADLESS'] = '1'
+      if self.options.xvfb:
+        return xvfb.run_executable(cmd, env)
+      else:
+         return run_command(cmd, env=env)
+
+    except Exception:
+      rc = 1
+      traceback.print_exc()
+      valid = False
+
+    if not valid:
+      failures = ['(entire test suite)']
+      with open(self.options.isolated_script_test_output, 'w') as fp:
+        json.dump({
+            'valid': valid,
+            'failures': failures,
+        }, fp)
+
+    return rc

testing/scripts/run_chromedriver_tests.py

@@ -32,34 +32,24 @@ import traceback
 
 import common
 
-def main():
-  parser = argparse.ArgumentParser()
-
-  # --isolated-script-test-output is passed through to the script.
 
-  # This argument is ignored for now.
-  parser.add_argument('--isolated-script-test-chartjson-output', type=str)
-  # This argument is ignored for now.
-  parser.add_argument('--isolated-script-test-perf-output', type=str)
-  # This argument is translated below.
-  parser.add_argument('--isolated-script-test-filter', type=str)
+class ChromeDriverAdapter(common.BaseIsolatedScriptArgsAdapter):
 
-  args, rest_args = parser.parse_known_args()
+  def generate_test_output_args(self, output):
+    return ['--isolated-script-test-output', output]
 
-  filtered_tests = args.isolated_script_test_filter
-  if filtered_tests:
-    if any('--filter' in arg for arg in rest_args):
-      parser.error(
+  def generate_test_filter_args(self, test_filter_str):
+    if any('--filter' in arg for arg in self.rest_args):
+      self.parser.error(
           'can\'t have the test call filter with the'
           '--isolated-script-test-filter argument to the wrapper script')
 
-    # https://github.com/google/googletest/blob/master/googletest/docs/advanced.md#running-a-subset-of-the-tests
-    # says that the gtest filter should accept single colons separating
-    # individual tests. The input is double colon separated, so translate it.
-    rest_args = rest_args + ['--filter', filtered_tests.replace('::', ':')]
+    return ['--filter', test_filter_str.replace('::', ':')]
 
-  cmd = [sys.executable] + rest_args
-  return common.run_command(cmd)
+
+def main():
+  adapter = ChromeDriverAdapter()
+  adapter.run_test()
 
 
 # This is not really a "script test" so does not need to manually add

testing/scripts/run_gpu_integration_test_as_googletest.py

@@ -27,7 +27,6 @@ followed by a subsequent Python script. It could be generalized to
 invoke an arbitrary executable.
 """
 
-import argparse
 import json
 import os
 import shutil
@@ -37,101 +36,18 @@ import traceback
 
 import common
 
-# Add src/testing/ into sys.path for importing xvfb.
-sys.path.append(os.path.join(os.path.dirname(__file__), '..'))
-import xvfb
+class GpuIntegrationTestAdapater(common.BaseIsolatedScriptArgsAdapter):
 
-# Unfortunately we need to copy these variables from ../test_env.py.
-# Importing it and using its get_sandbox_env breaks test runs on Linux
-# (it seems to unset DISPLAY).
-CHROME_SANDBOX_ENV = 'CHROME_DEVEL_SANDBOX'
-CHROME_SANDBOX_PATH = '/opt/chromium/chrome_sandbox'
+  def generate_test_output_args(self, output):
+    return ['--write-full-results-to', output]
+
+  def generate_sharding_args(self, total_shards, shard_index):
+    return ['--total-shards=%d' % total_shards,
+            '--shard-index=%d' % shard_index]
 
 def main():
-  parser = argparse.ArgumentParser()
-  parser.add_argument(
-      '--isolated-script-test-output', type=str,
-      required=True)
-  parser.add_argument(
-      '--isolated-script-test-filter', type=str,
-      required=False)
-  parser.add_argument('--xvfb', help='Start xvfb.', action='store_true')
-  args, rest_args = parser.parse_known_args()
-  # Remove the chartjson extra arg until this script cares about chartjson
-  # results from telemetry
-  index = 0
-  for arg in rest_args:
-    if ('--isolated-script-test-chartjson-output' in arg or
-        '--isolated-script-test-perf-output' in arg):
-      rest_args.pop(index)
-      break
-    index += 1
-  if args.isolated_script_test_filter:
-    filter_list = common.extract_filter_list(args.isolated_script_test_filter)
-
-    # isolated_script_test_filter comes in like:
-    #   gpu_tests.webgl_conformance_integration_test.WebGLConformanceIntegrationTest.WebglExtension_WEBGL_depth_texture  # pylint: disable=line-too-long
-    # but we need to pass it to --test-filter like this:
-    #   WebglExtension_WEBGL_depth_texture
-    filter_list = [f.split('.')[-1] for f in filter_list]
-
-    # Need to convert this to a valid regex.
-    filter_regex = '(' + '|'.join(filter_list) + ')'
-    rest_args.append('--test-filter=' + filter_regex)
-
-  xvfb_proc = None
-  openbox_proc = None
-  xcompmgr_proc = None
-  env = os.environ.copy()
-  # Assume we want to set up the sandbox environment variables all the
-  # time; doing so is harmless on non-Linux platforms and is needed
-  # all the time on Linux.
-  env[CHROME_SANDBOX_ENV] = CHROME_SANDBOX_PATH
-  if args.xvfb and xvfb.should_start_xvfb(env):
-    xvfb_proc, openbox_proc, xcompmgr_proc = xvfb.start_xvfb(env=env,
-                                                             build_dir='.')
-    assert xvfb_proc and openbox_proc and xcompmgr_proc, 'Failed to start xvfb'
-  # Compatibility with gtest-based sharding.
-  total_shards = None
-  shard_index = None
-  if 'GTEST_TOTAL_SHARDS' in env:
-    total_shards = int(env['GTEST_TOTAL_SHARDS'])
-    del env['GTEST_TOTAL_SHARDS']
-  if 'GTEST_SHARD_INDEX' in env:
-    shard_index = int(env['GTEST_SHARD_INDEX'])
-    del env['GTEST_SHARD_INDEX']
-  sharding_args = []
-  if total_shards is not None and shard_index is not None:
-    sharding_args = [
-      '--total-shards=%d' % total_shards,
-      '--shard-index=%d' % shard_index
-    ]
-  try:
-    valid = True
-    rc = 0
-    try:
-      env['CHROME_HEADLESS'] = '1'
-      rc = common.run_command([sys.executable] + rest_args + sharding_args + [
-        '--write-full-results-to', args.isolated_script_test_output
-      ], env=env)
-    except Exception:
-      traceback.print_exc()
-      valid = False
-
-    if not valid:
-      failures = ['(entire test suite)']
-      with open(args.isolated_script_test_output, 'w') as fp:
-        json.dump({
-            'valid': valid,
-            'failures': failures,
-        }, fp)
-
-    return rc
-
-  finally:
-    xvfb.kill(xvfb_proc)
-    xvfb.kill(openbox_proc)
-    xvfb.kill(xcompmgr_proc)
+  adapter = GpuIntegrationTestAdapater()
+  return adapter.run_test()
 
 
 # This is not really a "script test" so does not need to manually add

testing/scripts/run_isolated_script_test.py

@@ -49,52 +49,43 @@ import xvfb
 # Known typ test runners this script wraps. They need a different argument name
 # when selecting which tests to run.
 # TODO(dpranke): Detect if the wrapped test suite uses typ better.
-KNOWN_TYP_TEST_RUNNERS = ['run_blinkpy_tests.py', 'metrics_python_tests.py']
+KNOWN_TYP_TEST_RUNNERS = {'run_blinkpy_tests.py', 'metrics_python_tests.py'}
 
 
-def main():
-  parser = argparse.ArgumentParser()
-  parser.add_argument('--isolated-script-test-output', type=str,
-                      required=True)
-  parser.add_argument('--xvfb', help='start xvfb', action='store_true')
-
-  # This argument is ignored for now.
-  parser.add_argument('--isolated-script-test-chartjson-output', type=str)
-  # This argument is ignored for now.
-  parser.add_argument('--isolated-script-test-perf-output', type=str)
-  # This argument is translated below.
-  parser.add_argument('--isolated-script-test-filter', type=str)
-
-  args, rest_args = parser.parse_known_args()
-
-  env = os.environ.copy()
-  env['CHROME_HEADLESS'] = '1'
-  cmd = [sys.executable] + rest_args
-  cmd += ['--write-full-results-to', args.isolated_script_test_output]
-  temp_filter_file = None
-  try:
-    if args.isolated_script_test_filter:
-      filter_list = common.extract_filter_list(args.isolated_script_test_filter)
-      # Need to dump this to a file in order to use --file-list.
-      temp_filter_file = tempfile.NamedTemporaryFile(mode='w', delete=False)
-      temp_filter_file.write('\n'.join(filter_list))
-      temp_filter_file.close()
-
-      arg_name = 'test-list'
-      for arg in rest_args:
-        for runner in KNOWN_TYP_TEST_RUNNERS:
-          if runner in arg:
-            arg_name = 'file-list'
-
-      cmd += ['--%s=' % arg_name + temp_filter_file.name]
-    if args.xvfb:
-      return xvfb.run_executable(cmd, env)
-    else:
-      return common.run_command(cmd, env=env)
-  finally:
-    if temp_filter_file:
-      os.unlink(temp_filter_file.name)
+class TypUnittestAdapter(common.BaseIsolatedScriptArgsAdapter):
+
+  def __init__(self):
+    super(TypUnittestAdapter, self).__init__()
+    self._temp_filter_file = None
+
+  def generate_sharding_args(self, total_shards, shard_index):
+    # This script only uses environment variable for sharding.
+    del total_shards, shard_index  # unused
+    return []
+
+  def generate_test_output_args(self, output):
+    return ['--write-full-results-to', output]
 
+  def generate_test_filter_args(self, test_filter_str):
+    filter_list = common.extract_filter_list(test_filter_str)
+    self._temp_filter_file.write('\n'.join(filter_list))
+    self._temp_filter_file.close()
+    arg_name = 'test-list'
+    if KNOWN_TYP_TEST_RUNNERS.intersection(self.rest_args):
+      arg_name = 'file-list'
+
+    return ['--%s=' % arg_name + self._temp_filter_file]
+
+  def run_test(self):
+    self._temp_filter_file = tempfile.NamedTemporaryFile(mode='w', delete=False)
+    try:
+      super(TypUnittestAdapter, self).run_test()
+    finally:
+      os.unlink(self._temp_filter_file.name)
+
+def main():
+  adapter = TypUnittestAdapter()
+  adapter.run_test()
 
 # This is not really a "script test" so does not need to manually add
 # any additional compile targets.

testing/scripts/run_telemetry_as_googletest.py

@@ -35,57 +35,23 @@ import sys
 
 import common
 
-# Add src/testing/ into sys.path for importing xvfb.
-sys.path.append(os.path.join(os.path.dirname(__file__), '..'))
-import xvfb
+
+class TelemetryUnittestAdapter(common.BaseIsolatedScriptArgsAdapter):
+
+  def generate_test_output_args(self, output):
+    return ['--write-full-results-to', output]
+
+  def generate_test_filter_args(self, test_filter_str):
+    return ['--test-filter', test_filter_str]
+
+  def generate_sharding_args(self, total_shards, shard_index):
+    return ['--total-shards=%d' % total_shards,
+            '--shard-index=%d' % shard_index]
 
 
 def main():
-  parser = argparse.ArgumentParser()
-  parser.add_argument(
-      '--isolated-script-test-output', type=str,
-      required=True)
-  parser.add_argument(
-      '--isolated-script-test-filter', type=str,
-      required=False)
-  parser.add_argument('--xvfb', help='Start xvfb.', action='store_true')
-  args, rest_args = parser.parse_known_args()
-  # Remove the chartjson extra arg until this script cares about chartjson
-  # results from telemetry
-  index = 0
-  for arg in rest_args:
-    if ('--isolated-script-test-chartjson-output' in arg or
-        '--isolated-script-test-perf-output' in arg):
-      rest_args.pop(index)
-      break
-    index += 1
-  if args.isolated_script_test_filter:
-    rest_args += ['--test-filter', args.isolated_script_test_filter]
-
-  # Compatibility with gtest-based sharding.
-  total_shards = None
-  shard_index = None
-  env = os.environ.copy()
-  env['CHROME_HEADLESS'] = '1'
-
-  if 'GTEST_TOTAL_SHARDS' in env:
-    total_shards = int(env['GTEST_TOTAL_SHARDS'])
-    del env['GTEST_TOTAL_SHARDS']
-  if 'GTEST_SHARD_INDEX' in env:
-    shard_index = int(env['GTEST_SHARD_INDEX'])
-    del env['GTEST_SHARD_INDEX']
-  sharding_args = []
-  if total_shards is not None and shard_index is not None:
-    sharding_args = [
-      '--total-shards=%d' % total_shards,
-      '--shard-index=%d' % shard_index
-    ]
-  cmd = [sys.executable] + rest_args + sharding_args + [
-      '--write-full-results-to', args.isolated_script_test_output]
-  if args.xvfb:
-    return xvfb.run_executable(cmd, env)
-  else:
-    return common.run_command(cmd, env=env)
+  adapter = TelemetryUnittestAdapter()
+  return adapter.run_test()
 
 
 # This is not really a "script test" so does not need to manually add