[webkit-changes] [235564] trunk/Tools

Fri, 31 Aug 2018 13:02:05 -0700

Title: [235564] trunk/Tools
Revision
235564
Author
simon.fra...@apple.com
Date
2018-08-31 13:00:22 -0700 (Fri, 31 Aug 2018)

Log Message

Make it possible to mark tests as leaks in TestExpectations
https://bugs.webkit.org/show_bug.cgi?id=189088

Reviewed by Jon Lee.

Have webkitpy parse out "Leak" expectations in TestExpectations, and do the right
thing if the test run did not use --world-leaks. Add unit tests for leaks combined
with various other result combinations.

* Scripts/webkitpy/layout_tests/controllers/layout_test_runner.py:
(LayoutTestRunner._update_summary_with_result):
(LayoutTestRunner._annotate_results_with_additional_failures):
* Scripts/webkitpy/layout_tests/models/test_expectations.py:
(TestExpectationParser):
(TestExpectations): The 'Leak' line was duplicated here, so remove a copy.
(TestExpectations.remove_leak_failures):
(TestExpectations.matches_an_expected_result):
* Scripts/webkitpy/layout_tests/models/test_expectations_unittest.py:
(Base.get_basic_tests):
* Scripts/webkitpy/port/test.py:
(TestList.add_reftest):
(unit_test_list):

Modified Paths

Diff

Modified: trunk/Tools/ChangeLog (235563 => 235564)


--- trunk/Tools/ChangeLog	2018-08-31 19:13:04 UTC (rev 235563)
+++ trunk/Tools/ChangeLog	2018-08-31 20:00:22 UTC (rev 235564)
@@ -1,3 +1,28 @@
+2018-08-31  Simon Fraser  <simon.fra...@apple.com>
+
+        Make it possible to mark tests as leaks in TestExpectations
+        https://bugs.webkit.org/show_bug.cgi?id=189088
+
+        Reviewed by Jon Lee.
+        
+        Have webkitpy parse out "Leak" expectations in TestExpectations, and do the right
+        thing if the test run did not use --world-leaks. Add unit tests for leaks combined
+        with various other result combinations.
+
+        * Scripts/webkitpy/layout_tests/controllers/layout_test_runner.py:
+        (LayoutTestRunner._update_summary_with_result):
+        (LayoutTestRunner._annotate_results_with_additional_failures):
+        * Scripts/webkitpy/layout_tests/models/test_expectations.py:
+        (TestExpectationParser):
+        (TestExpectations): The 'Leak' line was duplicated here, so remove a copy.
+        (TestExpectations.remove_leak_failures):
+        (TestExpectations.matches_an_expected_result):
+        * Scripts/webkitpy/layout_tests/models/test_expectations_unittest.py:
+        (Base.get_basic_tests):
+        * Scripts/webkitpy/port/test.py:
+        (TestList.add_reftest):
+        (unit_test_list):
+
 2018-08-31  Zalan Bujtas  <za...@apple.com>
 
         [LFC] Add margin box verification back now that Display::Box has non-computed horizontal margin.

Modified: trunk/Tools/Scripts/webkitpy/layout_tests/controllers/layout_test_runner.py (235563 => 235564)


--- trunk/Tools/Scripts/webkitpy/layout_tests/controllers/layout_test_runner.py	2018-08-31 19:13:04 UTC (rev 235563)
+++ trunk/Tools/Scripts/webkitpy/layout_tests/controllers/layout_test_runner.py	2018-08-31 20:00:22 UTC (rev 235564)
@@ -182,7 +182,7 @@
             exp_str = got_str = 'SKIP'
             expected = True
         else:
-            expected = self._expectations.matches_an_expected_result(result.test_name, result.type, self._options.pixel_tests or result.reftest_type)
+            expected = self._expectations.matches_an_expected_result(result.test_name, result.type, self._options.pixel_tests or result.reftest_type, self._options.world_leaks)
             exp_str = self._expectations.model().get_expectations_string(result.test_name)
             got_str = self._expectations.model().expectation_to_string(result.type)
 
