Commit 00e98a39 authored by machenbach's avatar machenbach Committed by Commit bot

[foozzie] Improve failure state deduplication

The fuzz test cases now print the original test paths during execution.
This exploits this extra information and reports a hash of only one
original source file from the section that caused a difference.

The hash size is now limited to 3 to avoid possible duplicate
explosion, in case this doesn't work out as expected.

This prepares for patch 3 of:
https://chromereviews.googleplex.com/550337016/

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

Review-Url: https://codereview.chromium.org/2620343005
Cr-Commit-Position: refs/heads/master@{#42305}
parent d730551e
# #
# V8 correctness failure # V8 correctness failure
# V8 correctness configs: x64,fullcode:x64,ignition_staging # V8 correctness configs: x64,fullcode:x64,ignition_staging
# V8 correctness sources: f602adcc # V8 correctness sources: f60
# V8 correctness suppression: # V8 correctness suppression:
# #
# CHECK # CHECK
...@@ -20,7 +20,9 @@ ...@@ -20,7 +20,9 @@
### Start of configuration x64,fullcode: ### Start of configuration x64,fullcode:
1 1
v8-foozzie source: name/to/a/file.js
2 2
v8-foozzie source: name/to/file.js
weird error weird error
^ ^
3 3
...@@ -32,7 +34,9 @@ unknown ...@@ -32,7 +34,9 @@ unknown
### Start of configuration x64,ignition_staging: ### Start of configuration x64,ignition_staging:
1 1
v8-foozzie source: name/to/a/file.js
2 2
v8-foozzie source: name/to/file.js
weird other error weird other error
^ ^
3 3
......
{ {
"sources": ["name/to/file.js"] "sources": ["name/to/a/file.js", "name/to/file.js"]
} }
...@@ -4,7 +4,9 @@ ...@@ -4,7 +4,9 @@
print """ print """
1 1
v8-foozzie source: name/to/a/file.js
2 2
v8-foozzie source: name/to/file.js
weird error weird error
^ ^
3 3
......
...@@ -4,7 +4,9 @@ ...@@ -4,7 +4,9 @@
print """ print """
1 1
v8-foozzie source: name/to/a/file.js
2 2
v8-foozzie source: name/to/file.js
weird other error weird other error
^ ^
3 3
......
...@@ -4,7 +4,9 @@ ...@@ -4,7 +4,9 @@
print """ print """
1 1
v8-foozzie source: name/to/a/file.js
2 2
v8-foozzie source: name/to/file.js
weird other error weird other error
^ ^
3 3
......
...@@ -37,6 +37,10 @@ TIMEOUT = 3 ...@@ -37,6 +37,10 @@ TIMEOUT = 3
RETURN_PASS = 0 RETURN_PASS = 0
RETURN_FAIL = 2 RETURN_FAIL = 2
# The number of hex digits used from the hash of the original source file path.
# Keep the number small to avoid duplicate explosion.
SOURCE_HASH_LENGTH = 3
BASE_PATH = os.path.dirname(os.path.abspath(__file__)) BASE_PATH = os.path.dirname(os.path.abspath(__file__))
PREAMBLE = [ PREAMBLE = [
os.path.join(BASE_PATH, 'v8_mock.js'), os.path.join(BASE_PATH, 'v8_mock.js'),
...@@ -202,6 +206,8 @@ def main(): ...@@ -202,6 +206,8 @@ def main():
) )
# Get metadata. # 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: with open(options.meta_data_path) as f:
metadata = json.load(f) metadata = json.load(f)
...@@ -242,17 +248,16 @@ def main(): ...@@ -242,17 +248,16 @@ def main():
if fail_bailout(second_config_output, suppress.ignore_by_output2): if fail_bailout(second_config_output, suppress.ignore_by_output2):
return RETURN_FAIL return RETURN_FAIL
difference = suppress.diff( difference, source = suppress.diff(
first_config_output.stdout, second_config_output.stdout) first_config_output.stdout, second_config_output.stdout)
if difference: if difference:
# The first three entries will be parsed by clusterfuzz. Format changes # The first three entries will be parsed by clusterfuzz. Format changes
# will require changes on the clusterfuzz side. # will require changes on the clusterfuzz side.
first_config_label = '%s,%s' % (options.first_arch, options.first_config) first_config_label = '%s,%s' % (options.first_arch, options.first_config)
second_config_label = '%s,%s' % (options.second_arch, options.second_config) second_config_label = '%s,%s' % (options.second_arch, options.second_config)
hsh = lambda x: hashlib.sha1(x).hexdigest()[:8]
print FAILURE_TEMPLATE % dict( print FAILURE_TEMPLATE % dict(
configs='%s:%s' % (first_config_label, second_config_label), configs='%s:%s' % (first_config_label, second_config_label),
sources=','.join(map(hsh, metadata['sources'])), sources=hashlib.sha1(source).hexdigest()[:SOURCE_HASH_LENGTH],
suppression='', # We can't tie bugs to differences. suppression='', # We can't tie bugs to differences.
first_config_label=first_config_label, first_config_label=first_config_label,
second_config_label=second_config_label, second_config_label=second_config_label,
......
...@@ -21,12 +21,12 @@ class UnitTest(unittest.TestCase): ...@@ -21,12 +21,12 @@ class UnitTest(unittest.TestCase):
'x64', 'fullcode', 'x64', 'default') 'x64', 'fullcode', 'x64', 'default')
one = '' one = ''
two = '' two = ''
diff = None diff = None, 'none'
self.assertEquals(diff, suppress.diff(one, two)) self.assertEquals(diff, suppress.diff(one, two))
one = 'a \n b\nc();' one = 'a \n b\nc();'
two = 'a \n b\nc();' two = 'a \n b\nc();'
diff = None diff = None, 'none'
self.assertEquals(diff, suppress.diff(one, two)) self.assertEquals(diff, suppress.diff(one, two))
# Ignore line before caret, caret position, stack trace char numbers # Ignore line before caret, caret position, stack trace char numbers
...@@ -49,7 +49,7 @@ stack line :2: foo ...@@ -49,7 +49,7 @@ stack line :2: foo
Validation of asm.js module failed: baz Validation of asm.js module failed: baz
undefined undefined
""" """
diff = None diff = None, 'none'
self.assertEquals(diff, suppress.diff(one, two)) self.assertEquals(diff, suppress.diff(one, two))
one = """ one = """
...@@ -59,7 +59,7 @@ Extra line ...@@ -59,7 +59,7 @@ Extra line
two = """ two = """
Still equal Still equal
""" """
diff = '- Extra line' diff = '- Extra line', 'none'
self.assertEquals(diff, suppress.diff(one, two)) self.assertEquals(diff, suppress.diff(one, two))
one = """ one = """
...@@ -69,7 +69,7 @@ Still equal ...@@ -69,7 +69,7 @@ Still equal
Still equal Still equal
Extra line Extra line
""" """
diff = '+ Extra line' diff = '+ Extra line', 'none'
self.assertEquals(diff, suppress.diff(one, two)) self.assertEquals(diff, suppress.diff(one, two))
one = """ one = """
...@@ -81,7 +81,7 @@ undefined ...@@ -81,7 +81,7 @@ undefined
otherfile.js: TypeError: undefined is not a constructor otherfile.js: TypeError: undefined is not a constructor
""" """
diff = """- somefile.js: TypeError: undefined is not a constructor diff = """- somefile.js: TypeError: undefined is not a constructor
+ otherfile.js: TypeError: undefined is not a constructor""" + otherfile.js: TypeError: undefined is not a constructor""", 'none'
self.assertEquals(diff, suppress.diff(one, two)) self.assertEquals(diff, suppress.diff(one, two))
......
...@@ -148,6 +148,7 @@ IGNORE_LINES = [ ...@@ -148,6 +148,7 @@ IGNORE_LINES = [
ALLOWED_LINE_DIFFS = [re.compile(exp) for exp in ALLOWED_LINE_DIFFS] ALLOWED_LINE_DIFFS = [re.compile(exp) for exp in ALLOWED_LINE_DIFFS]
IGNORE_LINES = [re.compile(exp) for exp in IGNORE_LINES] IGNORE_LINES = [re.compile(exp) for exp in IGNORE_LINES]
ORIGINAL_SOURCE_PREFIX = 'v8-foozzie source: '
def line_pairs(lines): def line_pairs(lines):
return itertools.izip_longest( return itertools.izip_longest(
...@@ -185,6 +186,14 @@ def ignore_by_regexp(line1, line2, allowed): ...@@ -185,6 +186,14 @@ def ignore_by_regexp(line1, line2, allowed):
def diff_output(output1, output2, allowed, ignore1, ignore2): def diff_output(output1, output2, allowed, ignore1, ignore2):
"""Returns a tuple (difference, source).
The difference is None if there's no difference, otherwise a string
with a readable diff.
The source is a string with the last source output within the test case.
It is the string 'none' if no such output existed.
"""
def useful_line(ignore): def useful_line(ignore):
def fun(line): def fun(line):
return all(not e.match(line) for e in ignore) return all(not e.match(line) for e in ignore)
...@@ -193,6 +202,10 @@ def diff_output(output1, output2, allowed, ignore1, ignore2): ...@@ -193,6 +202,10 @@ def diff_output(output1, output2, allowed, ignore1, ignore2):
lines1 = filter(useful_line(ignore1), output1) lines1 = filter(useful_line(ignore1), output1)
lines2 = filter(useful_line(ignore2), output2) lines2 = filter(useful_line(ignore2), output2)
# This keeps track where we are in the original source file of the fuzz
# test case.
source = 'none'
for ((line1, lookahead1), (line2, lookahead2)) in itertools.izip_longest( for ((line1, lookahead1), (line2, lookahead2)) in itertools.izip_longest(
line_pairs(lines1), line_pairs(lines2), fillvalue=(None, None)): line_pairs(lines1), line_pairs(lines2), fillvalue=(None, None)):
...@@ -201,12 +214,17 @@ def diff_output(output1, output2, allowed, ignore1, ignore2): ...@@ -201,12 +214,17 @@ def diff_output(output1, output2, allowed, ignore1, ignore2):
# One iterator ends earlier. # One iterator ends earlier.
if line1 is None: if line1 is None:
return '+ %s' % short_line_output(line2) return '+ %s' % short_line_output(line2), source
if line2 is None: if line2 is None:
return '- %s' % short_line_output(line1) return '- %s' % short_line_output(line1), source
# If lines are equal, no further checks are necessary. # If lines are equal, no further checks are necessary.
if line1 == line2: if line1 == line2:
# Instrumented original-source-file output must be equal in both
# versions. It only makes sense to update it here when both lines
# are equal.
if line1.startswith(ORIGINAL_SOURCE_PREFIX):
source = line1[len(ORIGINAL_SOURCE_PREFIX):]
continue continue
# Look ahead. If next line is a caret, ignore this line. # Look ahead. If next line is a caret, ignore this line.
...@@ -218,10 +236,13 @@ def diff_output(output1, output2, allowed, ignore1, ignore2): ...@@ -218,10 +236,13 @@ def diff_output(output1, output2, allowed, ignore1, ignore2):
continue continue
# Lines are different. # Lines are different.
return '- %s\n+ %s' % (short_line_output(line1), short_line_output(line2)) return (
'- %s\n+ %s' % (short_line_output(line1), short_line_output(line2)),
source,
)
# No difference found. # No difference found.
return None return None, source
def get_suppression(arch1, config1, arch2, config2): def get_suppression(arch1, config1, arch2, config2):
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment