Add variables support to trace_test_cases.py, removing the last use of --product-dir.

Add --verbose support to trace_test_cases.py.

Clean up isolate dictionary generation and filter known spew. Adds <(EXECUTABLE_SUFFIX) where appropriate.

R=cmp@chromium.org
NOTRY=true
BUG=
TEST=


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

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

5 files changed, 123 insertions, 62 deletions

diff --git a/tools/isolate/isolate.py b/tools/isolate/isolate.py
index a4d0a01..07a08bd 100755
--- a/tools/isolate/isolate.py
+++ b/tools/isolate/isolate.py
@@ -249,7 +249,7 @@ def process_variables(variables, relative_base_dir, error):
   to an absolute path, then sets it as relative to relative_base_dir.
   """
   variables = variables.copy()
-  for i in ('DEPTH', 'PRODUCT_DIR'):
+  for i in isolate_common.PATH_VARIABLES:
     if i not in variables:
       continue
     variable = os.path.normpath(variables[i])
@@ -566,31 +566,14 @@ def MODEread(_outdir, state):
   logfile = state.result_file + '.log'
   if not os.path.isfile(logfile):
     return 1
-  product_dir = None
-  if state.resultdir and state.root_dir:
-    # Defaults to none if both are the same directory.
-    try:
-      product_dir = os.path.relpath(state.resultdir, state.root_dir) or None
-    except ValueError:
-      # This happens on Windows if state.resultdir is one drive, let's say
-      # 'C:\' and state.root_dir on another one like 'D:\'.
-      product_dir = None
   try:
     results = trace_inputs.load_trace(
         logfile, state.root_dir, api, isolate_common.default_blacklist)
-    simplified = trace_inputs.extract_directories(state.root_dir, results.files)
-    variables = isolate_common.generate_dict(
-            (f.path for f in simplified),
-            state.result.relative_cwd,
-            product_dir)
-    # Outputs in a way that is easy to merge with merge_isolate.py.
-    value = {
-      'conditions': [
-        ['OS=="%s"' % isolate_common.get_flavor(), {
-          'variables': variables,
-        }],
-      ],
-    }
+    value = isolate_common.generate_isolate(
+        results.existent,
+        state.root_dir,
+        state.saved_state.variables,
+        state.result.relative_cwd)
     isolate_common.pretty_print(value, sys.stdout)
     return 0
   except trace_inputs.TracingFailure, e:
diff --git a/tools/isolate/isolate_common.py b/tools/isolate/isolate_common.py
index e37781c..b04b8a1 100644
--- a/tools/isolate/isolate_common.py
+++ b/tools/isolate/isolate_common.py
@@ -9,8 +9,13 @@
 import logging
 import os
 import posixpath
+import re
 import sys
 
+import trace_inputs
+
+
+PATH_VARIABLES = ('DEPTH', 'PRODUCT_DIR')
 
 KEY_TRACKED = 'isolate_dependency_tracked'
 KEY_UNTRACKED = 'isolate_dependency_untracked'
@@ -54,43 +59,86 @@ def get_flavor():
 def default_blacklist(f):
   """Filters unimportant files normally ignored."""
   return (
-      f.endswith('.pyc') or
+      f.endswith(('.pyc', 'testserver.log')) or
       _GIT_PATH in f or
       _SVN_PATH in f)
 
 
-def generate_dict(files, cwd_dir, product_dir):
-  """Converts the list of files into a .isolate dictionary.
+def classify_files(files):
+  """Converts the list of files into a .isolate 'variables' dictionary.
 
   Arguments:
-  - files: list of files to generate a dictionary out of.
-  - cwd_dir: directory to base all the files from, relative to root_dir.
-  - product_dir: directory to replace with <(PRODUCT_DIR), relative to root_dir.
+  - files: list of files names to generate a dictionary out of.
+  """
+  files = list(files)
+  tracked = sorted(f for f in files if not f.endswith('/') and ' ' not in f)
+  untracked = sorted(f for f in files if f.endswith('/') or ' ' in f)
+  variables = {}
+  if tracked:
+    variables[KEY_TRACKED] = tracked
+  if untracked:
+    variables[KEY_UNTRACKED] = untracked
+  return variables
+
+
+def generate_simplified(files, root_dir, variables, relative_cwd):
+  """Generates a clean and complete .isolate 'variables' dictionary.
+
+  Cleans up and extracts only files from within root_dir then processes
+  variables and relative_cwd.
   """
