Diff
Modified: trunk/LayoutTests/ChangeLog (96271 => 96272)
--- trunk/LayoutTests/ChangeLog 2011-09-29 00:09:02 UTC (rev 96271)
+++ trunk/LayoutTests/ChangeLog 2011-09-29 00:09:39 UTC (rev 96272)
@@ -1,3 +1,17 @@
+2011-09-28 Dirk Pranke <[email protected]>
+
+ Modify new-run-webkit-tests to return the percentage difference in image diffs where possible.
+
+ https://bugs.webkit.org/show_bug.cgi?id=67253
+
+ Reviewed by Eric Seidel.
+
+ * fast/harness/resources/results-test.js:
+ (mockExpectation):
+ (runDefaultSingleRowTest):
+ ():
+ * fast/harness/results.html:
+
2011-09-28 Chris Rogers <[email protected]>
DelayNode must set the context on delayTime AudioParam to support automation
Modified: trunk/LayoutTests/fast/harness/resources/results-test.js (96271 => 96272)
--- trunk/LayoutTests/fast/harness/resources/results-test.js 2011-09-29 00:09:02 UTC (rev 96271)
+++ trunk/LayoutTests/fast/harness/resources/results-test.js 2011-09-29 00:09:39 UTC (rev 96272)
@@ -34,12 +34,14 @@
};
}
-function mockExpectation(expected, actual)
+function mockExpectation(expected, actual, diff_percentage)
{
+ diff_percentage = (typeof(diff_percentage) == undefined) ? 0 : diff_percentage;
return {
expected: expected,
time_ms: 1,
actual: actual,
+ image_diff_percent: diff_percentage,
has_stderr: false
};
}
@@ -83,10 +85,10 @@
}
}
-function runDefaultSingleRowTest(test, expected, actual, isExpected, textResults, imageResults)
+function runDefaultSingleRowTest(test, expected, actual, diff_percentage, isExpected, textResults, imageResults)
{
results = mockResults();
- results.tests[test] = mockExpectation(expected, actual);
+ results.tests[test] = mockExpectation(expected, actual, diff_percentage);
runSingleRowTest(results, isExpected, textResults, imageResults);
}
@@ -245,17 +247,17 @@
assertTrue(window.getComputedStyle(document.getElementById('results-table').parentNode, null)['display'] == 'none');
});
- runDefaultSingleRowTest('bar-skip.html', 'TEXT', 'SKIP', true, '', '');
- runDefaultSingleRowTest('bar-flaky-fail.html', 'PASS FAIL', 'TEXT', true, 'expected actual diff ', '');
- runDefaultSingleRowTest('bar-flaky-fail-unexpected.html', 'PASS TEXT', 'IMAGE', false, '', 'images diff ');
- runDefaultSingleRowTest('bar-audio.html', 'TEXT', 'AUDIO', false, 'expected audio actual audio ', '');
- runDefaultSingleRowTest('bar-image.html', 'TEXT', 'IMAGE', false, '', 'images diff ');
- runDefaultSingleRowTest('bar-image-plus-text.html', 'TEXT', 'IMAGE+TEXT', false, 'expected actual diff ', 'images diff ');
+ runDefaultSingleRowTest('bar-skip.html', 'TEXT', 'SKIP', 0, true, '', '');
+ runDefaultSingleRowTest('bar-flaky-fail.html', 'PASS FAIL', 'TEXT', 0, true, 'expected actual diff ', '');
+ runDefaultSingleRowTest('bar-flaky-fail-unexpected.html', 'PASS TEXT', 'IMAGE', 1, false, '', 'images diff (1%) ');
+ runDefaultSingleRowTest('bar-audio.html', 'TEXT', 'AUDIO', 0, false, 'expected audio actual audio ', '');
+ runDefaultSingleRowTest('bar-image.html', 'TEXT', 'IMAGE', 1, false, '', 'images diff (1%) ');
+ runDefaultSingleRowTest('bar-image-plus-text.html', 'TEXT', 'IMAGE+TEXT', 1, false, 'expected actual diff ', 'images diff (1%) ');
results = mockResults();
- results.tests['bar-reftest.html'] = mockExpectation('PASS', 'IMAGE');
+ results.tests['bar-reftest.html'] = mockExpectation('PASS', 'IMAGE', 1);
results.tests['bar-reftest.html'].is_reftest = true;
- runSingleRowTest(results, false, '', 'ref html images diff ');
+ runSingleRowTest(results, false, '', 'ref html images diff (1%) ');
results = mockResults();
results.tests['bar-reftest-mismatch.html'] = mockExpectation('PASS', 'IMAGE');
@@ -410,26 +412,26 @@
shouldUseTracLinks = oldShouldUseTracLinks;
results = mockResults();
- results.tests['bar.html'] = mockExpectation('PASS', 'IMAGE');
+ results.tests['bar.html'] = mockExpectation('PASS', 'IMAGE', 1);
runTest(results, function() {
- assertTrue(document.querySelector('tbody td:nth-child(3)').textContent == 'images diff ');
+ assertTrue(document.querySelector('tbody td:nth-child(3)').textContent == 'images diff (1%) ');
document.getElementById('toggle-images').checked = false;
// FIXME: We shouldn't need to call updateTogglingImages. Setting checked above should call it.
updateTogglingImages();
// FIXME: We get extra spaces in the DOM every time we enable/disable image toggling.
- assertTrue(document.querySelector('tbody td:nth-child(3)').textContent == 'expected actual diff ');
+ assertTrue(document.querySelector('tbody td:nth-child(3)').textContent == 'expected actual diff (1%) ');
document.getElementById('toggle-images').checked = true;
updateTogglingImages();
- assertTrue(document.querySelector('tbody td:nth-child(3)').textContent == ' images diff ');
+ assertTrue(document.querySelector('tbody td:nth-child(3)').textContent == ' images diff (1%) ');
});
results = mockResults();
- results.tests['reading-options-from-localstorage.html'] = mockExpectation('IMAGE+TEXT', 'IMAGE+TEXT');
+ results.tests['reading-options-from-localstorage.html'] = mockExpectation('IMAGE+TEXT', 'IMAGE+TEXT', 1);
runTest(results, function() {
assertTrue(window.getComputedStyle(document.querySelector('tbody'), null)['display'] != 'none');
- assertTrue(document.querySelector('tbody td:nth-child(3)').textContent == 'expected actual diff ');
+ assertTrue(document.querySelector('tbody td:nth-child(3)').textContent == 'expected actual diff (1%) ');
}, '{"toggle-images":false,"unexpected-results":false}');
function enclosingNodeWithTagNameHasClassName(node, tagName, className) {
Modified: trunk/LayoutTests/fast/harness/results.html (96271 => 96272)
--- trunk/LayoutTests/fast/harness/results.html 2011-09-29 00:09:02 UTC (rev 96271)
+++ trunk/LayoutTests/fast/harness/results.html 2011-09-29 00:09:39 UTC (rev 96272)
@@ -545,8 +545,8 @@
row += resultLink(test_prefix, '-actual.png', 'actual');
}
- // FIXME: old-run-webkit-tests shows the diff percentage as the text contents of the "diff" link.
- row += resultLink(test_prefix, '-diff.png', 'diff');
+ var diff = testObject.image_diff_percent;
+ row += resultLink(test_prefix, '-diff.png', 'diff (' + diff + '%)');
}
}
Modified: trunk/Tools/ChangeLog (96271 => 96272)
--- trunk/Tools/ChangeLog 2011-09-29 00:09:02 UTC (rev 96271)
+++ trunk/Tools/ChangeLog 2011-09-29 00:09:39 UTC (rev 96272)
@@ -1,3 +1,30 @@
+2011-09-28 Dirk Pranke <[email protected]>
+
+ Modify new-run-webkit-tests to return the percentage difference in image diffs where possible.
+
+ https://bugs.webkit.org/show_bug.cgi?id=67253
+
+ Reviewed by Eric Seidel.
+
+ Nearly all of the work in this was done by Simon Fraser; I'm
+ just repackaging it and cleaning it up a bit. This change
+ modifies port.diff_image() to return a tuple of (pass/fail,
+ %age), adds the value to the FailureImageHashMismatch, and
+ writes the value into the full_results.json files to be
+ displayed in the results page.
+
+ * Scripts/webkitpy/layout_tests/controllers/manager.py:
+ * Scripts/webkitpy/layout_tests/controllers/single_test_runner.py:
+ * Scripts/webkitpy/layout_tests/layout_package/test_result_writer.py:
+ * Scripts/webkitpy/layout_tests/models/test_failures.py:
+ * Scripts/webkitpy/layout_tests/port/base.py:
+ * Scripts/webkitpy/layout_tests/port/chromium.py:
+ * Scripts/webkitpy/layout_tests/port/chromium_unittest.py:
+ * Scripts/webkitpy/layout_tests/port/port_testcase.py:
+ * Scripts/webkitpy/layout_tests/port/test.py:
+ * Scripts/webkitpy/layout_tests/port/webkit.py:
+ * Scripts/webkitpy/layout_tests/run_webkit_tests_integrationtest.py:
+
2011-09-25 Kentaro Hara <[email protected]>
Unreviewed. Adding myself to committers.py.
Modified: trunk/Tools/Scripts/webkitpy/layout_tests/controllers/manager.py (96271 => 96272)
--- trunk/Tools/Scripts/webkitpy/layout_tests/controllers/manager.py 2011-09-29 00:09:02 UTC (rev 96271)
+++ trunk/Tools/Scripts/webkitpy/layout_tests/controllers/manager.py 2011-09-29 00:09:39 UTC (rev 96272)
@@ -170,6 +170,8 @@
for f in result.failures:
if 'is_reftest' in result.failures:
test_dict['is_reftest'] = True
+ if type(f) is test_failures.FailureImageHashMismatch:
+ test_dict['image_diff_percent'] = f.diff_percent
if test_failures.FailureReftestMismatchDidNotOccur in failure_types:
test_dict['is_mismatch_reftest'] = True
Modified: trunk/Tools/Scripts/webkitpy/layout_tests/controllers/single_test_runner.py (96271 => 96272)
--- trunk/Tools/Scripts/webkitpy/layout_tests/controllers/single_test_runner.py 2011-09-29 00:09:02 UTC (rev 96271)
+++ trunk/Tools/Scripts/webkitpy/layout_tests/controllers/single_test_runner.py 2011-09-29 00:09:39 UTC (rev 96272)
@@ -276,9 +276,10 @@
elif not expected_driver_output.image_hash:
failures.append(test_failures.FailureMissingImageHash())
elif driver_output.image_hash != expected_driver_output.image_hash:
- driver_output.image_diff = self._port.diff_image(driver_output.image, expected_driver_output.image)
+ diff_result = self._port.diff_image(driver_output.image, expected_driver_output.image)
+ driver_output.image_diff = diff_result[0]
if driver_output.image_diff:
- failures.append(test_failures.FailureImageHashMismatch())
+ failures.append(test_failures.FailureImageHashMismatch(diff_result[1]))
return failures
def _run_reftest(self):
Modified: trunk/Tools/Scripts/webkitpy/layout_tests/layout_package/test_result_writer.py (96271 => 96272)
--- trunk/Tools/Scripts/webkitpy/layout_tests/layout_package/test_result_writer.py 2011-09-29 00:09:02 UTC (rev 96271)
+++ trunk/Tools/Scripts/webkitpy/layout_tests/layout_package/test_result_writer.py 2011-09-29 00:09:39 UTC (rev 96272)
@@ -70,7 +70,7 @@
elif isinstance(failure, test_failures.FailureReftestMismatch):
writer.write_image_files(driver_output.image, expected_driver_output.image)
# FIXME: This work should be done earlier in the pipeline (e.g., when we compare images for non-ref tests).
- image_diff = port.diff_image(driver_output.image, expected_driver_output.image)
+ image_diff = port.diff_image(driver_output.image, expected_driver_output.image)[0]
if image_diff:
writer.write_image_diff_files(image_diff)
writer.copy_file(port.reftest_expected_filename(test_name), '-expected.html')
Modified: trunk/Tools/Scripts/webkitpy/layout_tests/models/test_failures.py (96271 => 96272)
--- trunk/Tools/Scripts/webkitpy/layout_tests/models/test_failures.py 2011-09-29 00:09:02 UTC (rev 96271)
+++ trunk/Tools/Scripts/webkitpy/layout_tests/models/test_failures.py 2011-09-29 00:09:39 UTC (rev 96272)
@@ -167,6 +167,8 @@
class FailureImageHashMismatch(TestFailure):
"""Image hashes didn't match."""
+ def __init__(self, diff_percent=0):
+ self.diff_percent = diff_percent
@staticmethod
def message():
Modified: trunk/Tools/Scripts/webkitpy/layout_tests/port/base.py (96271 => 96272)
--- trunk/Tools/Scripts/webkitpy/layout_tests/port/base.py 2011-09-29 00:09:02 UTC (rev 96271)
+++ trunk/Tools/Scripts/webkitpy/layout_tests/port/base.py 2011-09-29 00:09:39 UTC (rev 96272)
@@ -257,7 +257,7 @@
return expected_audio != actual_audio
def diff_image(self, expected_contents, actual_contents, tolerance=0):
- """Compare two images and return an image diff.
+ """Compare two images and return a tuple of an image diff, and a percentage difference (0-100).
|tolerance| should be a percentage value (0.0 - 100.0).
If it is omitted, the port default tolerance value is used.
Modified: trunk/Tools/Scripts/webkitpy/layout_tests/port/chromium.py (96271 => 96272)
--- trunk/Tools/Scripts/webkitpy/layout_tests/port/chromium.py 2011-09-29 00:09:02 UTC (rev 96271)
+++ trunk/Tools/Scripts/webkitpy/layout_tests/port/chromium.py 2011-09-29 00:09:39 UTC (rev 96272)
@@ -168,11 +168,11 @@
# If only one of them exists, return that one.
if not actual_contents and not expected_contents:
- return None
+ return (None, 0)
if not actual_contents:
- return expected_contents
+ return (expected_contents, 0)
if not expected_contents:
- return actual_contents
+ return (actual_contents, 0)
tempdir = self._filesystem.mkdtemp()
@@ -213,7 +213,7 @@
if exit_code == 1:
result = self._filesystem.read_binary_file(native_diff_filename)
self._filesystem.rmtree(str(tempdir))
- return result
+ return (result, 0) # FIXME: how to get % diff?
def path_from_chromium_base(self, *comps):
"""Returns the full path to path made by joining the top of the
Modified: trunk/Tools/Scripts/webkitpy/layout_tests/port/chromium_unittest.py (96271 => 96272)
--- trunk/Tools/Scripts/webkitpy/layout_tests/port/chromium_unittest.py 2011-09-29 00:09:02 UTC (rev 96271)
+++ trunk/Tools/Scripts/webkitpy/layout_tests/port/chromium_unittest.py 2011-09-29 00:09:39 UTC (rev 96272)
@@ -262,11 +262,11 @@
# Images are different.
port._executive = executive_mock.MockExecutive2(run_command_fn=mock_run_command)
- self.assertEquals(mock_image_diff, port.diff_image("EXPECTED", "ACTUAL"))
+ self.assertEquals(mock_image_diff, port.diff_image("EXPECTED", "ACTUAL")[0])
# Images are the same.
port._executive = executive_mock.MockExecutive2(exit_code=0)
- self.assertEquals(None, port.diff_image("EXPECTED", "ACTUAL"))
+ self.assertEquals(None, port.diff_image("EXPECTED", "ACTUAL")[0])
# There was some error running image_diff.
port._executive = executive_mock.MockExecutive2(exit_code=2)
Modified: trunk/Tools/Scripts/webkitpy/layout_tests/port/port_testcase.py (96271 => 96272)
--- trunk/Tools/Scripts/webkitpy/layout_tests/port/port_testcase.py 2011-09-29 00:09:02 UTC (rev 96271)
+++ trunk/Tools/Scripts/webkitpy/layout_tests/port/port_testcase.py 2011-09-29 00:09:39 UTC (rev 96272)
@@ -226,10 +226,10 @@
tmpfd, tmpfile = port._filesystem.open_binary_tempfile('')
tmpfd.close()
- self.assertFalse(port.diff_image(contents1, contents1))
- self.assertTrue(port.diff_image(contents1, contents2))
+ self.assertFalse(port.diff_image(contents1, contents1)[0])
+ self.assertTrue(port.diff_image(contents1, contents2)[0])
- self.assertTrue(port.diff_image(contents1, contents2, tmpfile))
+ self.assertTrue(port.diff_image(contents1, contents2, tmpfile)[0])
port._filesystem.remove(tmpfile)
@@ -237,24 +237,24 @@
port = self.make_port()
if not port:
return
- self.assertFalse(port.diff_image(None, None))
- self.assertFalse(port.diff_image(None, ''))
- self.assertFalse(port.diff_image('', None))
- self.assertFalse(port.diff_image('', ''))
+ self.assertFalse(port.diff_image(None, None)[0])
+ self.assertFalse(port.diff_image(None, '')[0])
+ self.assertFalse(port.diff_image('', None)[0])
+ self.assertFalse(port.diff_image('', '')[0])
def test_diff_image__missing_actual(self):
port = self.make_port()
if not port:
return
- self.assertTrue(port.diff_image(None, 'foo'))
- self.assertTrue(port.diff_image('', 'foo'))
+ self.assertTrue(port.diff_image(None, 'foo')[0])
+ self.assertTrue(port.diff_image('', 'foo')[0])
def test_diff_image__missing_expected(self):
port = self.make_port()
if not port:
return
- self.assertTrue(port.diff_image('foo', None))
- self.assertTrue(port.diff_image('foo', ''))
+ self.assertTrue(port.diff_image('foo', None)[0])
+ self.assertTrue(port.diff_image('foo', '')[0])
def test_check_build(self):
port = self.make_port()
Modified: trunk/Tools/Scripts/webkitpy/layout_tests/port/test.py (96271 => 96272)
--- trunk/Tools/Scripts/webkitpy/layout_tests/port/test.py 2011-09-29 00:09:02 UTC (rev 96271)
+++ trunk/Tools/Scripts/webkitpy/layout_tests/port/test.py 2011-09-29 00:09:39 UTC (rev 96272)
@@ -325,8 +325,8 @@
def diff_image(self, expected_contents, actual_contents):
diffed = actual_contents != expected_contents
if diffed:
- return "< %s\n---\n> %s\n" % (expected_contents, actual_contents)
- return None
+ return ["< %s\n---\n> %s\n" % (expected_contents, actual_contents), 1]
+ return (None, 0)
def layout_tests_dir(self):
return LAYOUT_TEST_DIR
Modified: trunk/Tools/Scripts/webkitpy/layout_tests/port/webkit.py (96271 => 96272)
--- trunk/Tools/Scripts/webkitpy/layout_tests/port/webkit.py 2011-09-29 00:09:02 UTC (rev 96271)
+++ trunk/Tools/Scripts/webkitpy/layout_tests/port/webkit.py 2011-09-29 00:09:39 UTC (rev 96272)
@@ -151,11 +151,11 @@
# Handle the case where the test didn't actually generate an image.
# FIXME: need unit tests for this.
if not actual_contents and not expected_contents:
- return None
+ return (None, 0)
if not actual_contents or not expected_contents:
# FIXME: It's not clear what we should return in this case.
# Maybe we should throw an exception?
- return True
+ return (True, 0)
process = self._start_image_diff_process(expected_contents, actual_contents)
return self._read_image_diff(process)
@@ -180,12 +180,15 @@
timeout = 2.0
deadline = time.time() + timeout
output = sp.read_line(timeout)
+ output_image = ""
+ diff_percent = 0
while not sp.timed_out and not sp.crashed and output:
if output.startswith('Content-Length'):
m = re.match('Content-Length: (\d+)', output)
content_length = int(m.group(1))
timeout = deadline - time.time()
- output = sp.read(timeout, content_length)
+ output_image = sp.read(timeout, content_length)
+ output = sp.read_line(timeout)
break
elif output.startswith('diff'):
break
@@ -201,8 +204,9 @@
if output.startswith('diff'):
m = re.match('diff: (.+)% (passed|failed)', output)
if m.group(2) == 'passed':
- return None
- return output
+ return [None, 0]
+ diff_percent = float(m.group(1))
+ return (output_image, diff_percent)
def default_results_directory(self):
# Results are store relative to the built products to make it easy
Modified: trunk/Tools/Scripts/webkitpy/layout_tests/run_webkit_tests_integrationtest.py (96271 => 96272)
--- trunk/Tools/Scripts/webkitpy/layout_tests/run_webkit_tests_integrationtest.py 2011-09-29 00:09:02 UTC (rev 96271)
+++ trunk/Tools/Scripts/webkitpy/layout_tests/run_webkit_tests_integrationtest.py 2011-09-29 00:09:39 UTC (rev 96272)
@@ -49,6 +49,12 @@
except ImportError:
multiprocessing = None
+try:
+ import json
+except ImportError:
+ # python 2.5 compatibility
+ import webkitpy.thirdparty.simplejson as json
+
# FIXME: remove this when we fix test-webkitpy to work properly on cygwin
# (bug 63846).
SHOULD_TEST_PROCESSES = multiprocessing and sys.platform not in ('cygwin', 'win32')
@@ -580,7 +586,7 @@
class ImageDiffTestPort(TestPort):
def diff_image(self, expected_contents, actual_contents):
self.tolerance_used_for_diff_image = self._options.tolerance
- return True
+ return (True, 1)
def get_port_for_run(args):
options, parsed_args = run_webkit_tests.parse_args(args)
@@ -675,6 +681,26 @@
MainTest = skip_if(MainTest, sys.platform == 'cygwin' and compare_version(sys, '2.6')[0] < 0, 'new-run-webkit-tests tests hang on Cygwin Python 2.5.2')
+class EndToEndTest(unittest.TestCase):
+ def parse_full_results(self, full_results_text):
+ json_to_eval = full_results_text.replace("ADD_RESULTS(", "").replace(");", "")
+ compressed_results = json.loads(json_to_eval)
+ return compressed_results
+
+ def test_end_to_end(self):
+ fs = unit_test_filesystem()
+ res, out, err, user = logging_run(record_results=True, tests_included=True, filesystem=fs)
+
+ # Six tests should fail, so the return code should be 6.
+ self.assertEquals(res, 6)
+ results = self.parse_full_results(fs.files['/tmp/layout-test-results/full_results.json'])
+
+ # Check to ensure we're passing back image diff %age correctly.
+ self.assertEquals(results['tests']['failures']['expected']['image.html']['image_diff_percent'], 1)
+
+ # Check that we attempted to display the results page in a browser.
+ self.assertTrue(user.opened_urls)
+
class RebaselineTest(unittest.TestCase):
def assertBaselines(self, file_list, file, extensions, err):
"assert that the file_list contains the baselines."""
Modified: trunk/Tools/Scripts/webkitpy/to_be_moved/rebaseline_chromium_webkit_tests.py (96271 => 96272)
--- trunk/Tools/Scripts/webkitpy/to_be_moved/rebaseline_chromium_webkit_tests.py 2011-09-29 00:09:02 UTC (rev 96271)
+++ trunk/Tools/Scripts/webkitpy/to_be_moved/rebaseline_chromium_webkit_tests.py 2011-09-29 00:09:39 UTC (rev 96272)
@@ -514,7 +514,7 @@
"""
if is_image:
- return self._port.diff_image(output1, output2)
+ return self._port.diff_image(output1, output2)[0]
return self._port.compare_text(output1, output2)
@@ -599,7 +599,7 @@
_log.debug(' Html: diffing "%s" and "%s"', old_file, new_file)
old_output = self._filesystem.read_binary_file(old_file)
new_output = self._filesystem.read_binary_file(new_file)
- image_diff = self._port.diff_image(old_output, new_output)
+ image_diff = self._port.diff_image(old_output, new_output)[0]
self._filesystem.write_binary_file(diff_file, image_diff)
if has_diff:
