Adding sharding support to fix_test_cases.py

Modify the code in run_test_cases.py to allow other classes to use it's sharding and filtering code in an optionparser right away.

Also update what output is logged when run_test_cases.py fails to run a test and cleanup some of the environment variable settings.

NOTRY=true
BUG=


Review URL: https://chromiumcodereview.appspot.com/10875020

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@152996 0039d316-1c4b-4281-b951-d872f2087c98

3 files changed, 110 insertions, 68 deletions

diff --git a/tools/isolate/fix_test_cases.py b/tools/isolate/fix_test_cases.py
index efbb0c0..057765a 100755
--- a/tools/isolate/fix_test_cases.py
+++ b/tools/isolate/fix_test_cases.py
@@ -50,7 +50,7 @@ def trace_and_merge(result, test):
       [sys.executable, 'isolate.py', 'merge', '-r', result])
 
 
-def run_all(result):
+def run_all(result, shard_index, shard_count):
   """Runs all the tests. Returns the tests that failed or None on failure.
 
   Assumes run_test_cases.py is implicitly called.
@@ -59,11 +59,16 @@ def run_all(result):
   os.close(handle)
   env = os.environ.copy()
   env['RUN_TEST_CASES_RESULT_FILE'] = result_file
-  subprocess.call(
-      [sys.executable, 'isolate.py', 'run', '-r', result], env=env)
+  env['GTEST_SHARD_INDEX'] = str(shard_index)
+  env['GTEST_TOTAL_SHARDS'] = str(shard_count)
+  cmd = [sys.executable, 'isolate.py', 'run', '-r', result]
+  return_code = subprocess.call(cmd, env=env)
   if not os.path.isfile(result_file):
     print >> sys.stderr, 'Failed to find %s' % result_file
     return None
+  if return_code:
+    print >> sys.stderr, 'Failed to run isolate.py run'
+    return None
   with open(result_file) as f:
     try:
       data = json.load(f)
@@ -105,7 +110,7 @@ def trace_and_verify(result, test):
   return run(result, test)
 
 
-def fix_all(result):
+def fix_all(result, shard_index, shard_count):
   """Runs all the test cases in a gtest executable and trace the failing tests.
 
   Returns True on success.
@@ -119,7 +124,7 @@ def fix_all(result):
       print >> 'Please unset %s' % i
       return False
 
-  test_cases = run_all(result)
+  test_cases = run_all(result, shard_index, shard_count)
   if test_cases is None:
     return False
 
@@ -161,11 +166,14 @@ def fix_all(result):
 
 
 def main():
-  if len(sys.argv) != 2:
-    print >> sys.stderr, 'Use with the name of the test only, e.g. "unit_tests"'
-    return 1
+  parser = run_test_cases.OptionParserWithTestSharding(
+      usage='%prog <option> [test]')
+  options, args = parser.parse_args()
+
+  if len(args) != 1:
+    parser.error('Use with the name of the test only, e.g. "unit_tests"')
 
-  basename = sys.argv[1]
+  basename = args[0]
   executable = '../../out/Release/%s' % basename
   result = '%s.results' % executable
   if sys.platform == 'win32':
@@ -179,7 +187,7 @@ def main():
         '%s doesn\'t exist, please build %s_run' % (result, basename))
     return 1
 
-  return not fix_all(result)
+  return not fix_all(result, options.index, options.shards)
 
 
 if __name__ == '__main__':
diff --git a/tools/isolate/run_test_cases.py b/tools/isolate/run_test_cases.py
index df6a94e..cd90abf 100755
--- a/tools/isolate/run_test_cases.py
+++ b/tools/isolate/run_test_cases.py
@@ -366,6 +366,18 @@ def fix_python_path(cmd):
   return out
 
 
