[foozzie] Stop using extra metadata files.

Continuation of:
https://codereview.chromium.org/2620343005/

This removes usage of metadata files entirely. Instead we extract
the instrumentation about source files from the test cases.

This also adds extra output of the original source file in the
detailed failure text for easier debugging. The hashes alone
made it hard to reason.

BUG=chromium:673246
NOTRY=true
TBR=tandrii@chromium.org,mbarbella@chromium.org

Review-Url: https://codereview.chromium.org/2634743004
Cr-Commit-Position: refs/heads/master@{#42371}

tools/foozzie/testdata/failure_output.txt

@@ -17,6 +17,9 @@
 - unknown
 + not unknown
 #
+# Source file:
+name/to/file.js
+#
 ### Start of configuration x64,fullcode:
 
 1

tools/foozzie/testdata/meta-123.js

-{
-  "sources": ["name/to/a/file.js", "name/to/file.js"]
-}

tools/foozzie/testdata/resources-123.js

-meta-test1.json

tools/foozzie/v8_foozzie.py

@@ -51,7 +51,7 @@ SUPPORTED_ARCHS = ['ia32', 'x64', 'arm', 'arm64']
 FAILURE_HEADER_TEMPLATE = """#
 # V8 correctness failure
 # V8 correctness configs: %(configs)s
-# V8 correctness sources: %(sources)s
+# V8 correctness sources: %(source_key)s
 # V8 correctness suppression: %(suppression)s
 """
 
@@ -69,6 +69,9 @@ FAILURE_TEMPLATE = FAILURE_HEADER_TEMPLATE + """#
 # Difference:
 %(difference)s
 #
+# Source file:
+%(source)s
+#
 ### Start of configuration %(first_config_label)s:
 %(first_config_output)s
 ### End of configuration %(first_config_label)s
@@ -79,6 +82,7 @@ FAILURE_TEMPLATE = FAILURE_HEADER_TEMPLATE + """#
 """
 
 FUZZ_TEST_RE = re.compile(r'.*fuzz(-\d+\.js)')
+SOURCE_RE = re.compile(r'print\("v8-foozzie source: (.*)"\);')
 
 def parse_args():
   parser = argparse.ArgumentParser()
@@ -117,14 +121,6 @@ def parse_args():
           os.path.isfile(options.testcase)), (
       'Test case %s doesn\'t exist' % options.testcase)
 
-  # Deduce metadata file name from test case. This also removes
-  # the prefix the test case might get during minimization.
-  suffix = FUZZ_TEST_RE.match(os.path.basename(options.testcase)).group(1)
-  options.meta_data_path = os.path.join(
-      os.path.dirname(options.testcase), 'meta' + suffix)
-  assert os.path.exists(options.meta_data_path), (
-      'Metadata %s doesn\'t exist' % options.meta_data_path)
-
   # Use first d8 as default for second d8.
   options.second_d8 = options.second_d8 or options.first_d8
 
@@ -147,27 +143,26 @@ def parse_args():
   return options
 
 
-def metadata_bailout(metadata, ignore_fun):
-  """Print failure state and return if ignore_fun matches metadata."""
-  bug = (ignore_fun(metadata) or '').strip()
+def get_meta_data(content):
+  """Extracts original-source-file paths from test case content."""
+  sources = []
+  for line in content.splitlines():
+    match = SOURCE_RE.match(line)
+    if match:
+      sources.append(match.group(1))
+  return {'sources': sources}
+
+
+def content_bailout(content, ignore_fun):
+  """Print failure state and return if ignore_fun matches content."""
+  bug = (ignore_fun(content) or '').strip()
   if bug:
     print FAILURE_HEADER_TEMPLATE % dict(
-        configs='', sources='', suppression=bug)
+        configs='', source_key='', suppression=bug)
     return True
   return False
 
 
-def test_pattern_bailout(testcase, ignore_fun):
-  """Print failure state and return if ignore_fun matches testcase."""
-  with open(testcase) as f:
-    bug = (ignore_fun(f.read()) or '').strip()
-    if bug:
-      print FAILURE_HEADER_TEMPLATE % dict(
-          configs='', sources='', suppression=bug)
-      return True
-  return False
-
-
 def pass_bailout(output, step_number):
   """Print info and return if in timeout or crash pass states."""
   if output.HasTimedOut():
@@ -186,7 +181,7 @@ def fail_bailout(output, ignore_by_output_fun):
   bug = (ignore_by_output_fun(output.stdout) or '').strip()
   if bug:
     print FAILURE_HEADER_TEMPLATE % dict(
-        configs='', sources='', suppression=bug)
+        configs='', source_key='', suppression=bug)
     return True
   return False
 
@@ -200,18 +195,15 @@ def main():
       options.second_arch, options.second_config,
   )
 
-  # Get metadata.
-  # TODO(machenbach): We probably don't need the metadata file anymore
-  # now that the metadata is printed in the test cases.
-  with open(options.meta_data_path) as f:
-    metadata = json.load(f)
-
-  if metadata_bailout(metadata, suppress.ignore_by_metadata):
+  # Static bailout based on test case content or metadata.
+  with open(options.testcase) as f:
+    content = f.read()
+  if content_bailout(get_meta_data(content), suppress.ignore_by_metadata):
     return RETURN_FAIL
-
-  if test_pattern_bailout(options.testcase, suppress.ignore_by_content):
+  if content_bailout(content, suppress.ignore_by_content):
     return RETURN_FAIL
 
+  # Set up runtime arguments.
   common_flags = FLAGS + ['--random-seed', str(options.random_seed)]
   first_config_flags = common_flags + CONFIGS[options.first_config]
   second_config_flags = common_flags + CONFIGS[options.second_config]
@@ -243,7 +235,7 @@ def main():
   if fail_bailout(second_config_output, suppress.ignore_by_output2):
     return RETURN_FAIL
 
-  difference, source_key = suppress.diff(
+  difference, source, source_key = suppress.diff(
       first_config_output.stdout, second_config_output.stdout)
   if difference:
     # The first three entries will be parsed by clusterfuzz. Format changes
@@ -252,7 +244,7 @@ def main():
     second_config_label = '%s,%s' % (options.second_arch, options.second_config)
     print FAILURE_TEMPLATE % dict(
         configs='%s:%s' % (first_config_label, second_config_label),
-        sources=source_key,
+        source_key=source_key,
         suppression='', # We can't tie bugs to differences.
         first_config_label=first_config_label,
         second_config_label=second_config_label,
@@ -260,6 +252,7 @@ def main():
         second_config_flags=' '.join(second_config_flags),
         first_config_output=first_config_output.stdout,
         second_config_output=second_config_output.stdout,
+        source=source,
         difference=difference,
     )
     return RETURN_FAIL
@@ -279,11 +272,11 @@ if __name__ == "__main__":
     # Make sure clusterfuzz reports internal errors and wrong usage.
     # Use one label for all internal and usage errors.
     print FAILURE_HEADER_TEMPLATE % dict(
-        configs='', sources='', suppression='wrong_usage')
+        configs='', source_key='', suppression='wrong_usage')
     result = RETURN_FAIL
   except Exception as e:
     print FAILURE_HEADER_TEMPLATE % dict(
-        configs='', sources='', suppression='internal_error')
+        configs='', source_key='', suppression='internal_error')
     print '# Internal error: %s' % e
     traceback.print_exc(file=sys.stdout)
     result = RETURN_FAIL

tools/foozzie/v8_foozzie_test.py

@@ -21,12 +21,12 @@ class UnitTest(unittest.TestCase):
         'x64', 'fullcode', 'x64', 'default')
     one = ''
     two = ''
-    diff = None, 'none'
+    diff = None, 'none', 'none'
     self.assertEquals(diff, suppress.diff(one, two))
 
     one = 'a \n  b\nc();'
     two = 'a \n  b\nc();'
-    diff = None, 'none'
+    diff = None, 'none', 'none'
     self.assertEquals(diff, suppress.diff(one, two))
 
     # Ignore line before caret, caret position, stack trace char numbers
@@ -49,7 +49,7 @@ stack line :2: foo
 Validation of asm.js module failed: baz
   undefined
 """
-    diff = None, 'none'
+    diff = None, 'none', 'none'
     self.assertEquals(diff, suppress.diff(one, two))
 
     one = """
@@ -59,7 +59,7 @@ Extra line
     two = """
 Still equal
 """
-    diff = '- Extra line', 'none'
+    diff = '- Extra line', 'none', 'none'
     self.assertEquals(diff, suppress.diff(one, two))
 
     one = """
@@ -69,7 +69,7 @@ Still equal
 Still equal
 Extra line
 """
-    diff = '+ Extra line', 'none'
+    diff = '+ Extra line', 'none', 'none'
     self.assertEquals(diff, suppress.diff(one, two))
 
     one = """
@@ -81,7 +81,7 @@ undefined
 otherfile.js: TypeError: undefined is not a constructor
 """
     diff = """- somefile.js: TypeError: undefined is not a constructor
-+ otherfile.js: TypeError: undefined is not a constructor""", 'none'
++ otherfile.js: TypeError: undefined is not a constructor""", 'none', 'none'
     self.assertEquals(diff, suppress.diff(one, two))
 
 

tools/foozzie/v8_suppressions.py

@@ -145,6 +145,7 @@ IGNORE_LINES = [re.compile(exp) for exp in IGNORE_LINES]
 SOURCE_HASH_LENGTH = 3
 
 ORIGINAL_SOURCE_PREFIX = 'v8-foozzie source: '
+ORIGINAL_SOURCE_DEFAULT = 'none'
 
 def line_pairs(lines):
   return itertools.izip_longest(
@@ -182,13 +183,15 @@ def ignore_by_regexp(line1, line2, allowed):
 
 
 def diff_output(output1, output2, allowed, ignore1, ignore2):
-  """Returns a tuple (difference, source_key).
+  """Returns a tuple (difference, source, source_key).
 
   The difference is None if there's no difference, otherwise a string
   with a readable diff.
 
-  The source_key is a short hash of the last source output within the test
-  case. It is the string 'none' if no such output existed.
+  The source is the last source output within the test case. It is the string
+  'none' if no such output existed.
+
+  The source_key is a short hash of source or 'none'.
   """
   def useful_line(ignore):
     def fun(line):
@@ -200,7 +203,8 @@ def diff_output(output1, output2, allowed, ignore1, ignore2):
 
   # This keeps track where we are in the original source file of the fuzz
   # test case.
-  source_key = 'none'
+  source = ORIGINAL_SOURCE_DEFAULT
+  source_key = ORIGINAL_SOURCE_DEFAULT
 
   for ((line1, lookahead1), (line2, lookahead2)) in itertools.izip_longest(
       line_pairs(lines1), line_pairs(lines2), fillvalue=(None, None)):
@@ -210,9 +214,9 @@ def diff_output(output1, output2, allowed, ignore1, ignore2):
 
     # One iterator ends earlier.
     if line1 is None:
-      return '+ %s' % short_line_output(line2), source_key
+      return '+ %s' % short_line_output(line2), source, source_key
     if line2 is None:
-      return '- %s' % short_line_output(line1), source_key
+      return '- %s' % short_line_output(line1), source, source_key
 
     # If lines are equal, no further checks are necessary.
     if line1 == line2:
@@ -235,11 +239,12 @@ def diff_output(output1, output2, allowed, ignore1, ignore2):
     # Lines are different.
     return (
         '- %s\n+ %s' % (short_line_output(line1), short_line_output(line2)),
+        source,
         source_key,
     )
 
   # No difference found.
-  return None, source_key
+  return None, source, source_key
 
 
 def get_suppression(arch1, config1, arch2, config2):