-  cwd_dir = cleanup_path(cwd_dir)
-  product_dir = cleanup_path(product_dir)
+  # Constants.
+  # Skip log in PRODUCT_DIR.
+  LOG_FILE = re.compile(r'^\<\(PRODUCT_DIR\)\/[^/]+\.log$')
+  EXECUTABLE = re.compile(
+      r'^(\<\(PRODUCT_DIR\)\/[^/\.]+)' +
+      re.escape(variables.get('EXECUTABLE_SUFFIX', '')) +
+      r'$')
+
+  # Preparation work.
+  relative_cwd = cleanup_path(relative_cwd)
+  # Creates the right set of variables here. We only care about
+  # PATH_VARIABLES.
+  variables = dict(
+      ('<(%s)' % k, variables[k]) for k in PATH_VARIABLES if k in variables)
+
+  # Actual work: Process the files.
+  files = trace_inputs.extract_directories(root_dir, files)
+  files = (f.replace_variables(variables) for f in files)
 
   def fix(f):
     """Bases the file on the most restrictive variable."""
     logging.debug('fix(%s)' % f)
     # Important, GYP stores the files with / and not \.
     f = f.replace(os.path.sep, '/')
-    if product_dir and f.startswith(product_dir):
-      return '<(PRODUCT_DIR)/%s' % f[len(product_dir):]
-    else:
-      # cwd_dir is usually the directory containing the gyp file. It may be
+    # If it's not already a variable.
+    if not f.startswith('<'):
+      # relative_cwd is usually the directory containing the gyp file. It may be
       # empty if the whole directory containing the gyp file is needed.
-      return posix_relpath(f, cwd_dir) or './'
-
-  corrected = [fix(f) for f in files]
-  tracked = [f for f in corrected if not f.endswith('/') and ' ' not in f]
-  untracked = [f for f in corrected if f.endswith('/') or ' ' in f]
-  variables = {}
-  if tracked:
-    variables[KEY_TRACKED] = tracked
-  if untracked:
-    variables[KEY_UNTRACKED] = untracked
-  return variables
+      f = posix_relpath(f, relative_cwd) or './'
+
+    # Now strips off known files we want to ignore and to any specific mangling
+    # as necessary. It's easier to do it here than generate a blacklist.
+    match = EXECUTABLE.match(f)
+    if match:
+      return match.group(1) + '<(EXECUTABLE_SUFFIX)'
+    if LOG_FILE.match(f):
+      return None
+    return f
+
+  return classify_files(filter(None, (fix(f.path) for f in files)))
+
+
+def generate_isolate(files, root_dir, variables, relative_cwd):
+  """Generates a clean and complete .isolate file."""
+  result = generate_simplified(files, root_dir, variables, relative_cwd)
+  return {
+    'conditions': [
+      ['OS=="%s"' % get_flavor(), {
+        'variables': result,
+      }],
+    ],
+  }
 
 
 def pretty_print(variables, stdout):
diff --git a/tools/isolate/isolate_smoke_test.py b/tools/isolate/isolate_smoke_test.py
index 6826220..ff14bd1 100755
--- a/tools/isolate/isolate_smoke_test.py
+++ b/tools/isolate/isolate_smoke_test.py
@@ -556,8 +556,8 @@ class Isolate_read_trace(IsolateModeBase):
     expected = self._wrap_in_condition(
         {
           isolate.isolate_common.KEY_TRACKED: [
-            'touch_root.py',
             '../../isolate.py',
+            'touch_root.py',
           ],
         })
     out = self._execute('read', 'touch_root.isolate', [], True)
@@ -703,8 +703,8 @@ class IsolateNoOutdir(IsolateBase):
     output = self._execute('read', [], True)
     expected = {
       isolate.isolate_common.KEY_TRACKED: [
-        'touch_root.py',
         '../../isolate.py',
+        'touch_root.py',
       ],
     }
     self.assertEquals(self._wrap_in_condition(expected), output)
diff --git a/tools/isolate/trace_test_cases.py b/tools/isolate/trace_test_cases.py
index df807c1..f010fbe 100755
--- a/tools/isolate/trace_test_cases.py
+++ b/tools/isolate/trace_test_cases.py
@@ -135,6 +135,8 @@ def trace_test_cases(
     else:
       print >> sys.stderr, 'Got exception while tracing %s: %s' % (
           item['trace'], item['exception'])
+  print '%.1fs Done stripping root.' % (
+      time.time() - progress.start)
 
   # Flatten.
   flattened = {}
@@ -152,12 +154,16 @@ def trace_test_cases(
             'returncode': item['returncode'],
             'valid': item['valid'],
             'variables':
-              isolate_common.generate_dict(
-                sorted(f.path for f in item_results.files),
-                cwd_dir,
-                variables['<(PRODUCT_DIR)']),
+              isolate_common.generate_simplified(
+                  item_results.existent,
+                  root_dir,
+                  variables,
+                  cwd_dir),
           })
         del flattened[test_case]['trace']