+def setup_gtest_env():
+  """Copy the enviroment variables and setup for running a gtest."""
+  env = os.environ.copy()
+  for name in GTEST_ENV_VARS_TO_REMOVE:
+    env.pop(name, None)
+
+  # Forcibly enable color by default, if not already disabled.
+  env.setdefault('GTEST_COLOR', 'on')
+
+  return env
+
+
 def gtest_list_tests(executable):
   """List all the test cases for a google test.
 
@@ -373,13 +385,16 @@ def gtest_list_tests(executable):
   """
   cmd = [executable, '--gtest_list_tests']
   cmd = fix_python_path(cmd)
+  env = setup_gtest_env()
   try:
-    p = subprocess.Popen(cmd, stdout=subprocess.PIPE, stderr=subprocess.PIPE)
+    p = subprocess.Popen(cmd, stdout=subprocess.PIPE, stderr=subprocess.PIPE,
+                         env=env)
   except OSError, e:
     raise Failure('Failed to run %s\n%s' % (executable, str(e)))
   out, err = p.communicate()
   if p.returncode:
-    raise Failure('Failed to run %s\n%s' % (executable, err), p.returncode)
+    raise Failure('Failed to run %s\nstdout:\n%s\nstderr:\n%s' %
+                  (executable, out, err), p.returncode)
   # pylint: disable=E1103
   if err and not err.startswith('Xlib:  extension "RANDR" missing on display '):
     logging.error('Unexpected spew in gtest_list_tests:\n%s\n%s', err, cmd)
@@ -464,11 +479,7 @@ class Runner(object):
     self.retry_count = retry_count
     # It is important to remove the shard environment variables since it could
     # conflict with --gtest_filter.
-    self.env = os.environ.copy()
-    for name in GTEST_ENV_VARS_TO_REMOVE:
-      self.env.pop(name, None)
-    # Forcibly enable color by default, if not already disabled.
-    self.env.setdefault('GTEST_COLOR', 'on')
+    self.env = setup_gtest_env()
 
   def map(self, test_case):
     """Traces a single test case and returns its output."""
@@ -607,14 +618,79 @@ def run_test_cases(executable, test_cases, jobs, timeout, result_file):
   return int(bool(fail))
 
 
+class OptionParserWithTestSharding(optparse.OptionParser):
+  """Adds automatic handling of test sharding"""
+  def __init__(self, *args, **kwargs):
+    optparse.OptionParser.__init__(self, *args, **kwargs)
+
+    def as_digit(variable, default):
+      return int(variable) if variable.isdigit() else default
+
+    group = optparse.OptionGroup(self, 'Which shard to run')
+    group.add_option(
+        '-i', '--index',
+        type='int',
+        default=as_digit(os.environ.get('GTEST_SHARD_INDEX', ''), None),
+        help='Shard index to run')
+    group.add_option(
+        '-s', '--shards',
+        type='int',
+        default=as_digit(os.environ.get('GTEST_TOTAL_SHARDS', ''), None),
+        help='Total number of shards to calculate from the --index to run')
+    self.add_option_group(group)
+
+
+class OptionParserWithTestShardingAndFiltering(OptionParserWithTestSharding):
+  """Adds automatic handling of test sharding and filtering."""
+  def __init__(self, *args, **kwargs):
+    OptionParserWithTestSharding.__init__(self, *args, **kwargs)
+
+    group = optparse.OptionGroup(self, 'Which test cases to run')
+    group.add_option(
+        '-w', '--whitelist',
+        default=[],
+        action='append',
+        help='filter to apply to test cases to run, wildcard-style, defaults '
+        'to all test')
+    group.add_option(
+        '-b', '--blacklist',
+        default=[],
+        action='append',
+        help='filter to apply to test cases to skip, wildcard-style, defaults '
+        'to no test')
+    group.add_option(
+        '-T', '--test-case-file',
+        help='File containing the exact list of test cases to run')
+    group.add_option(
+        '--gtest_filter',
+        default=os.environ.get('GTEST_FILTER', ''),
+        help='Runs a single test, provideded to keep compatibility with '
+        'other tools')
+    self.add_option_group(group)
+
+  def parse_args(self, *args, **kwargs):
+    options, args = optparse.OptionParser.parse_args(self, *args, **kwargs)
+
+    if options.gtest_filter:
+      # Override any other option.
+      # Based on UnitTestOptions::FilterMatchesTest() in
+      # http://code.google.com/p/googletest/source/browse/#svn%2Ftrunk%2Fsrc
+      if '-' in options.gtest_filter:
+        options.whitelist, options.blacklist = options.gtest_filter.split('-',
+                                                                          1)
+      else:
+        options.whitelist = options.gtest_filter
+        options.blacklist = ''
+      options.whitelist = [i for i in options.whitelist.split(':') if i]
+      options.blacklist = [i for i in options.blacklist.split(':') if i]
+
+    return options, args
+
+
 def main(argv):
   """CLI frontend to validate arguments."""
