Attachment 348411 Details for Bug 189088 – Patch

[patch] Patch

bug-189088-20180829102840.patch (text/plain), 17.81 KB, created by Simon Fraser (smfr) on 2018-08-29 10:28:41 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Simon Fraser (smfr)

Created: 2018-08-29 10:28:41 PDT

Size: 17.81 KB

patch

obsolete

>Subversion Revision: 235453
>diff --git a/Tools/ChangeLog b/Tools/ChangeLog
>index 61b6c788923fcac46b784ad580067cf9bd18a675..325afd6c27831303b1e35c7f5fcf11f75d7fadcf 100644
>--- a/Tools/ChangeLog
>+++ b/Tools/ChangeLog
>@@ -1,3 +1,28 @@
>+2018-08-29  Simon Fraser  <simon.fraser@apple.com>
>+
>+        Make it possible to mark tests as leaks in TestExpectations
>+        https://bugs.webkit.org/show_bug.cgi?id=189088
>+
>+        Reviewed by NOBODY (OOPS!).
>+        
>+        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-28  Simon Fraser  <simon.fraser@apple.com>
> 
>         Teach webkitpy how to check leaks and treat leaks as test failures
>diff --git a/Tools/Scripts/webkitpy/layout_tests/controllers/layout_test_runner.py b/Tools/Scripts/webkitpy/layout_tests/controllers/layout_test_runner.py
>index 076547d4ed78550a80068afcd838b5ac0f8b1b64..4b19974d9874dd826d3ab5d6db2b80eed99d1412 100644
>--- a/Tools/Scripts/webkitpy/layout_tests/controllers/layout_test_runner.py
>+++ b/Tools/Scripts/webkitpy/layout_tests/controllers/layout_test_runner.py
>@@ -182,7 +182,7 @@ class LayoutTestRunner(object):
>             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 @@ class LayoutTestRunner(object):
>             # 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):
>diff --git a/Tools/Scripts/webkitpy/layout_tests/models/test_expectations.py b/Tools/Scripts/webkitpy/layout_tests/models/test_expectations.py
>index af4d1be7f50c811a999133f1cfb870af205d7901..c592a5e9be0237ead65390fd3f4503030507556f 100644
>--- a/Tools/Scripts/webkitpy/layout_tests/models/test_expectations.py
>+++ b/Tools/Scripts/webkitpy/layout_tests/models/test_expectations.py
>@@ -245,7 +245,6 @@ class TestExpectationParser(object):
>         'Failure': 'FAIL',
>         'ImageOnlyFailure': 'IMAGE',
>         'Missing': 'MISSING',
>-        'Leak': 'LEAK',
>         'Pass': 'PASS',
>         'Rebaseline': 'REBASELINE',
>         'Skip': 'SKIP',
>@@ -866,6 +865,19 @@ class TestExpectations(object):
>             expected_results.add(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 +962,14 @@ class TestExpectations(object):
>     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),
>diff --git a/Tools/Scripts/webkitpy/layout_tests/models/test_expectations_unittest.py b/Tools/Scripts/webkitpy/layout_tests/models/test_expectations_unittest.py
>index 6bda9732cd897287bad0dfd3aa1ea1a8a92e2780..7bfba1e8ab1494e10ddd0fa2d4e802ea34a83556 100644
>--- a/Tools/Scripts/webkitpy/layout_tests/models/test_expectations_unittest.py
>+++ b/Tools/Scripts/webkitpy/layout_tests/models/test_expectations_unittest.py
>@@ -54,18 +54,26 @@ class Base(unittest.TestCase):
>                 '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/flaky-leak.html',
>                 'passes/text.html']
> 
>     def get_basic_expectations(self):
>         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 ]
>+Bug(test) passes/flaky-leak.html [ Pass Leak ]
> """
> 
>     def parse_exp(self, expectations, overrides=None, is_lint_mode=False):
>@@ -79,8 +87,10 @@ Bug(test) failures/expected/image.html [ WontFix Mac ]
>         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 +101,12 @@ class BasicTests(Base):
>         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 +137,14 @@ class MiscTests(Base):
>         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 +174,7 @@ class MiscTests(Base):
>     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,20 +236,44 @@ class MiscTests(Base):
> 
>     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())
>+        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/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))
>+        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"
>@@ -348,6 +393,9 @@ class ExpectationSyntaxTests(Base):
>     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'])
>diff --git a/Tools/Scripts/webkitpy/port/test.py b/Tools/Scripts/webkitpy/port/test.py
>index f2d6c6ffcc34319c44b04e686136297da26c0cf6..a5bd26daf06d1bf5375ac459391ffa5b72f1fa67 100644
>--- a/Tools/Scripts/webkitpy/port/test.py
>+++ b/Tools/Scripts/webkitpy/port/test.py
>@@ -78,13 +78,18 @@ class TestList(object):
>             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)
>         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()
> 
>@@ -116,6 +121,7 @@ def unit_test_list():
>     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')
>@@ -198,6 +204,7 @@ layer at (0,0) size 800x34
>     tests.add('passes/checksum_in_image.html',
>               expected_image='tEXtchecksum\x00checksum_in_image-checksum')
>     tests.add('passes/skipped/skip.html')
>+    tests.add('passes/flaky-leak.html', leak=True)
> 
>     # Note that here the checksums don't match but the images do, so this test passes "unexpectedly".
>     # See https://bugs.webkit.org/show_bug.cgi?id=69444 .
>@@ -215,6 +222,7 @@ layer at (0,0) size 800x34
>     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)

Actions: View | Formatted Diff | Diff

Attachments on bug 189088: 348411 | 348424 | 348435