Changeset 122505 in webkit


Ignore:
Timestamp:
Jul 12, 2012 2:22:13 PM (12 years ago)
Author:
dpranke@chromium.org
Message:

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.

Tools:

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):

LayoutTests:

Remove tests for the 'ref_file' parameter in results.json
entries.

  • fast/harness/resources/results-test.js:
  • fast/harness/results.html:
Location:
trunk
Files:
8 edited

Legend:

Unmodified
Added
Removed
  • trunk/LayoutTests/ChangeLog

    r122504 r122505  
     12012-07-12  Dirk Pranke  <dpranke@chromium.org>
     2
     3        nrwt crashes saving the output for a platform-specific expected test reference
     4        https://bugs.webkit.org/show_bug.cgi?id=90872
     5
     6        Reviewed by Ojan Vafai.
     7
     8        Remove tests for the 'ref_file' parameter in results.json
     9        entries.
     10
     11        * fast/harness/resources/results-test.js:
     12        * fast/harness/results.html:
     13
    1142012-07-12  Adam Barth  <abarth@webkit.org>
    215
  • trunk/LayoutTests/fast/harness/resources/results-test.js

    r120736 r122505  
    99
    1010var g_testIndex = 0;
    11 var g_log = ["You should see a serios of PASS lines."];
     11var g_log = ["You should see a series of PASS lines."];
    1212
    1313// Make async actually be sync for the sake of simpler testing.
     
    275275    runSingleRowTest(results, false, '', 'ref mismatch html actual ');
    276276
    277     results = mockResults();
    278     results.tests['bar-reftest.html'] = mockExpectation('PASS', 'IMAGE', 1);
    279     results.tests['bar-reftest.html'].is_reftest = true;
    280     results.tests['bar-reftest.html'].ref_file = 'common.html';
    281     runSingleRowTest(results, false, '', 'ref html images diff (1%) ');
    282     runTest(results, function() {
    283         assertTrue(document.getElementsByClassName('result-link')[0].getAttribute('href') == 'common.html');
    284     });
    285 
    286     results = mockResults();
    287     results.tests['bar-reftest.html'] = mockExpectation('PASS', 'IMAGE');
    288     results.tests['bar-reftest.html'].is_mismatch_reftest = true;
    289     results.tests['bar-reftest.html'].ref_file = 'common.html';
    290     runSingleRowTest(results, false, '', 'ref mismatch html actual ');
    291     runTest(results, function() {
    292         assertTrue(document.getElementsByClassName('result-link')[0].getAttribute('href') == 'common.html');
    293     });
    294277
    295278    results = mockResults();
  • trunk/LayoutTests/fast/harness/results.html

    r120736 r122505  
    436436function resultLink(testPrefix, suffix, contents)
    437437{
    438     return referenceLink(testPrefix, testPrefix + suffix, contents);
    439 }
    440 
    441 function referenceLink(testPrefix, reference_filename, contents)
    442 {
    443     return '<a class=result-link href="' + reference_filename + '" data-prefix="' + testPrefix + '">' + contents + '</a> ';
     438    return '<a class=result-link href="' + testPrefix + suffix + '" data-prefix="' + testPrefix + '">' + contents + '</a> ';
    444439}
    445440
     
    578573
    579574        if (testObject.is_mismatch_reftest) {
    580             if (testObject.ref_file)
    581                 row += referenceLink(test_prefix, testObject.ref_file, 'ref mismatch html');
    582             else
    583                 row += resultLink(test_prefix, '-expected-mismatch.html', 'ref mismatch html');
     575            row += resultLink(test_prefix, '-expected-mismatch.html', 'ref mismatch html');
    584576            row += resultLink(test_prefix, '-actual.png', 'actual');
    585577        } else {
    586578            if (testObject.is_reftest) {
    587                 if (testObject.ref_file)
    588                     row += referenceLink(test_prefix, testObject.ref_file, 'ref html');
    589                 else
    590                     row += resultLink(test_prefix, '-expected.html', 'ref html');
     579                row += resultLink(test_prefix, '-expected.html', 'ref html');
    591580            }
    592581            if (globalState().shouldToggleImages) {
  • trunk/Tools/ChangeLog

    r122497 r122505  
     12012-07-12  Dirk Pranke  <dpranke@chromium.org>
     2
     3        nrwt crashes saving the output for a platform-specific expected test reference
     4        https://bugs.webkit.org/show_bug.cgi?id=90872
     5
     6        Reviewed by Ojan Vafai.
     7
     8        The expected output for a test is copied alongside the test
     9        itself in the layout-test-results directory; in other words, for
     10        foo/bar-expected.txt sits alongside foo/bar.html even if we're
     11        actually using platform/mac/foo/bar-expected.txt.
     12
     13        Unless the test is a reftest, in which case we would copy the
     14        output to platform/mac/foo/bar-expected.html and set a
     15        'ref_file' parameter in results.json to indicate the path. This
     16        can be useful in the cases where we have multiple references for
     17        a single test or when multiple tests share the same reference.
     18
     19        We found a bug where we weren't creating platform/mac/foo under
     20        the results directory, and so this wasn't actually working.
     21        However, treating reftests differently seems like a bad thing,
     22        so we should probably be consistent. This change puts the
     23        -expected.html next to the test, and reworks test_result_writer
     24        so that we create directories uniformly and consistently.
     25
     26        Note that we weren't catching this problem in unit tests because
     27        the MockFileSystem creates a directory automatically if it
     28        doesn't exist; this was done intentionally for convenience, but
     29        is really a bug and should be fixed; see https://bugs.webkit.org/show_bug.cgi?id=91028.
     30
     31        I have not added additional tests here since fixing that bug
     32        should be sufficient.
     33
     34        * Scripts/webkitpy/layout_tests/controllers/manager.py:
     35        (interpret_test_failures):
     36        * Scripts/webkitpy/layout_tests/controllers/manager_unittest.py:
     37        (ResultSummaryTest.test_interpret_test_failures):
     38        * Scripts/webkitpy/layout_tests/controllers/test_result_writer.py:
     39        (write_test_result):
     40        (TestResultWriter._write_binary_file):
     41        (TestResultWriter):
     42        (TestResultWriter._write_text_file):
     43        (TestResultWriter.write_output_files):
     44        (TestResultWriter.write_stderr):
     45        (TestResultWriter.write_crash_log):
     46        (TestResultWriter.create_text_diff_and_write_result):
     47        (TestResultWriter.write_image_diff_files):
     48        (write_reftest):
     49        * Scripts/webkitpy/layout_tests/run_webkit_tests_integrationtest.py:
     50        (EndToEndTest.test_reftest_with_two_notrefs):
     51
    1522012-07-12  Dirk Pranke  <dpranke@chromium.org>
    253
  • trunk/Tools/Scripts/webkitpy/layout_tests/controllers/manager.py

    r122394 r122505  
    8989        elif isinstance(failure, test_failures.FailureReftestMismatch):
    9090            test_dict['is_reftest'] = True
    91             test_dict['ref_file'] = port.relative_test_filename(failure.reference_filename)
    9291            test_dict['image_diff_percent'] = failure.diff_percent
    9392        elif isinstance(failure, test_failures.FailureReftestMismatchDidNotOccur):
    9493            test_dict['is_mismatch_reftest'] = True
    95             test_dict['ref_file'] = port.relative_test_filename(failure.reference_filename)
    9694
    9795    if test_failures.FailureMissingResult in failure_types:
  • trunk/Tools/Scripts/webkitpy/layout_tests/controllers/manager_unittest.py

    r120864 r122505  
    453453        self.assertTrue('is_reftest' in test_dict)
    454454        self.assertFalse('is_mismatch_reftest' in test_dict)
    455         self.assertEqual(test_dict['ref_file'], 'foo/common.html')
    456455
    457456        test_dict = interpret_test_failures(self.port, 'foo/reftest.html',
     
    464463        self.assertFalse('is_reftest' in test_dict)
    465464        self.assertTrue(test_dict['is_mismatch_reftest'])
    466         self.assertEqual(test_dict['ref_file'], 'foo/common.html')
    467465
    468466    def get_result(self, test_name, result_type=test_expectations.PASS, run_time=0):
  • trunk/Tools/Scripts/webkitpy/layout_tests/controllers/test_result_writer.py

    r120865 r122505  
    4141    root_output_dir = port.results_directory()
    4242    writer = TestResultWriter(filesystem, port, root_output_dir, test_name)
     43
    4344    if driver_output.error:
    4445        writer.write_stderr(driver_output.error)
     
    7576                else:
    7677                    _log.warn('Can not get image diff. ImageDiff program might not work correctly.')
    77             writer.copy_file(failure.reference_filename)
     78            writer.write_reftest(failure.reference_filename)
    7879        elif isinstance(failure, test_failures.FailureReftestMismatchDidNotOccur):
    7980            writer.write_image_files(driver_output.image, expected_image=None)
    80             writer.copy_file(failure.reference_filename)
     81            writer.write_reftest(failure.reference_filename)
    8182        else:
    8283            assert isinstance(failure, (test_failures.FailureTimeout, test_failures.FailureReftestNoImagesGenerated))
     
    125126        return fs.splitext(output_filename)[0] + modifier
    126127
     128    def _write_binary_file(self, path, contents):
     129        if contents is not None:
     130            self._make_output_directory()
     131            self._filesystem.write_binary_file(path, contents)
     132
     133    def _write_text_file(self, path, contents):
     134        if contents is not None:
     135            self._make_output_directory()
     136            self._filesystem.write_text_file(path, contents)
     137
    127138    def _output_testname(self, modifier):
    128139        fs = self._filesystem
     
    142153          expected: A string containing the expected test output
    143154        """
    144         self._make_output_directory()
    145155        actual_filename = self.output_filename(self.FILENAME_SUFFIX_ACTUAL + file_type)
    146156        expected_filename = self.output_filename(self.FILENAME_SUFFIX_EXPECTED + file_type)
    147157
    148         fs = self._filesystem
    149         if output is not None:
    150             fs.write_binary_file(actual_filename, output)
    151         if expected is not None:
    152             fs.write_binary_file(expected_filename, expected)
     158        self._write_binary_file(actual_filename, output)
     159        self._write_binary_file(expected_filename, expected)
    153160
    154161    def write_stderr(self, error):
    155         fs = self._filesystem
    156162        filename = self.output_filename(self.FILENAME_SUFFIX_STDERR + ".txt")
    157         fs.maybe_make_directory(fs.dirname(filename))
    158         fs.write_binary_file(filename, error)
     163        self._write_text_file(filename, error)
    159164
    160165    def write_crash_log(self, crash_log):
    161         fs = self._filesystem
    162166        filename = self.output_filename(self.FILENAME_SUFFIX_CRASH_LOG + ".txt")
    163         fs.maybe_make_directory(fs.dirname(filename))
    164         if crash_log is not None:
    165             fs.write_text_file(filename, crash_log)
     167        self._write_text_file(filename, crash_log)
    166168
    167169    def write_text_files(self, actual_text, expected_text):
     
    174176            return
    175177
    176         self._make_output_directory()
    177178        file_type = '.txt'
    178179        actual_filename = self.output_filename(self.FILENAME_SUFFIX_ACTUAL + file_type)
    179180        expected_filename = self.output_filename(self.FILENAME_SUFFIX_EXPECTED + file_type)
    180         fs = self._filesystem
    181181        # We treat diff output as binary. Diff output may contain multiple files
    182182        # in conflicting encodings.
    183183        diff = self._port.diff_text(expected_text, actual_text, expected_filename, actual_filename)
    184184        diff_filename = self.output_filename(self.FILENAME_SUFFIX_DIFF + file_type)
    185         fs.write_binary_file(diff_filename, diff)
     185        self._write_binary_file(diff_filename, diff)
    186186
    187187        # Shell out to wdiff to get colored inline diffs.
     
    189189            wdiff = self._port.wdiff_text(expected_filename, actual_filename)
    190190            wdiff_filename = self.output_filename(self.FILENAME_SUFFIX_WDIFF)
    191             fs.write_binary_file(wdiff_filename, wdiff)
     191            self._write_binary_file(wdiff_filename, wdiff)
    192192
    193193        # Use WebKit's PrettyPatch.rb to get an HTML diff.
     
    195195            pretty_patch = self._port.pretty_patch_text(diff_filename)
    196196            pretty_patch_filename = self.output_filename(self.FILENAME_SUFFIX_PRETTY_PATCH)
    197             fs.write_binary_file(pretty_patch_filename, pretty_patch)
     197            self._write_binary_file(pretty_patch_filename, pretty_patch)
    198198
    199199    def write_audio_files(self, actual_audio, expected_audio):
     
    205205    def write_image_diff_files(self, image_diff):
    206206        diff_filename = self.output_filename(self.FILENAME_SUFFIX_IMAGE_DIFF)
    207         fs = self._filesystem
    208         fs.write_binary_file(diff_filename, image_diff)
     207        self._write_binary_file(diff_filename, image_diff)
    209208
    210209        diffs_html_filename = self.output_filename(self.FILENAME_SUFFIX_IMAGE_DIFFS_HTML)
     
    264263        self._filesystem.write_text_file(diffs_html_filename, html)
    265264
    266     def copy_file(self, src_filepath):
    267         fs = self._filesystem
    268         assert fs.exists(src_filepath), 'src_filepath: %s' % src_filepath
    269         dst_filepath = fs.join(self._root_output_dir, self._port.relative_test_filename(src_filepath))
    270         self._make_output_directory()
    271         fs.copyfile(src_filepath, dst_filepath)
     265    def write_reftest(self, src_filepath):
     266        fs = self._filesystem
     267        dst_dir = fs.dirname(fs.join(self._root_output_dir, self._test_name))
     268        dst_filepath = fs.join(dst_dir, fs.basename(src_filepath))
     269        self._write_text_file(dst_filepath, fs.read_text_file(src_filepath))
  • trunk/Tools/Scripts/webkitpy/layout_tests/run_webkit_tests_integrationtest.py

    r121881 r122505  
    952952        self.assertTrue("multiple-both-success.html" not in json["tests"]["reftests"]["foo"])
    953953        self.assertEqual(json["tests"]["reftests"]["foo"]["multiple-match-failure.html"],
    954             {"expected": "PASS", "ref_file": "reftests/foo/second-mismatching-ref.html", "actual": "IMAGE", "image_diff_percent": 1, 'is_reftest': True})
     954            {"expected": "PASS", "actual": "IMAGE", "image_diff_percent": 1, 'is_reftest': True})
    955955        self.assertEqual(json["tests"]["reftests"]["foo"]["multiple-mismatch-failure.html"],
    956             {"expected": "PASS", "ref_file": "reftests/foo/matching-ref.html", "actual": "IMAGE", "is_mismatch_reftest": True})
     956            {"expected": "PASS", "actual": "IMAGE", "is_mismatch_reftest": True})
    957957        self.assertEqual(json["tests"]["reftests"]["foo"]["multiple-both-failure.html"],
    958             {"expected": "PASS", "ref_file": "reftests/foo/matching-ref.html", "actual": "IMAGE", "is_mismatch_reftest": True})
     958            {"expected": "PASS", "actual": "IMAGE", "is_mismatch_reftest": True})
    959959
    960960
Note: See TracChangeset for help on using the changeset viewer.