-  def as_digit(variable, default):
-    if variable.isdigit():
-      return int(variable)
-    return default
-
-  parser = optparse.OptionParser(usage='%prog <options> [gtest]')
+  parser = OptionParserWithTestShardingAndFiltering(
+      usage='%prog <options> [gtest]')
   parser.add_option(
       '-j', '--jobs',
       type='int',
@@ -638,39 +714,6 @@ def main(argv):
       '--result',
       default=os.environ.get('RUN_TEST_CASES_RESULT_FILE', ''),
       help='Override the default name of the generated .run_test_cases file')
-
-  group = optparse.OptionGroup(parser, 'Which test cases to run')
-  group.add_option(
-      '-w', '--whitelist',
-      default=[],
-      action='append',
-      help='filter to apply to test cases to run, wildcard-style, defaults to '
-           'all test')
-  group.add_option(
-      '-b', '--blacklist',
-      default=[],
-      action='append',
-      help='filter to apply to test cases to skip, wildcard-style, defaults to '
-           'no test')
-  group.add_option(
-      '-i', '--index',
-      type='int',
-      default=as_digit(os.environ.get('GTEST_SHARD_INDEX', ''), None),
-      help='Shard index to run')
-  group.add_option(
-      '-s', '--shards',
-      type='int',
-      default=as_digit(os.environ.get('GTEST_TOTAL_SHARDS', ''), None),
-      help='Total number of shards to calculate from the --index to run')
-  group.add_option(
-      '-T', '--test-case-file',
-      help='File containing the exact list of test cases to run')
-  group.add_option(
-      '--gtest_filter',
-      default=os.environ.get('GTEST_FILTER', ''),
-      help='Runs a single test, provideded to keep compatibility with '
-           'other tools')
-  parser.add_option_group(group)
   options, args = parser.parse_args(argv)
 
   levels = [logging.ERROR, logging.INFO, logging.DEBUG]
@@ -693,18 +736,6 @@ def main(argv):
   if not os.path.isfile(executable):
     parser.error('"%s" doesn\'t exist.' % executable)
 
-  if options.gtest_filter:
-    # Override any other option.
-    # Based on UnitTestOptions::FilterMatchesTest() in
-    # http://code.google.com/p/googletest/source/browse/#svn%2Ftrunk%2Fsrc
-    if '-' in options.gtest_filter:
-      options.whitelist, options.blacklist = options.gtest_filter.split('-', 1)
-    else:
-      options.whitelist = options.gtest_filter
-      options.blacklist = ''
-    options.whitelist = [i for i in options.whitelist.split(':') if i]
-    options.blacklist = [i for i in options.blacklist.split(':') if i]
-
   # Grab the test cases.
   if options.test_case_file:
     with open(options.test_case_file, 'r') as f:
diff --git a/tools/isolate/run_test_cases_smoke_test.py b/tools/isolate/run_test_cases_smoke_test.py
index 5a17d40..39727092 100755
--- a/tools/isolate/run_test_cases_smoke_test.py
+++ b/tools/isolate/run_test_cases_smoke_test.py
@@ -145,6 +145,9 @@ class TraceTestCases(unittest.TestCase):
 
     expected_out_re = [
         'Failed to run .+gtest_fake_error.py',
+        'stdout:',
+        '',
+        'stderr:',
         'Unable to list tests'
     ]