Diff
Modified: trunk/LayoutTests/ChangeLog (122504 => 122505)
--- trunk/LayoutTests/ChangeLog 2012-07-12 21:09:59 UTC (rev 122504)
+++ trunk/LayoutTests/ChangeLog 2012-07-12 21:22:13 UTC (rev 122505)
@@ -1,3 +1,16 @@
+2012-07-12 Dirk Pranke <dpra...@chromium.org>
+
+ nrwt crashes saving the output for a platform-specific expected test reference
+ https://bugs.webkit.org/show_bug.cgi?id=90872
+
+ Reviewed by Ojan Vafai.
+
+ Remove tests for the 'ref_file' parameter in results.json
+ entries.
+
+ * fast/harness/resources/results-test.js:
+ * fast/harness/results.html:
+
2012-07-12 Adam Barth <aba...@webkit.org>
Unreviewed. Delete two empty directories.
Modified: trunk/LayoutTests/fast/harness/resources/results-test.js (122504 => 122505)
--- trunk/LayoutTests/fast/harness/resources/results-test.js 2012-07-12 21:09:59 UTC (rev 122504)
+++ trunk/LayoutTests/fast/harness/resources/results-test.js 2012-07-12 21:22:13 UTC (rev 122505)
@@ -8,7 +8,7 @@
document.querySelector('head').appendChild(testStyles);
var g_testIndex = 0;
-var g_log = ["You should see a serios of PASS lines."];
+var g_log = ["You should see a series of PASS lines."];
// Make async actually be sync for the sake of simpler testing.
function async(func, args)
@@ -274,25 +274,8 @@
results.tests['bar-reftest-mismatch.html'].is_mismatch_reftest = true;
runSingleRowTest(results, false, '', 'ref mismatch html actual ');
- results = mockResults();
- results.tests['bar-reftest.html'] = mockExpectation('PASS', 'IMAGE', 1);
- results.tests['bar-reftest.html'].is_reftest = true;
- results.tests['bar-reftest.html'].ref_file = 'common.html';
- runSingleRowTest(results, false, '', 'ref html images diff (1%) ');
- runTest(results, function() {
- assertTrue(document.getElementsByClassName('result-link')[0].getAttribute('href') == 'common.html');
- });
results = mockResults();
- results.tests['bar-reftest.html'] = mockExpectation('PASS', 'IMAGE');
- results.tests['bar-reftest.html'].is_mismatch_reftest = true;
- results.tests['bar-reftest.html'].ref_file = 'common.html';
- runSingleRowTest(results, false, '', 'ref mismatch html actual ');
- runTest(results, function() {
- assertTrue(document.getElementsByClassName('result-link')[0].getAttribute('href') == 'common.html');
- });
-
- results = mockResults();
var subtree = results.tests['foo'] = {}
subtree['bar-flaky-pass.html'] = mockExpectation('PASS TEXT', 'PASS');
runTest(results, function() {
Modified: trunk/LayoutTests/fast/harness/results.html (122504 => 122505)
--- trunk/LayoutTests/fast/harness/results.html 2012-07-12 21:09:59 UTC (rev 122504)
+++ trunk/LayoutTests/fast/harness/results.html 2012-07-12 21:22:13 UTC (rev 122505)
@@ -435,14 +435,9 @@
function resultLink(testPrefix, suffix, contents)
{
- return referenceLink(testPrefix, testPrefix + suffix, contents);
+ return '<a class=result-link href="" + testPrefix + suffix + '" data-prefix="' + testPrefix + '">' + contents + '</a> ';
}
-function referenceLink(testPrefix, reference_filename, contents)
-{
- return '<a class=result-link href="" + reference_filename + '" data-prefix="' + testPrefix + '">' + contents + '</a> ';
-}
-
function isFailureExpected(expected, actual)
{
var isExpected = true;
@@ -577,17 +572,11 @@
globalState().hasImageFailures = true;
if (testObject.is_mismatch_reftest) {
- if (testObject.ref_file)
- row += referenceLink(test_prefix, testObject.ref_file, 'ref mismatch html');
- else
- row += resultLink(test_prefix, '-expected-mismatch.html', 'ref mismatch html');
+ row += resultLink(test_prefix, '-expected-mismatch.html', 'ref mismatch html');
row += resultLink(test_prefix, '-actual.png', 'actual');
} else {
if (testObject.is_reftest) {
- if (testObject.ref_file)
- row += referenceLink(test_prefix, testObject.ref_file, 'ref html');
- else
- row += resultLink(test_prefix, '-expected.html', 'ref html');
+ row += resultLink(test_prefix, '-expected.html', 'ref html');
}
if (globalState().shouldToggleImages) {
row += resultLink(test_prefix, '-diffs.html', 'images');
Modified: trunk/Tools/ChangeLog (122504 => 122505)
--- trunk/Tools/ChangeLog 2012-07-12 21:09:59 UTC (rev 122504)
+++ trunk/Tools/ChangeLog 2012-07-12 21:22:13 UTC (rev 122505)
@@ -1,5 +1,56 @@
2012-07-12 Dirk Pranke <dpra...@chromium.org>
+ nrwt crashes saving the output for a platform-specific expected test reference
+ https://bugs.webkit.org/show_bug.cgi?id=90872
+
+ Reviewed by Ojan Vafai.
+
+ The expected output for a test is copied alongside the test
+ itself in the layout-test-results directory; in other words, for
+ foo/bar-expected.txt sits alongside foo/bar.html even if we're
+ actually using platform/mac/foo/bar-expected.txt.
+
+ Unless the test is a reftest, in which case we would copy the
+ output to platform/mac/foo/bar-expected.html and set a
+ 'ref_file' parameter in results.json to indicate the path. This
+ can be useful in the cases where we have multiple references for
+ a single test or when multiple tests share the same reference.
+
+ We found a bug where we weren't creating platform/mac/foo under
+ the results directory, and so this wasn't actually working.
+ However, treating reftests differently seems like a bad thing,
+ so we should probably be consistent. This change puts the
+ -expected.html next to the test, and reworks test_result_writer
+ so that we create directories uniformly and consistently.
+
+ Note that we weren't catching this problem in unit tests because
+ the MockFileSystem creates a directory automatically if it
+ doesn't exist; this was done intentionally for convenience, but
+ is really a bug and should be fixed; see https://bugs.webkit.org/show_bug.cgi?id=91028.
+
+ I have not added additional tests here since fixing that bug
+ should be sufficient.
+
+ * Scripts/webkitpy/layout_tests/controllers/manager.py:
+ (interpret_test_failures):
+ * Scripts/webkitpy/layout_tests/controllers/manager_unittest.py:
+ (ResultSummaryTest.test_interpret_test_failures):
+ * Scripts/webkitpy/layout_tests/controllers/test_result_writer.py:
+ (write_test_result):
+ (TestResultWriter._write_binary_file):
+ (TestResultWriter):
+ (TestResultWriter._write_text_file):
+ (TestResultWriter.write_output_files):
+ (TestResultWriter.write_stderr):
+ (TestResultWriter.write_crash_log):
+ (TestResultWriter.create_text_diff_and_write_result):
+ (TestResultWriter.write_image_diff_files):
+ (write_reftest):
+ * Scripts/webkitpy/layout_tests/run_webkit_tests_integrationtest.py:
+ (EndToEndTest.test_reftest_with_two_notrefs):
+
+2012-07-12 Dirk Pranke <dpra...@chromium.org>
+
nrwt: reimplement manager_worker_broker in a much simpler form
https://bugs.webkit.org/show_bug.cgi?id=90513
Modified: trunk/Tools/Scripts/webkitpy/layout_tests/controllers/manager.py (122504 => 122505)
--- trunk/Tools/Scripts/webkitpy/layout_tests/controllers/manager.py 2012-07-12 21:09:59 UTC (rev 122504)
+++ trunk/Tools/Scripts/webkitpy/layout_tests/controllers/manager.py 2012-07-12 21:22:13 UTC (rev 122505)
@@ -88,11 +88,9 @@
test_dict['image_diff_percent'] = failure.diff_percent
elif isinstance(failure, test_failures.FailureReftestMismatch):
test_dict['is_reftest'] = True
- test_dict['ref_file'] = port.relative_test_filename(failure.reference_filename)
test_dict['image_diff_percent'] = failure.diff_percent
elif isinstance(failure, test_failures.FailureReftestMismatchDidNotOccur):
test_dict['is_mismatch_reftest'] = True
- test_dict['ref_file'] = port.relative_test_filename(failure.reference_filename)
if test_failures.FailureMissingResult in failure_types:
test_dict['is_missing_text'] = True
Modified: trunk/Tools/Scripts/webkitpy/layout_tests/controllers/manager_unittest.py (122504 => 122505)
--- trunk/Tools/Scripts/webkitpy/layout_tests/controllers/manager_unittest.py 2012-07-12 21:09:59 UTC (rev 122504)
+++ trunk/Tools/Scripts/webkitpy/layout_tests/controllers/manager_unittest.py 2012-07-12 21:22:13 UTC (rev 122505)
@@ -452,7 +452,6 @@
[test_failures.FailureReftestMismatch(self.port.abspath_for_test('foo/common.html'))])
self.assertTrue('is_reftest' in test_dict)
self.assertFalse('is_mismatch_reftest' in test_dict)
- self.assertEqual(test_dict['ref_file'], 'foo/common.html')
test_dict = interpret_test_failures(self.port, 'foo/reftest.html',
[test_failures.FailureReftestMismatchDidNotOccur(self.port.abspath_for_test('foo/reftest-expected-mismatch.html'))])
@@ -463,7 +462,6 @@
[test_failures.FailureReftestMismatchDidNotOccur(self.port.abspath_for_test('foo/common.html'))])
self.assertFalse('is_reftest' in test_dict)
self.assertTrue(test_dict['is_mismatch_reftest'])
- self.assertEqual(test_dict['ref_file'], 'foo/common.html')
def get_result(self, test_name, result_type=test_expectations.PASS, run_time=0):
failures = []
Modified: trunk/Tools/Scripts/webkitpy/layout_tests/controllers/test_result_writer.py (122504 => 122505)
--- trunk/Tools/Scripts/webkitpy/layout_tests/controllers/test_result_writer.py 2012-07-12 21:09:59 UTC (rev 122504)
+++ trunk/Tools/Scripts/webkitpy/layout_tests/controllers/test_result_writer.py 2012-07-12 21:22:13 UTC (rev 122505)
@@ -40,6 +40,7 @@
"""Write the test result to the result output directory."""
root_output_dir = port.results_directory()
writer = TestResultWriter(filesystem, port, root_output_dir, test_name)
+
if driver_output.error:
writer.write_stderr(driver_output.error)
@@ -74,10 +75,10 @@
failure.diff_percent = diff_percent
else:
_log.warn('Can not get image diff. ImageDiff program might not work correctly.')
- writer.copy_file(failure.reference_filename)
+ writer.write_reftest(failure.reference_filename)
elif isinstance(failure, test_failures.FailureReftestMismatchDidNotOccur):
writer.write_image_files(driver_output.image, expected_image=None)
- writer.copy_file(failure.reference_filename)
+ writer.write_reftest(failure.reference_filename)
else:
assert isinstance(failure, (test_failures.FailureTimeout, test_failures.FailureReftestNoImagesGenerated))
@@ -124,6 +125,16 @@
output_filename = fs.join(self._root_output_dir, self._test_name)
return fs.splitext(output_filename)[0] + modifier
+ def _write_binary_file(self, path, contents):
+ if contents is not None:
+ self._make_output_directory()
+ self._filesystem.write_binary_file(path, contents)
+
+ def _write_text_file(self, path, contents):
+ if contents is not None:
+ self._make_output_directory()
+ self._filesystem.write_text_file(path, contents)
+
def _output_testname(self, modifier):
fs = self._filesystem
return fs.splitext(fs.basename(self._test_name))[0] + modifier
@@ -141,28 +152,19 @@
output: A string containing the test output
expected: A string containing the expected test output
"""
- self._make_output_directory()
actual_filename = self.output_filename(self.FILENAME_SUFFIX_ACTUAL + file_type)
expected_filename = self.output_filename(self.FILENAME_SUFFIX_EXPECTED + file_type)
- fs = self._filesystem
- if output is not None:
- fs.write_binary_file(actual_filename, output)
- if expected is not None:
- fs.write_binary_file(expected_filename, expected)
+ self._write_binary_file(actual_filename, output)
+ self._write_binary_file(expected_filename, expected)
def write_stderr(self, error):
- fs = self._filesystem
filename = self.output_filename(self.FILENAME_SUFFIX_STDERR + ".txt")
- fs.maybe_make_directory(fs.dirname(filename))
- fs.write_binary_file(filename, error)
+ self._write_text_file(filename, error)
def write_crash_log(self, crash_log):
- fs = self._filesystem
filename = self.output_filename(self.FILENAME_SUFFIX_CRASH_LOG + ".txt")
- fs.maybe_make_directory(fs.dirname(filename))
- if crash_log is not None:
- fs.write_text_file(filename, crash_log)
+ self._write_text_file(filename, crash_log)
def write_text_files(self, actual_text, expected_text):
self.write_output_files(".txt", actual_text, expected_text)
@@ -173,28 +175,26 @@
if not actual_text or not expected_text:
return
- self._make_output_directory()
file_type = '.txt'
actual_filename = self.output_filename(self.FILENAME_SUFFIX_ACTUAL + file_type)
expected_filename = self.output_filename(self.FILENAME_SUFFIX_EXPECTED + file_type)
- fs = self._filesystem
# We treat diff output as binary. Diff output may contain multiple files
# in conflicting encodings.
diff = self._port.diff_text(expected_text, actual_text, expected_filename, actual_filename)
diff_filename = self.output_filename(self.FILENAME_SUFFIX_DIFF + file_type)
- fs.write_binary_file(diff_filename, diff)
+ self._write_binary_file(diff_filename, diff)
# Shell out to wdiff to get colored inline diffs.
if self._port.wdiff_available():
wdiff = self._port.wdiff_text(expected_filename, actual_filename)
wdiff_filename = self.output_filename(self.FILENAME_SUFFIX_WDIFF)
- fs.write_binary_file(wdiff_filename, wdiff)
+ self._write_binary_file(wdiff_filename, wdiff)
# Use WebKit's PrettyPatch.rb to get an HTML diff.
if self._port.pretty_patch_available():
pretty_patch = self._port.pretty_patch_text(diff_filename)
pretty_patch_filename = self.output_filename(self.FILENAME_SUFFIX_PRETTY_PATCH)
- fs.write_binary_file(pretty_patch_filename, pretty_patch)
+ self._write_binary_file(pretty_patch_filename, pretty_patch)
def write_audio_files(self, actual_audio, expected_audio):
self.write_output_files('.wav', actual_audio, expected_audio)
@@ -204,8 +204,7 @@
def write_image_diff_files(self, image_diff):
diff_filename = self.output_filename(self.FILENAME_SUFFIX_IMAGE_DIFF)
- fs = self._filesystem
- fs.write_binary_file(diff_filename, image_diff)
+ self._write_binary_file(diff_filename, image_diff)
diffs_html_filename = self.output_filename(self.FILENAME_SUFFIX_IMAGE_DIFFS_HTML)
# FIXME: old-run-webkit-tests shows the diff percentage as the text contents of the "diff" link.
@@ -263,9 +262,8 @@
}
self._filesystem.write_text_file(diffs_html_filename, html)
- def copy_file(self, src_filepath):
+ def write_reftest(self, src_filepath):
fs = self._filesystem
- assert fs.exists(src_filepath), 'src_filepath: %s' % src_filepath
- dst_filepath = fs.join(self._root_output_dir, self._port.relative_test_filename(src_filepath))
- self._make_output_directory()
- fs.copyfile(src_filepath, dst_filepath)
+ dst_dir = fs.dirname(fs.join(self._root_output_dir, self._test_name))
+ dst_filepath = fs.join(dst_dir, fs.basename(src_filepath))
+ self._write_text_file(dst_filepath, fs.read_text_file(src_filepath))
Modified: trunk/Tools/Scripts/webkitpy/layout_tests/run_webkit_tests_integrationtest.py (122504 => 122505)
--- trunk/Tools/Scripts/webkitpy/layout_tests/run_webkit_tests_integrationtest.py 2012-07-12 21:09:59 UTC (rev 122504)
+++ trunk/Tools/Scripts/webkitpy/layout_tests/run_webkit_tests_integrationtest.py 2012-07-12 21:22:13 UTC (rev 122505)
@@ -951,11 +951,11 @@
self.assertTrue("multiple-mismatch-success.html" not in json["tests"]["reftests"]["foo"])
self.assertTrue("multiple-both-success.html" not in json["tests"]["reftests"]["foo"])
self.assertEqual(json["tests"]["reftests"]["foo"]["multiple-match-failure.html"],
- {"expected": "PASS", "ref_file": "reftests/foo/second-mismatching-ref.html", "actual": "IMAGE", "image_diff_percent": 1, 'is_reftest': True})
+ {"expected": "PASS", "actual": "IMAGE", "image_diff_percent": 1, 'is_reftest': True})
self.assertEqual(json["tests"]["reftests"]["foo"]["multiple-mismatch-failure.html"],
- {"expected": "PASS", "ref_file": "reftests/foo/matching-ref.html", "actual": "IMAGE", "is_mismatch_reftest": True})
+ {"expected": "PASS", "actual": "IMAGE", "is_mismatch_reftest": True})
self.assertEqual(json["tests"]["reftests"]["foo"]["multiple-both-failure.html"],
- {"expected": "PASS", "ref_file": "reftests/foo/matching-ref.html", "actual": "IMAGE", "is_mismatch_reftest": True})
+ {"expected": "PASS", "actual": "IMAGE", "is_mismatch_reftest": True})
class RebaselineTest(unittest.TestCase, StreamTestingMixin):