@@ -198,8 +198,8 @@
             # When running a chunk (--run-chunk), results_by_name contains all the tests, but (confusingly) all_tests only contains those in the chunk that was run,
             # and we don't want to modify the results of a test that didn't run. existing_result.test_number is only non-None for tests that ran.
             if existing_result and existing_result.test_number is not None:
-                was_expected = self._expectations.matches_an_expected_result(new_result.test_name, existing_result.type, self._options.pixel_tests or existing_result.reftest_type)
-                now_expected = self._expectations.matches_an_expected_result(new_result.test_name, new_result.type, self._options.pixel_tests or new_result.reftest_type)
+                was_expected = self._expectations.matches_an_expected_result(new_result.test_name, existing_result.type, self._options.pixel_tests or existing_result.reftest_type, self._options.world_leaks)
+                now_expected = self._expectations.matches_an_expected_result(new_result.test_name, new_result.type, self._options.pixel_tests or new_result.reftest_type, self._options.world_leaks)
                 run_results.change_result_to_failure(existing_result, new_result, was_expected, now_expected)
 
     def start_servers(self):

Modified: trunk/Tools/Scripts/webkitpy/layout_tests/models/test_expectations.py (235563 => 235564)


--- trunk/Tools/Scripts/webkitpy/layout_tests/models/test_expectations.py	2018-08-31 19:13:04 UTC (rev 235563)
+++ trunk/Tools/Scripts/webkitpy/layout_tests/models/test_expectations.py	2018-08-31 20:00:22 UTC (rev 235564)
@@ -241,11 +241,10 @@
     # FIXME: Update the original modifiers list and remove this once the old syntax is gone.
     _expectation_tokens = {
         'Crash': 'CRASH',
-        'Leak': 'LEAK',
         'Failure': 'FAIL',
         'ImageOnlyFailure': 'IMAGE',
+        'Leak': 'LEAK',
         'Missing': 'MISSING',
-        'Leak': 'LEAK',
         'Pass': 'PASS',
         'Rebaseline': 'REBASELINE',
         'Skip': 'SKIP',
@@ -404,6 +403,9 @@
         self.related_files = {}  # Dictionary of files to lines number in that file which may have caused the list of warnings.
         self.not_applicable_to_current_platform = False
 
+    def __str__(self):
+        return self.to_string(None)
+
     def is_invalid(self):
         return self.warnings and self.warnings != [TestExpectationParser.MISSING_BUG_WARNING]
 
@@ -863,10 +865,23 @@
         expected_results = expected_results.copy()
         if IMAGE in expected_results:
             expected_results.remove(IMAGE)
-            expected_results.add(PASS)
+            expected_results.add(PASS) # FIXME: does it always become a pass?
         return expected_results
 
     @staticmethod
+    def remove_leak_failures(expected_results):
+        """Returns a copy of the expected results for a test, except that we
+        drop any leak failures and return the remaining expectations. For example,
+        if we're not running with --world-leaks, then tests expected to fail as LEAK
+        will PASS."""
+        expected_results = expected_results.copy()
+        if LEAK in expected_results:
+            expected_results.remove(LEAK)
+            if not expected_results:
+                expected_results.add(PASS)
+        return expected_results
+
+    @staticmethod
     def has_pixel_failures(actual_results):
         return IMAGE in actual_results or FAIL in actual_results
 
@@ -950,10 +965,14 @@
     def get_rebaselining_failures(self):
         return self._model.get_test_set(REBASELINE)
 
-    def matches_an_expected_result(self, test, result, pixel_tests_are_enabled):
+    def matches_an_expected_result(self, test, result, pixel_tests_are_enabled, world_leaks_are_enabled):
         expected_results = self._model.get_expectations(test)
         if not pixel_tests_are_enabled:
             expected_results = self.remove_pixel_failures(expected_results)
+
+        if not world_leaks_are_enabled:
+            expected_results = self.remove_leak_failures(expected_results)
+
         return self.result_was_expected(result,
                                    expected_results,
                                    self.is_rebaselining(test),

Modified: trunk/Tools/Scripts/webkitpy/layout_tests/models/test_expectations_unittest.py (235563 => 235564)


--- trunk/Tools/Scripts/webkitpy/layout_tests/models/test_expectations_unittest.py	2018-08-31 19:13:04 UTC (rev 235563)
+++ trunk/Tools/Scripts/webkitpy/layout_tests/models/test_expectations_unittest.py	2018-08-31 20:00:22 UTC (rev 235564)
@@ -54,8 +54,11 @@
                 'failures/expected/image_checksum.html',
                 'failures/expected/crash.html',
                 'failures/expected/leak.html',
+                'failures/expected/flaky-leak.html',
                 'failures/expected/missing_text.html',
                 'failures/expected/image.html',
+                'failures/expected/reftest.html',
+                'failures/expected/leaky-reftest.html',
                 'passes/text.html']
 
     def get_basic_expectations(self):
@@ -62,10 +65,13 @@
         return """
 Bug(test) failures/expected/text.html [ Failure ]
 Bug(test) failures/expected/crash.html [ WontFix ]
-Bug(test) failures/expected/crash.html [ Leak ]
+Bug(test) failures/expected/leak.html [ Leak ]
+Bug(test) failures/expected/flaky-leak.html [ Failure Leak ]
 Bug(test) failures/expected/missing_image.html [ Rebaseline Missing ]
 Bug(test) failures/expected/image_checksum.html [ WontFix ]
 Bug(test) failures/expected/image.html [ WontFix Mac ]
+Bug(test) failures/expected/reftest.html [ ImageOnlyFailure ]
+Bug(test) failures/expected/leaky-reftest.html [ ImageOnlyFailure Leak ]
 """
 
     def parse_exp(self, expectations, overrides=None, is_lint_mode=False):
@@ -79,9 +85,11 @@
         self._exp.parse_all_expectations()
 
     def assert_exp(self, test, result):
-        self.assertEqual(self._exp.model().get_expectations(test),
-                          set([result]))
+        self.assertEqual(self._exp.model().get_expectations(test), set([result]))
 
+    def assert_exp_set(self, test, result_set):
+        self.assertEqual(self._exp.model().get_expectations(test), result_set)
+
     def assert_bad_expectations(self, expectations, overrides=None):
         self.assertRaises(ParseError, self.parse_exp, expectations, is_lint_mode=True, overrides=overrides)
 
@@ -91,8 +99,12 @@
         self.parse_exp(self.get_basic_expectations())
         self.assert_exp('failures/expected/text.html', FAIL)
         self.assert_exp('failures/expected/image_checksum.html', PASS)
-        self.assert_exp('failures/expected/leak.html', PASS)
+        self.assert_exp('failures/expected/reftest.html', IMAGE)
+        self.assert_exp_set('failures/expected/leaky-reftest.html', set([IMAGE, LEAK]))
+        self.assert_exp('failures/expected/leak.html', LEAK)
+        self.assert_exp_set('failures/expected/flaky-leak.html', set([FAIL, LEAK]))
         self.assert_exp('passes/text.html', PASS)
+        # self.assert_exp_set('passes/flaky-leak.html', set([PASS, LEAK]))
         self.assert_exp('failures/expected/image.html', PASS)
 
 
@@ -123,6 +135,14 @@
         self.assertEqual(TestExpectations.remove_pixel_failures(set([FAIL])), set([FAIL]))
         self.assertEqual(TestExpectations.remove_pixel_failures(set([PASS, IMAGE, CRASH])), set([PASS, CRASH]))
 
+    def test_remove_leak_failures(self):
+        self.assertEqual(TestExpectations.remove_leak_failures(set([FAIL])), set([FAIL]))
+        self.assertEqual(TestExpectations.remove_leak_failures(set([PASS])), set([PASS]))
+        self.assertEqual(TestExpectations.remove_leak_failures(set([LEAK])), set([PASS]))
+        self.assertEqual(TestExpectations.remove_leak_failures(set([PASS, LEAK])), set([PASS]))
+        self.assertEqual(TestExpectations.remove_leak_failures(set([FAIL, LEAK])), set([FAIL]))
+        self.assertEqual(TestExpectations.remove_leak_failures(set([PASS, IMAGE, LEAK, CRASH])), set([PASS, IMAGE, CRASH]))
+
     def test_suffixes_for_expectations(self):
         self.assertEqual(TestExpectations.suffixes_for_expectations(set([FAIL])), set(['txt', 'png', 'wav']))
         self.assertEqual(TestExpectations.suffixes_for_expectations(set([IMAGE])), set(['png']))
@@ -152,8 +172,7 @@
     def test_expectation_to_string(self):
         # Normal cases are handled by other tests.
         self.parse_exp(self.get_basic_expectations())
-        self.assertRaises(ValueError, self._exp.model().expectation_to_string,
-                          -1)
+        self.assertRaises(ValueError, self._exp.model().expectation_to_string, -1)
 
     def test_get_test_set(self):
         # Handle some corner cases for this routine not covered by other tests.
@@ -215,21 +234,45 @@
 
     def test_pixel_tests_flag(self):
         def match(test, result, pixel_tests_enabled):
-            return self._exp.matches_an_expected_result(
-                test, result, pixel_tests_enabled)
+            return self._exp.matches_an_expected_result(test, result, pixel_tests_enabled, False)
 
         self.parse_exp(self.get_basic_expectations())
-        self.assertTrue(match('failures/expected/text.html', FAIL, True))
-        self.assertTrue(match('failures/expected/text.html', FAIL, False))
-        self.assertFalse(match('failures/expected/text.html', CRASH, True))
-        self.assertFalse(match('failures/expected/text.html', CRASH, False))
-        self.assertTrue(match('failures/expected/image_checksum.html', PASS,
-                              True))
-        self.assertTrue(match('failures/expected/image_checksum.html', PASS,
-                              False))
-        self.assertTrue(match('failures/expected/crash.html', PASS, False))
-        self.assertTrue(match('passes/text.html', PASS, False))
+        pixel_tests_enabled = True
+        pixel_tests_disabled = False
+        self.assertTrue(match('failures/expected/text.html', FAIL, pixel_tests_enabled))
+        self.assertTrue(match('failures/expected/text.html', FAIL, pixel_tests_disabled))
+        self.assertFalse(match('failures/expected/text.html', CRASH, pixel_tests_enabled))
+        self.assertFalse(match('failures/expected/text.html', CRASH, pixel_tests_disabled))
+        self.assertTrue(match('failures/expected/image_checksum.html', PASS, pixel_tests_enabled))
+        self.assertTrue(match('failures/expected/image_checksum.html', PASS, pixel_tests_disabled))
+        self.assertTrue(match('failures/expected/crash.html', PASS, pixel_tests_disabled))
+        self.assertTrue(match('passes/text.html', PASS, pixel_tests_disabled))
 
+    def test_world_leaks_flag(self):
+        def match(test, result, pixel_tests_enabled, world_leaks_enabled):
+            return self._exp.matches_an_expected_result(test, result, pixel_tests_enabled, world_leaks_enabled)
+
+        pixel_tests_enabled = True
+        pixel_tests_disabled = False
+        world_leaks_enabled = True
+        world_leaks_disabled = False
+
+        self.parse_exp(self.get_basic_expectations())
+        self.assertTrue(match('failures/expected/leak.html', LEAK, pixel_tests_enabled, world_leaks_enabled))
+        self.assertTrue(match('failures/expected/leak.html', PASS, pixel_tests_enabled, world_leaks_disabled))
+        self.assertTrue(match('failures/expected/flaky-leak.html', FAIL, pixel_tests_enabled, world_leaks_disabled))
+
+        self.assertTrue(match('failures/expected/leaky-reftest.html', LEAK, pixel_tests_disabled, world_leaks_enabled))
+        self.assertTrue(match('failures/expected/leaky-reftest.html', PASS, pixel_tests_disabled, world_leaks_disabled))
+
+        self.assertTrue(match('failures/expected/leaky-reftest.html', IMAGE, pixel_tests_enabled, world_leaks_enabled))
+        self.assertTrue(match('failures/expected/leaky-reftest.html', LEAK, pixel_tests_enabled, world_leaks_enabled))
+        self.assertTrue(match('failures/expected/leaky-reftest.html', IMAGE, pixel_tests_enabled, world_leaks_disabled))
+
+        self.assertFalse(match('failures/expected/text.html', PASS, pixel_tests_enabled, world_leaks_enabled))
+        self.assertFalse(match('failures/expected/text.html', CRASH, pixel_tests_enabled, world_leaks_disabled))
+        self.assertTrue(match('passes/text.html', PASS, pixel_tests_enabled, world_leaks_disabled))
+
     def test_more_specific_override_resets_skip(self):
         self.parse_exp("Bug(x) failures/expected [ Skip ]\n"
                        "Bug(x) failures/expected/text.html [ ImageOnlyFailure ]\n")
@@ -348,6 +391,9 @@
     def test_slow(self):
         self.assert_tokenize_exp('foo.html [ Slow ]', modifiers=['SLOW'], expectations=['PASS'])
 
+    def test_leak(self):
+        self.assert_tokenize_exp('foo.html [ Leak ]', modifiers=[], expectations=['LEAK'])
+
     def test_wontfix(self):
         self.assert_tokenize_exp('foo.html [ WontFix ]', modifiers=['WONTFIX', 'SKIP'], expectations=['PASS'])
         self.assert_tokenize_exp('foo.html [ WontFix ImageOnlyFailure ]', modifiers=['WONTFIX'], expectations=['IMAGE'])

Modified: trunk/Tools/Scripts/webkitpy/layout_tests/models/test_results.py (235563 => 235564)


--- trunk/Tools/Scripts/webkitpy/layout_tests/models/test_results.py	2018-08-31 19:13:04 UTC (rev 235563)
+++ trunk/Tools/Scripts/webkitpy/layout_tests/models/test_results.py	2018-08-31 20:00:22 UTC (rev 235564)
@@ -68,6 +68,14 @@
     def __hash__(self):
         return self.test_name.__hash__()
 
+    def __str__(self):
+        result = self.test_name
+        if len(self.failures):
+            result += ' failures: ' + ','.join([failure.message() for failure in self.failures])
+        if len(self.reftest_type):
+            result += ' reftest_type: ' + ','.join(self.reftest_type)
+        return result
+
     def convert_to_failure(self, failure_result):
         if self.type is failure_result.type:
             return

Modified: trunk/Tools/Scripts/webkitpy/layout_tests/models/test_run_results.py (235563 => 235564)


--- trunk/Tools/Scripts/webkitpy/layout_tests/models/test_run_results.py	2018-08-31 19:13:04 UTC (rev 235563)
+++ trunk/Tools/Scripts/webkitpy/layout_tests/models/test_run_results.py	2018-08-31 20:00:22 UTC (rev 235564)
@@ -118,7 +118,7 @@
             self.unexpected -= 1
             if had_failures:
                 self.unexpected_failures -= 1
-        else:
+        elif existing_expected and not new_expected:
             # test changed from expected to unexpected
             self.expected -= 1
             self.unexpected_results_by_name[existing_result.test_name] = existing_result

Modified: trunk/Tools/Scripts/webkitpy/port/test.py (235563 => 235564)


--- trunk/Tools/Scripts/webkitpy/port/test.py	2018-08-31 19:13:04 UTC (rev 235563)
+++ trunk/Tools/Scripts/webkitpy/port/test.py	2018-08-31 20:00:22 UTC (rev 235564)
@@ -78,7 +78,7 @@
             test.__dict__[key] = value
         self.tests[name] = test
 
-    def add_reftest(self, name, reference_name, same_image):
+    def add_reftest(self, name, reference_name, same_image, **kwargs):
         self.add(name, actual_checksum='xxx', actual_image='XXX', is_reftest=True)
         if same_image:
             self.add(reference_name, actual_checksum='xxx', actual_image='XXX', is_reftest=True)
@@ -85,6 +85,11 @@
         else:
             self.add(reference_name, actual_checksum='yyy', actual_image='YYY', is_reftest=True)
 
+        if kwargs:
+            test = self.tests[name]
+            for key, value in kwargs.items():
+                test.__dict__[key] = value
+
     def keys(self):
         return self.tests.keys()
 
@@ -97,7 +102,7 @@
 #
 # These numbers may need to be updated whenever we add or delete tests.
 #
-TOTAL_TESTS = 74
+TOTAL_TESTS = 76
 TOTAL_SKIPS = 9
 TOTAL_RETRIES = 15
 
@@ -116,6 +121,7 @@
     tests.add('failures/expected/hang.html', hang=True)
     tests.add('failures/expected/missing_text.html', expected_text=None)
     tests.add('failures/expected/leak.html', leak=True)
+    tests.add('failures/expected/flaky-leak.html', leak=True)
     tests.add('failures/expected/image.html',
               actual_image='image_fail-pngtEXtchecksum\x00checksum_fail',
               expected_image='image-pngtEXtchecksum\x00checksum-png')
@@ -215,6 +221,7 @@
     tests.add_reftest('passes/xhtreftest.xht', 'passes/xhtreftest-expected.html', same_image=True)
     tests.add_reftest('passes/phpreftest.php', 'passes/phpreftest-expected-mismatch.svg', same_image=False)
     tests.add_reftest('failures/expected/reftest.html', 'failures/expected/reftest-expected.html', same_image=False)
+    tests.add_reftest('failures/expected/leaky-reftest.html', 'failures/expected/leaky-reftest-expected.html', same_image=False, leak=True)
     tests.add_reftest('failures/expected/mismatch.html', 'failures/expected/mismatch-expected-mismatch.html', same_image=True)
     tests.add_reftest('failures/unexpected/reftest.html', 'failures/unexpected/reftest-expected.html', same_image=False)
     tests.add_reftest('failures/unexpected/mismatch.html', 'failures/unexpected/mismatch-expected-mismatch.html', same_image=True)
@@ -283,6 +290,8 @@
         filesystem.write_text_file(LAYOUT_TEST_DIR + '/platform/test/TestExpectations', """
 Bug(test) failures/expected/crash.html [ Crash ]
 Bug(test) failures/expected/leak.html [ Leak ]
+Bug(test) failures/expected/flaky-leak.html [ Failure Leak ]
+Bug(test) failures/expected/leaky-reftest.html [ ImageOnlyFailure Leak ]
 Bug(test) failures/expected/image.html [ ImageOnlyFailure ]
 Bug(test) failures/expected/audio.html [ Failure ]
 Bug(test) failures/expected/image_checksum.html [ ImageOnlyFailure ]
@@ -599,11 +608,15 @@
             return None
 
         test_world_leaks_output = """TEST: file:///test.checkout/LayoutTests/failures/expected/leak.html
-ABANDONED DOCUMENT: file:///test.checkout/LayoutTests//failures/expected/leak.html
+ABANDONED DOCUMENT: file:///test.checkout/LayoutTests/failures/expected/leak.html
 TEST: file:///test.checkout/LayoutTests/failures/unexpected/leak.html
-ABANDONED DOCUMENT: file:///test.checkout/LayoutTests//failures/expected/leak.html
+ABANDONED DOCUMENT: file:///test.checkout/LayoutTests/failures/expected/flaky-leak.html
+TEST: file:///test.checkout/LayoutTests/failures/unexpected/flaky-leak.html
+ABANDONED DOCUMENT: file:///test.checkout/LayoutTests/failures/expected/leak.html
 TEST: file:///test.checkout/LayoutTests/failures/unexpected/leak.html
-ABANDONED DOCUMENT: file:///test.checkout/LayoutTests//failures/expected/leak-subframe.html"""
+ABANDONED DOCUMENT: file:///test.checkout/LayoutTests/failures/expected/leak-subframe.html
+TEST: file:///test.checkout/LayoutTests/failures/expected/leaky-reftest.html
+ABANDONED DOCUMENT: file:///test.checkout/LayoutTests/failures/expected/leaky-reftest.html"""
         return self._parse_world_leaks_output(test_world_leaks_output)
 
     def stop(self):

_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to