Attachment 349084 Details for Bug 189293 – Patch

[patch] Patch

bug-189293-20180906153644.patch (text/plain), 5.29 KB, created by Simon Fraser (smfr) on 2018-09-06 15:36:44 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Simon Fraser (smfr)

Created: 2018-09-06 15:36:44 PDT

Size: 5.29 KB

patch

obsolete

>Subversion Revision: 235741
>diff --git a/Tools/ChangeLog b/Tools/ChangeLog
>index 1a1e216449451bb436d1d73300f2df9e1161676a..ce1c1b4e51c249014b6fb8bbc39cf2ed139c10c2 100644
>--- a/Tools/ChangeLog
>+++ b/Tools/ChangeLog
>@@ -1,3 +1,19 @@
>+2018-09-06  Simon Fraser  <simon.fraser@apple.com>
>+
>+        Log when leak detection changes the test result
>+        https://bugs.webkit.org/show_bug.cgi?id=189293
>+
>+        Reviewed by NOBODY (OOPS!).
>+        
>+        When you have an "Leak" expectation in TestExpectations, it's confusing to see a test unexpectedly pass,
>+        but then show up as an expected fail at the end (because leak detection happens at the end of a shard).
>+        So log when leak detection changes a test result.
>+
>+        * Scripts/webkitpy/layout_tests/controllers/layout_test_runner.py:
>+        (LayoutTestRunner._annotate_results_with_additional_failures):
>+        * Scripts/webkitpy/layout_tests/models/test_expectations.py:
>+        (TestExpectations):
>+
> 2018-09-06  Myles C. Maxfield  <mmaxfield@apple.com>
> 
>         [WHLSL] The parser is too slow
>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 4b19974d9874dd826d3ab5d6db2b80eed99d1412..bf57f6f3fb25f4e34783d86094eee06a009902b2 100644
>--- a/Tools/Scripts/webkitpy/layout_tests/controllers/layout_test_runner.py
>+++ b/Tools/Scripts/webkitpy/layout_tests/controllers/layout_test_runner.py
>@@ -200,6 +200,11 @@ class LayoutTestRunner(object):
>             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, 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)
>+                if was_expected != now_expected:
>+                    # When annotation is not just about leaks, this logging should be changed.
>+                    _log.warning('  %s -> changed by leak detection from a %s (%s) to a %s (%s)' % (new_result.test_name,
>+                        TestExpectations.EXPECTATION_DESCRIPTION[existing_result.type], 'expected' if was_expected else 'unexpected',
>+                        TestExpectations.EXPECTATION_DESCRIPTION[new_result.type], 'expected' if now_expected else 'unexpected'))
>                 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 b3acc6725a74e4a3f8b938a8d0df48f87d699b20..47f01c98ce1fea834aea0ba375fb50f4b7d4a88b 100644
>--- a/Tools/Scripts/webkitpy/layout_tests/models/test_expectations.py
>+++ b/Tools/Scripts/webkitpy/layout_tests/models/test_expectations.py
>@@ -801,6 +801,19 @@ class TestExpectations(object):
>                     'leak': LEAK,
>                     'skip': SKIP}
> 
>+    # Singulars
>+    EXPECTATION_DESCRIPTION = {SKIP: 'skipped',
>+                                PASS: 'pass',
>+                                FAIL: 'failure',
>+                                IMAGE: 'image-only failure',
>+                                TEXT: 'text-only failure',
>+                                IMAGE_PLUS_TEXT: 'image and text failure',
>+                                AUDIO: 'audio failure',
>+                                CRASH: 'crash',
>+                                TIMEOUT: 'timeout',
>+                                MISSING: 'missing',
>+                                LEAK: 'leak'}
>+
>     # (aggregated by category, pass/fail/skip, type)
>     EXPECTATION_DESCRIPTIONS = {SKIP: 'skipped',
>                                 PASS: 'passes',
>diff --git a/Tools/Scripts/webkitpy/layout_tests/views/printing.py b/Tools/Scripts/webkitpy/layout_tests/views/printing.py
>index 0dec9c81ac4d4785d10a767303f7319873a39a2e..1d12371108e9e1b5561cc35966cf46f05ad5329d 100644
>--- a/Tools/Scripts/webkitpy/layout_tests/views/printing.py
>+++ b/Tools/Scripts/webkitpy/layout_tests/views/printing.py
>@@ -321,7 +321,7 @@ class Printer(object):
>     def print_finished_test(self, result, expected, exp_str, got_str):
>         test_name = result.test_name
> 
>-        result_message = self._result_message(result.type, result.failures, expected, self._options.verbose)
>+        result_message = self._result_message(result.type, result.failures, expected, exp_str, self._options.verbose)
> 
>         if self._options.details:
>             self._print_test_trace(result, exp_str, got_str)
>@@ -340,8 +340,11 @@ class Printer(object):
>             self._completed_tests = []
>         self._running_tests.remove(test_name)
> 
>-    def _result_message(self, result_type, failures, expected, verbose):
>-        exp_string = ' unexpectedly' if not expected else ''
>+    def _result_message(self, result_type, failures, expected, exp_str, verbose):
>+        exp_string = ''
>+        if not expected:
>+            exp_string = ' (leak detection is pending)' if 'LEAK' in exp_str else ' unexpectedly'
>+
>         if result_type == test_expectations.PASS:
>             return ' passed%s' % exp_string
>         else:

Flags:

jonlee: review+
commit-queue: commit-queue-

Actions: View | Formatted Diff | Diff

Attachments on bug 189293: 348883 | 349084