+  print '%.1fs Done flattening.' % (
+      time.time() - progress.start)
+
   # Make it dense if there is more than 20 results.
   trace_inputs.write_json(
       output_file,
@@ -172,9 +178,11 @@ def trace_test_cases(
   # Convert back to a list, discard the keys.
   files = files.values()
 
-  # TODO(maruel): Have isolate_common process a dict of variables.
-  value = isolate_common.generate_dict(
-      sorted(f.path for f in files), cwd_dir, variables['<(PRODUCT_DIR)'])
+  value = isolate_common.generate_isolate(
+      files,
+      root_dir,
+      variables,
+      cwd_dir)
   with open('%s.isolate' % output_file, 'wb') as f:
     isolate_common.pretty_print(value, f)
   return 0
@@ -182,6 +190,11 @@ def trace_test_cases(
 
 def main():
   """CLI frontend to validate arguments."""
+  default_variables = [('OS', isolate_common.get_flavor())]
+  if sys.platform in ('win32', 'cygwin'):
+    default_variables.append(('EXECUTABLE_SUFFIX', '.exe'))
+  else:
+    default_variables.append(('EXECUTABLE_SUFFIX', ''))
   parser = optparse.OptionParser(
       usage='%prog <options> [gtest]',
       description=sys.modules['__main__'].__doc__)
@@ -194,9 +207,13 @@ def main():
            'gyp processing. Should be set to the relative path containing the '
            'gyp file, e.g. \'chrome\' or \'net\'')
   parser.add_option(
-      '-p', '--product-dir',
-      default='out/Release',
-      help='Directory for PRODUCT_DIR. Default: %default')
+      '-V', '--variable',
+      nargs=2,
+      action='append',
+      default=default_variables,
+      dest='variables',
+      metavar='FOO BAR',
+      help='Variables to process in the .isolate file, default: %default')
   parser.add_option(
       '--root-dir',
       default=ROOT_DIR,
@@ -225,8 +242,18 @@ def main():
       default=120,
       type='int',
       help='number of parallel jobs')
+  parser.add_option(
+      '-v', '--verbose',
+      action='count',
+      default=0,
+      help='Use multiple times to increase verbosity')
   options, args = parser.parse_args()
 
+  levels = [logging.ERROR, logging.WARNING, logging.INFO, logging.DEBUG]
+  logging.basicConfig(
+      level=levels[min(len(levels)-1, options.verbose)],
+      format='%(levelname)5s %(module)15s(%(lineno)3d): %(message)s')
+
   if len(args) != 1:
     parser.error(
         'Please provide the executable line to run, if you need fancy things '
@@ -234,15 +261,14 @@ def main():
         '.')
   executable = args[0]
   if not os.path.isabs(executable):
-    executable = os.path.join(options.root_dir, options.product_dir, args[0])
+    executable = os.path.abspath(os.path.join(options.root_dir, executable))
   if not options.out:
     options.out = '%s.test_cases' % executable
-  variables = {'<(PRODUCT_DIR)': options.product_dir}
   return trace_test_cases(
       executable,
       options.root_dir,
       options.cwd,
-      variables,
+      dict(options.variables),
       options.whitelist,
       options.blacklist,
       options.jobs,
diff --git a/tools/isolate/trace_test_cases_smoke_test.py b/tools/isolate/trace_test_cases_smoke_test.py
index faa11130..57faf23 100755
--- a/tools/isolate/trace_test_cases_smoke_test.py
+++ b/tools/isolate/trace_test_cases_smoke_test.py
@@ -109,9 +109,11 @@ class TraceTestCases(unittest.TestCase):
         '--out', self.temp_file,
         '--root-dir', ROOT_DIR,
         '--cwd', ROOT_DIR,
-        '--product-dir', 'data',
+        '--variable', 'PRODUCT_DIR', 'data',
         TARGET_PATH,
     ]
+    if VERBOSE:
+      cmd.extend(['-v'] * 3)
     logging.debug(' '.join(cmd))
     proc = subprocess.Popen(
         cmd, stdout=subprocess.PIPE, stderr=subprocess.PIPE)
@@ -124,6 +126,8 @@ class TraceTestCases(unittest.TestCase):
         r'',
         r'\d+\.\ds Done post-processing logs\. Parsing logs\.',
         r'\d+\.\ds Done parsing logs\.',
+        r'\d+.\ds Done stripping root\.',
+        r'\d+.\ds Done flattening\.',
         r'',
       ])
     self.assertTrue(re.match('^%s$' % expected_out_re, out), repr(out))