Attachment 348357 Details for Bug 189067 – Patch

[patch] Patch

bug-189067-20180828162013.patch (text/plain), 44.37 KB, created by Simon Fraser (smfr) on 2018-08-28 16:20:14 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Simon Fraser (smfr)

Created: 2018-08-28 16:20:14 PDT

Size: 44.37 KB

patch

obsolete

>Subversion Revision: 235434
>diff --git a/Tools/ChangeLog b/Tools/ChangeLog
>index 4a432cdddbb91eb2eb5599c749df98aca4426de2..615aea8bd31986727b36eb39ed251fee28817fdf 100644
>--- a/Tools/ChangeLog
>+++ b/Tools/ChangeLog
>@@ -1,3 +1,79 @@
>+2018-08-28  Simon Fraser  <simon.fraser@apple.com>
>+
>+        Teach webkitpy how to check leaks and treat leaks as test failures
>+        https://bugs.webkit.org/show_bug.cgi?id=189067
>+
>+        Reviewed by NOBODY (OOPS!).
>+        
>+        Add a new "--world-leaks" argument to run-webkit-tests. When enabled, DRT/WTR are launched
>+        with a --world-leaks argument (which is renamed in this patch for consistency). This enables the
>+        behavior added in r235408, namely that they check for leaked documents after each test, and at
>+        the end of one (if --run-singly) or a set of tests run in a single DRT/WTR instance handle the
>+        "#CHECK FOR WORLD LEAKS" command to get still-live documents.
>+        
>+        LayoutTestRunner in webkitpy now has the notion of doing "post-tests work", called via _finished_test_group(),
>+        and here it sends the "#CHECK FOR WORLD LEAKS" command to the runner and parses the resulting output block.
>+        If this results block includes leaks, we convert an existing TestResult into a LEAK failure
>+        in TestRunResults.change_result_to_failure(). Leaks are then added to the ouput JSON for display in results.html
>+
>+        Unit tests are updated with some leak examples.
>+
>+        * DumpRenderTree/mac/DumpRenderTree.mm:
>+        (initializeGlobalsFromCommandLineOptions):
>+        * Scripts/webkitpy/common/net/resultsjsonparser_unittest.py:
>+        (ParsedJSONResultsTest):
>+        * Scripts/webkitpy/layout_tests/controllers/layout_test_runner.py:
>+        (LayoutTestRunner._annotate_results_with_additional_failures):
>+        (LayoutTestRunner._handle_finished_test_group):
>+        (Worker.handle):
>+        (Worker._run_test):
>+        (Worker._do_post_tests_work):
>+        (Worker._finished_test_group):
>+        (Worker._run_test_in_another_thread):
>+        * Scripts/webkitpy/layout_tests/layout_package/json_layout_results_generator.py:
>+        (JSONLayoutResultsGenerator):
>+        * Scripts/webkitpy/layout_tests/models/test_expectations.py:
>+        (TestExpectationParser):
>+        (TestExpectations):
>+        * Scripts/webkitpy/layout_tests/models/test_expectations_unittest.py:
>+        (Base.get_basic_tests):
>+        * Scripts/webkitpy/layout_tests/models/test_failures.py:
>+        (determine_result_type):
>+        (FailureLeak):
>+        (FailureLeak.__init__):
>+        (FailureLeak.message):
>+        (FailureDocumentLeak):
>+        (FailureDocumentLeak.__init__):
>+        (FailureDocumentLeak.message):
>+        * Scripts/webkitpy/layout_tests/models/test_results.py:
>+        (TestResult.convert_to_failure):
>+        * Scripts/webkitpy/layout_tests/models/test_run_results.py:
>+        (TestRunResults.change_result_to_failure):
>+        (_interpret_test_failures):
>+        (summarize_results):
>+        * Scripts/webkitpy/layout_tests/models/test_run_results_unittest.py:
>+        (get_result):
>+        (run_results):
>+        (summarized_results):
>+        * Scripts/webkitpy/layout_tests/run_webkit_tests.py:
>+        (parse_args):
>+        * Scripts/webkitpy/layout_tests/run_webkit_tests_integrationtest.py:
>+        (parse_args):
>+        (RunTest.test_check_for_world_leaks):
>+        * Scripts/webkitpy/port/driver.py:
>+        (DriverPostTestOutput):
>+        (DriverPostTestOutput.__init__):
>+        (Driver.do_post_tests_work):
>+        (Driver._parse_world_leaks_output):
>+        (Driver.cmd_line):
>+        (DriverProxy.do_post_tests_work):
>+        * Scripts/webkitpy/port/test.py:
>+        (unit_test_list):
>+        * WebKitTestRunner/Options.cpp:
>+        (WTR::OptionsHandler::OptionsHandler):
>+        * WebKitTestRunner/TestController.cpp:
>+        (WTR::TestController::checkForWorldLeaks):
>+
> 2018-08-28  Alex Christensen  <achristensen@webkit.org>
> 
>         Fix 32-bit Mac build.
>diff --git a/Tools/DumpRenderTree/mac/DumpRenderTree.mm b/Tools/DumpRenderTree/mac/DumpRenderTree.mm
>index 2ad03a018457e540eed866b72f74656f4b9f6948..383a389aafab8c0289892986cdfd45afbac2cb6b 100644
>--- a/Tools/DumpRenderTree/mac/DumpRenderTree.mm
>+++ b/Tools/DumpRenderTree/mac/DumpRenderTree.mm
>@@ -1121,7 +1121,7 @@ static void initializeGlobalsFromCommandLineOptions(int argc, const char *argv[]
>         {"allow-any-certificate-for-allowed-hosts", no_argument, &allowAnyHTTPSCertificateForAllowedHosts, YES},
>         {"show-webview", no_argument, &showWebView, YES},
>         {"print-test-count", no_argument, &printTestCount, YES},
>-        {"check-for-world-leaks", no_argument, &checkForWorldLeaks, NO},
>+        {"world-leaks", no_argument, &checkForWorldLeaks, NO},
>         {nullptr, 0, nullptr, 0}
>     };
> 
>diff --git a/Tools/Scripts/webkitpy/common/net/resultsjsonparser_unittest.py b/Tools/Scripts/webkitpy/common/net/resultsjsonparser_unittest.py
>index 2f21f44b9189f040b570c243a152e1328623a01e..67a2ef6a5cf4b27c70f23051cf622ba329b09f49 100644
>--- a/Tools/Scripts/webkitpy/common/net/resultsjsonparser_unittest.py
>+++ b/Tools/Scripts/webkitpy/common/net/resultsjsonparser_unittest.py
>@@ -58,7 +58,13 @@ class ParsedJSONResultsTest(unittest.TestCase):
>                 },
>                 "prototype-strawberry.html": {
>                     "expected": "PASS",
>-                    "actual": "FAIL PASS"
>+                    "actual": "FAIL PASS",
>+                    "leaks": {
>+                        "documents": [
>+                            "file:///Volumes/Data/slave/webkit/build/LayoutTests/fast/dom/prototype-strawberry.html",
>+                            "about:blank"
>+                        ]
>+                    }
>                 }
>             }
>         },
>@@ -106,7 +112,13 @@ class ParsedJSONResultsTest(unittest.TestCase):
>                 },
>                 "prototype-strawberry.html": {
>                     "expected": "PASS",
>-                    "actual": "FAIL PASS"
>+                    "actual": "FAIL PASS",
>+                    "leaks": {
>+                        "documents": [
>+                            "file:///Volumes/Data/slave/webkit/build/LayoutTests/fast/dom/prototype-strawberry.html",
>+                            "about:blank"
>+                        ]
>+                    }
>                 }
>             }
>         },
>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 3e52e614aa89a62d8e5ee3fefafcdcca28b803d0..076547d4ed78550a80068afcd838b5ac0f8b1b64 100644
>--- a/Tools/Scripts/webkitpy/layout_tests/controllers/layout_test_runner.py
>+++ b/Tools/Scripts/webkitpy/layout_tests/controllers/layout_test_runner.py
>@@ -192,6 +192,16 @@ class LayoutTestRunner(object):
> 
>         self._interrupt_if_at_failure_limits(run_results)
> 
>+    def _annotate_results_with_additional_failures(self, run_results, results):
>+        for new_result in results:
>+            existing_result = run_results.results_by_name.get(new_result.test_name)
>+            # 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)
>+                run_results.change_result_to_failure(existing_result, new_result, was_expected, now_expected)
>+
>     def start_servers(self):
>         if self._needs_http and not self._did_start_http_server and not self._port.is_http_server_running():
>             self._printer.write_update('Starting HTTP server ...')
>@@ -232,6 +242,9 @@ class LayoutTestRunner(object):
>     def _handle_finished_test(self, worker_name, result, log_messages=[]):
>         self._update_summary_with_result(self._current_run_results, result)
> 
>+    def _handle_finished_test_group(self, worker_name, overlay_results, log_messages=[]):
>+        self._annotate_results_with_additional_failures(self._current_run_results, overlay_results)
>+
> 
> class Worker(object):
>     def __init__(self, caller, results_directory, options):
>@@ -269,6 +282,8 @@ class Worker(object):
>         for test_input in test_inputs:
>             self._run_test(test_input, test_list_name)
> 
>+        self._finished_test_group(test_inputs)
>+
>     def _update_test_input(self, test_input):
>         if test_input.reference_files is None:
>             # Lazy initialization.
>@@ -302,6 +317,29 @@ class Worker(object):
> 
>         self._clean_up_after_test(test_input, result)
> 
>+    def _do_post_tests_work(self, driver):
>+        additional_results = []
>+        if not driver:
>+            return additional_results
>+
>+        post_test_output = driver.do_post_tests_work()
>+        if post_test_output:
>+            for test_name, doc_list in post_test_output.world_leaks_dict.iteritems():
>+                additional_results.append(test_results.TestResult(test_name, [test_failures.FailureDocumentLeak(doc_list)]))
>+        return additional_results
>+
>+    def _finished_test_group(self, test_inputs):
>+        _log.debug("%s finished test group" % self._name)
>+
>+        if self._driver and self._driver.has_crashed():
>+            self._kill_driver()
>+
>+        additional_results = []
>+        if not self._options.run_singly:
>+            additional_results = self._do_post_tests_work(self._driver)
>+
>+        self._caller.post('finished_test_group', additional_results)
>+
>     def stop(self):
>         _log.debug("%s cleaning up" % self._name)
>         self._kill_driver()
>@@ -396,6 +434,11 @@ class Worker(object):
>             # thread's results.
>             _log.error('Test thread hung: killing all DumpRenderTrees')
>             failures = [test_failures.FailureTimeout()]
>+        else:
>+            failure_results = self._do_post_tests_work(driver)
>+            for failure_result in failure_results:
>+                if failure_result.test_name == result.test_name:
>+                    result.convert_to_failure(failure_result)
> 
>         driver.stop()
> 
>diff --git a/Tools/Scripts/webkitpy/layout_tests/layout_package/json_layout_results_generator.py b/Tools/Scripts/webkitpy/layout_tests/layout_package/json_layout_results_generator.py
>index 1bc10e68383b6bfce5325a3d2bcf1db0e66cdcf0..d0c22012b3bb3ce9c4aabf5042a32355384260bb 100644
>--- a/Tools/Scripts/webkitpy/layout_tests/layout_package/json_layout_results_generator.py
>+++ b/Tools/Scripts/webkitpy/layout_tests/layout_package/json_layout_results_generator.py
>@@ -49,6 +49,7 @@ class JSONLayoutResultsGenerator(json_results_generator.JSONResultsGenerator):
>                        test_expectations.TEXT: "F",
>                        test_expectations.AUDIO: "A",
>                        test_expectations.MISSING: "O",
>+                       test_expectations.LEAK: "L",
>                        test_expectations.IMAGE_PLUS_TEXT: "Z"}
> 
>     def __init__(self, port, builder_name, build_name, build_number,
>diff --git a/Tools/Scripts/webkitpy/layout_tests/models/test_expectations.py b/Tools/Scripts/webkitpy/layout_tests/models/test_expectations.py
>index e54ac94125f9dfacd7c6760788bd771b22fb43ff..af4d1be7f50c811a999133f1cfb870af205d7901 100644
>--- a/Tools/Scripts/webkitpy/layout_tests/models/test_expectations.py
>+++ b/Tools/Scripts/webkitpy/layout_tests/models/test_expectations.py
>@@ -43,7 +43,7 @@ _log = logging.getLogger(__name__)
> # FIXME: range() starts with 0 which makes if expectation checks harder
> # as PASS is 0.
> (PASS, FAIL, TEXT, IMAGE, IMAGE_PLUS_TEXT, AUDIO, TIMEOUT, CRASH, SKIP, WONTFIX,
>- SLOW, DUMPJSCONSOLELOGINSTDERR, REBASELINE, MISSING, FLAKY, NOW, NONE) = range(17)
>+ SLOW, LEAK, DUMPJSCONSOLELOGINSTDERR, REBASELINE, MISSING, FLAKY, NOW, NONE) = range(18)
> 
> # FIXME: Perhas these two routines should be part of the Port instead?
> BASELINE_SUFFIX_LIST = ('png', 'wav', 'txt')
>@@ -241,9 +241,11 @@ class TestExpectationParser(object):
>     # 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',
>         'Missing': 'MISSING',
>+        'Leak': 'LEAK',
>         'Pass': 'PASS',
>         'Rebaseline': 'REBASELINE',
>         'Skip': 'SKIP',
>@@ -794,6 +796,7 @@ class TestExpectations(object):
>                     'timeout': TIMEOUT,
>                     'crash': CRASH,
>                     'missing': MISSING,
>+                    'leak': LEAK,
>                     'skip': SKIP}
> 
>     # (aggregated by category, pass/fail/skip, type)
>@@ -806,9 +809,10 @@ class TestExpectations(object):
>                                 AUDIO: 'audio failures',
>                                 CRASH: 'crashes',
>                                 TIMEOUT: 'timeouts',
>-                                MISSING: 'missing results'}
>+                                MISSING: 'missing results',
>+                                LEAK: 'leaks'}
> 
>-    EXPECTATION_ORDER = (PASS, CRASH, TIMEOUT, MISSING, FAIL, IMAGE, SKIP)
>+    EXPECTATION_ORDER = (PASS, CRASH, TIMEOUT, MISSING, FAIL, IMAGE, LEAK, SKIP)
> 
>     BUILD_TYPES = ('debug', 'release')
> 
>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 07595d27a07af544db38fbfb5f2f9c42c9693357..6bda9732cd897287bad0dfd3aa1ea1a8a92e2780 100644
>--- a/Tools/Scripts/webkitpy/layout_tests/models/test_expectations_unittest.py
>+++ b/Tools/Scripts/webkitpy/layout_tests/models/test_expectations_unittest.py
>@@ -53,6 +53,7 @@ class Base(unittest.TestCase):
>         return ['failures/expected/text.html',
>                 'failures/expected/image_checksum.html',
>                 'failures/expected/crash.html',
>+                'failures/expected/leak.html',
>                 'failures/expected/missing_text.html',
>                 'failures/expected/image.html',
>                 'passes/text.html']
>@@ -61,6 +62,7 @@ class Base(unittest.TestCase):
>         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/missing_image.html [ Rebaseline Missing ]
> Bug(test) failures/expected/image_checksum.html [ WontFix ]
> Bug(test) failures/expected/image.html [ WontFix Mac ]
>@@ -89,6 +91,7 @@ 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('passes/text.html', PASS)
>         self.assert_exp('failures/expected/image.html', PASS)
> 
>diff --git a/Tools/Scripts/webkitpy/layout_tests/models/test_failures.py b/Tools/Scripts/webkitpy/layout_tests/models/test_failures.py
>index 7aeb8cf73cf5be6b3b9ef66b5d2481cbd29b0ba8..f8897b3a67495fb8f8c71f31dda974e2df175bcf 100644
>--- a/Tools/Scripts/webkitpy/layout_tests/models/test_failures.py
>+++ b/Tools/Scripts/webkitpy/layout_tests/models/test_failures.py
>@@ -59,6 +59,8 @@ def determine_result_type(failure_list):
>         return test_expectations.TIMEOUT
>     elif FailureEarlyExit in failure_types:
>         return test_expectations.SKIP
>+    elif FailureDocumentLeak in failure_types:
>+        return test_expectations.LEAK
>     elif (FailureMissingResult in failure_types or
>           FailureMissingImage in failure_types or
>           FailureMissingImageHash in failure_types or
>@@ -160,6 +162,23 @@ class FailureCrash(TestFailure):
>         writer.write_crash_log(crashed_driver_output.crash_log)
> 
> 
>+class FailureLeak(TestFailure):
>+    def __init__(self, leaked_document_urls):
>+        super(FailureLeak, self).__init__()
>+
>+    def message(self):
>+        return "leak"
>+
>+
>+class FailureDocumentLeak(FailureLeak):
>+    def __init__(self, leaked_document_urls):
>+        super(FailureDocumentLeak, self).__init__()
>+        self.leaked_document_urls = leaked_document_urls
>+
>+    def message(self):
>+        return "test leaked document%s %s" % ("s" if len(self.leaked_document_urls) else "", ', '.join(self.leaked_document_urls))
>+
>+
> class FailureMissingResult(FailureText):
>     def message(self):
>         return "-expected.txt was missing"
>diff --git a/Tools/Scripts/webkitpy/layout_tests/models/test_results.py b/Tools/Scripts/webkitpy/layout_tests/models/test_results.py
>index 82e1fb21171f3a2e6515fc710ad47b97f14a923d..922a96e33da842a69c0c3289de62002121be0b4e 100644
>--- a/Tools/Scripts/webkitpy/layout_tests/models/test_results.py
>+++ b/Tools/Scripts/webkitpy/layout_tests/models/test_results.py
>@@ -68,6 +68,12 @@ class TestResult(object):
>     def __hash__(self):
>         return self.test_name.__hash__()
> 
>+    def convert_to_failure(self, failure_result):
>+        if self.type is failure_result.type:
>+            return
>+        self.failures.extend(failure_result.failures)
>+        self.type = test_failures.determine_result_type(self.failures)
>+
>     def has_failure_matching_types(self, *failure_classes):
>         for failure in self.failures:
>             if type(failure) in failure_classes:
>diff --git a/Tools/Scripts/webkitpy/layout_tests/models/test_run_results.py b/Tools/Scripts/webkitpy/layout_tests/models/test_run_results.py
>index 5021a2ff3e848a698fa3dbaaecbc272f05d56124..197520d031f8976b3fe420c51495e7b074898e19 100644
>--- a/Tools/Scripts/webkitpy/layout_tests/models/test_run_results.py
>+++ b/Tools/Scripts/webkitpy/layout_tests/models/test_run_results.py
>@@ -93,6 +93,39 @@ class TestRunResults(object):
>         if test_is_slow:
>             self.slow_tests.add(test_result.test_name)
> 
>+    def change_result_to_failure(self, existing_result, new_result, existing_expected, new_expected):
>+        assert existing_result.test_name == new_result.test_name
>+        if existing_result.type is new_result.type:
>+            return
>+
>+        self.tests_by_expectation[existing_result.type].remove(existing_result.test_name)
>+        self.tests_by_expectation[new_result.type].add(new_result.test_name)
>+
>+        had_failures = len(existing_result.failures) > 0
>+
>+        existing_result.convert_to_failure(new_result)
>+
>+        if not had_failures and len(existing_result.failures):
>+            self.total_failures += 1
>+
>+        if len(existing_result.failures):
>+            self.failures_by_name[existing_result.test_name] = existing_result.failures
>+
>+        if not existing_expected and new_expected:
>+            # test changed from unexpected to expected
>+            self.expected += 1
>+            self.unexpected_results_by_name.pop(existing_result.test_name, None)
>+            self.unexpected -= 1
>+            if had_failures:
>+                self.unexpected_failures -= 1
>+        else:
>+            # test changed from expected to unexpected
>+            self.expected -= 1
>+            self.unexpected_results_by_name[existing_result.test_name] = existing_result
>+            self.unexpected += 1
>+            if len(existing_result.failures):
>+                self.unexpected_failures += 1
>+
>     def merge(self, test_run_results):
>         if not test_run_results:
>             return self
>@@ -132,6 +165,7 @@ class RunDetails(object):
> 
> def _interpret_test_failures(failures):
>     test_dict = {}
>+
>     failure_types = [type(failure) for failure in failures]
>     # FIXME: get rid of all this is_* values once there is a 1:1 map between
>     # TestFailure type and test_expectations.EXPECTATION.
>@@ -144,6 +178,14 @@ def _interpret_test_failures(failures):
>     if test_failures.FailureMissingImage in failure_types or test_failures.FailureMissingImageHash in failure_types:
>         test_dict['is_missing_image'] = True
> 
>+    if test_failures.FailureDocumentLeak in failure_types:
>+        leaks = []
>+        for failure in failures:
>+            if isinstance(failure, test_failures.FailureDocumentLeak):
>+                for url in failure.leaked_document_urls:
>+                    leaks.append({"document": url})
>+        test_dict['leaks'] = leaks
>+
>     if 'image_diff_percent' not in test_dict:
>         for failure in failures:
>             if isinstance(failure, test_failures.FailureImageHashMismatch) or isinstance(failure, test_failures.FailureReftestMismatch):
>@@ -178,8 +220,8 @@ def summarize_results(port_obj, expectations, initial_results, retry_results, en
>     num_missing = 0
>     num_regressions = 0
>     keywords = {}
>-    for expecation_string, expectation_enum in test_expectations.TestExpectations.EXPECTATIONS.iteritems():
>-        keywords[expectation_enum] = expecation_string.upper()
>+    for expectation_string, expectation_enum in test_expectations.TestExpectations.EXPECTATIONS.iteritems():
>+        keywords[expectation_enum] = expectation_string.upper()
> 
>     for modifier_string, modifier_enum in test_expectations.TestExpectations.MODIFIERS.iteritems():
>         keywords[modifier_enum] = modifier_string.upper()
>@@ -225,6 +267,10 @@ def summarize_results(port_obj, expectations, initial_results, retry_results, en
>             if test_name in initial_results.unexpected_results_by_name:
>                 num_missing += 1
>                 test_dict['report'] = 'MISSING'
>+        elif result_type == test_expectations.LEAK:
>+            if test_name in initial_results.unexpected_results_by_name:
>+                num_regressions += 1
>+                test_dict['report'] = 'REGRESSION'
>         elif test_name in initial_results.unexpected_results_by_name:
>             if retry_results and test_name not in retry_results.unexpected_results_by_name:
>                 actual.extend(expectations.model().get_expectations_string(test_name).split(" "))
>diff --git a/Tools/Scripts/webkitpy/layout_tests/models/test_run_results_unittest.py b/Tools/Scripts/webkitpy/layout_tests/models/test_run_results_unittest.py
>index 78f7397a09b4738cbf9cfd70f2a7b03d25045209..cbda1998065a3c7b7c976890c033da41e856c2b9 100644
>--- a/Tools/Scripts/webkitpy/layout_tests/models/test_run_results_unittest.py
>+++ b/Tools/Scripts/webkitpy/layout_tests/models/test_run_results_unittest.py
>@@ -43,12 +43,14 @@ def get_result(test_name, result_type=test_expectations.PASS, run_time=0):
>         failures = [test_failures.FailureAudioMismatch()]
>     elif result_type == test_expectations.CRASH:
>         failures = [test_failures.FailureCrash()]
>+    elif result_type == test_expectations.LEAK:
>+        failures = [test_failures.FailureDocumentLeak(['http://localhost:8000/failures/expected/leak.html'])]
>     return test_results.TestResult(test_name, failures=failures, test_run_time=run_time)
> 
> 
> def run_results(port):
>     tests = ['passes/text.html', 'failures/expected/timeout.html', 'failures/expected/crash.html', 'failures/expected/hang.html',
>-             'failures/expected/audio.html']
>+             'failures/expected/audio.html', 'failures/expected/leak.html']
>     expectations = test_expectations.TestExpectations(port, tests)
>     expectations.parse_all_expectations()
>     return test_run_results.TestRunResults(expectations, len(tests))
>@@ -63,16 +65,19 @@ def summarized_results(port, expected, passing, flaky, include_passes=False):
>         initial_results.add(get_result('failures/expected/audio.html', test_expectations.AUDIO), expected, test_is_slow)
>         initial_results.add(get_result('failures/expected/timeout.html', test_expectations.TIMEOUT), expected, test_is_slow)
>         initial_results.add(get_result('failures/expected/crash.html', test_expectations.CRASH), expected, test_is_slow)
>+        initial_results.add(get_result('failures/expected/leak.html', test_expectations.LEAK), expected, test_is_slow)
>     elif passing:
>         initial_results.add(get_result('passes/text.html'), expected, test_is_slow)
>         initial_results.add(get_result('failures/expected/audio.html'), expected, test_is_slow)
>         initial_results.add(get_result('failures/expected/timeout.html'), expected, test_is_slow)
>         initial_results.add(get_result('failures/expected/crash.html'), expected, test_is_slow)
>+        initial_results.add(get_result('failures/expected/leak.html'), expected, test_is_slow)
>     else:
>         initial_results.add(get_result('passes/text.html', test_expectations.TIMEOUT), expected, test_is_slow)
>         initial_results.add(get_result('failures/expected/audio.html', test_expectations.AUDIO), expected, test_is_slow)
>         initial_results.add(get_result('failures/expected/timeout.html', test_expectations.CRASH), expected, test_is_slow)
>         initial_results.add(get_result('failures/expected/crash.html', test_expectations.TIMEOUT), expected, test_is_slow)
>+        initial_results.add(get_result('failures/expected/leak.html', test_expectations.CRASH), expected, test_is_slow)
> 
>         # we only list hang.html here, since normally this is WontFix
>         initial_results.add(get_result('failures/expected/hang.html', test_expectations.TIMEOUT), expected, test_is_slow)
>@@ -82,6 +87,7 @@ def summarized_results(port, expected, passing, flaky, include_passes=False):
>         retry_results.add(get_result('passes/text.html'), True, test_is_slow)
>         retry_results.add(get_result('failures/expected/timeout.html'), True, test_is_slow)
>         retry_results.add(get_result('failures/expected/crash.html'), True, test_is_slow)
>+        retry_results.add(get_result('failures/expected/leak.html'), True, test_is_slow)
>     else:
>         retry_results = None
> 
>diff --git a/Tools/Scripts/webkitpy/layout_tests/run_webkit_tests.py b/Tools/Scripts/webkitpy/layout_tests/run_webkit_tests.py
>index ced6b9db88b39d206b9a88f1a22bf8b49cbb2250..4e8e315719067952f3fc3eacc00a65f1fade5b3f 100755
>--- a/Tools/Scripts/webkitpy/layout_tests/run_webkit_tests.py
>+++ b/Tools/Scripts/webkitpy/layout_tests/run_webkit_tests.py
>@@ -290,6 +290,7 @@ def parse_args(args):
>         optparse.make_option('--display-server', choices=['xvfb', 'xorg', 'weston', 'wayland'], default='xvfb',
>             help='"xvfb": Use a virtualized X11 server. "xorg": Use the current X11 session. '
>                  '"weston": Use a virtualized Weston server. "wayland": Use the current wayland session.'),
>+        optparse.make_option("--world-leaks", action="store_true", default=False, help="Check for world leaks (currently, only documents). Differs from --leaks in that this uses internal instrumentation, rather than external tools."),
>     ]))
> 
>     option_group_definitions.append(("iOS Options", [
>diff --git a/Tools/Scripts/webkitpy/layout_tests/run_webkit_tests_integrationtest.py b/Tools/Scripts/webkitpy/layout_tests/run_webkit_tests_integrationtest.py
>index 7ab327aeb70eab2fa7c998808e6a33edb945ed22..f04cb58cd680ae13e806fe3ed2b68e4506d4eed5 100644
>--- a/Tools/Scripts/webkitpy/layout_tests/run_webkit_tests_integrationtest.py
>+++ b/Tools/Scripts/webkitpy/layout_tests/run_webkit_tests_integrationtest.py
>@@ -67,6 +67,10 @@ def parse_args(extra_args=None, tests_included=False, new_results=False, print_n
> 
>     if not '--child-processes' in extra_args:
>         args.extend(['--child-processes', 1])
>+
>+    if not '--world-leaks' in extra_args:
>+        args.append('--world-leaks')
>+
>     args.extend(extra_args)
>     if not tests_included:
>         # We use the glob to test that globbing works.
>@@ -332,6 +336,9 @@ class RunTest(unittest.TestCase, StreamTestingMixin):
>     def test_gc_between_tests(self):
>         self.assertTrue(passing_run(['--gc-between-tests']))
> 
>+    def test_check_for_world_leaks(self):
>+        self.assertTrue(passing_run(['--world-leaks']))
>+
>     def test_complex_text(self):
>         self.assertTrue(passing_run(['--complex-text']))
> 
>diff --git a/Tools/Scripts/webkitpy/port/driver.py b/Tools/Scripts/webkitpy/port/driver.py
>index ea287fe000b2775d10d279237424d3ae3e989fbd..a803adb837f970cad84aa53ed8ecf80fcf17e943 100644
>--- a/Tools/Scripts/webkitpy/port/driver.py
>+++ b/Tools/Scripts/webkitpy/port/driver.py
>@@ -34,6 +34,7 @@ import shlex
> import sys
> import time
> import os
>+from collections import defaultdict
> 
> from os.path import normpath
> from webkitpy.common.system import path
>@@ -119,6 +120,13 @@ class DriverOutput(object):
>             self.error = re.sub(pattern[0], pattern[1], self.error)
> 
> 
>+class DriverPostTestOutput(object):
>+    """Groups data collected for a set of tests, collected after all those testse have run
>+    (for example, data about leaked objects)"""
>+    def __init__(self, world_leaks_dict):
>+        self.world_leaks_dict = world_leaks_dict
>+
>+
> class Driver(object):
>     """object for running test(s) using DumpRenderTree/WebKitTestRunner."""
> 
>@@ -242,6 +250,38 @@ class Driver(object):
>             crashed_process_name=self._crashed_process_name,
>             crashed_pid=self._crashed_pid, crash_log=crash_log, pid=pid)
> 
>+    def do_post_tests_work(self):
>+        if not self._port.get_option('world_leaks'):
>+            return None
>+
>+        if not self._server_process:
>+            return None
>+
>+        _log.debug('Checking for world leaks...')
>+        self._server_process.write('#CHECK FOR WORLD LEAKS\n')
>+        deadline = time.time() + 20
>+        block = self._read_block(deadline, '', wait_for_stderr_eof=True)
>+
>+        _log.debug('World leak result: %s' % (block.decoded_content))
>+
>+        return self._parse_world_leaks_output(block.decoded_content)
>+
>+    def _parse_world_leaks_output(self, output):
>+        tests_with_world_leaks = defaultdict(list)
>+
>+        last_test = None
>+        for line in output.splitlines():
>+            m = re.match('^TEST: (.+)$', line)
>+            if m:
>+                last_test = self.uri_to_test(m.group(1))
>+            m = re.match('^ABANDONED DOCUMENT: (.+)$', line)
>+            if m:
>+                leaked_document_url = m.group(1)
>+                if last_test:
>+                    tests_with_world_leaks[last_test].append(leaked_document_url)
>+
>+        return DriverPostTestOutput(tests_with_world_leaks)
>+
>     def _get_crash_log(self, stdout, stderr, newer_than):
>         return self._port._get_crash_log(self._crashed_process_name, self._crashed_pid, stdout, stderr, newer_than, target_host=self._target_host)
> 
>@@ -432,6 +472,8 @@ class Driver(object):
>             cmd.append('--accelerated-drawing')
>         if self._port.get_option('remote_layer_tree'):
>             cmd.append('--remote-layer-tree')
>+        if self._port.get_option('world_leaks'):
>+            cmd.append('--world-leaks')
>         if self._port.get_option('threaded'):
>             cmd.append('--threaded')
>         if self._no_timeout:
>@@ -705,6 +747,9 @@ class DriverProxy(object):
> 
>         return self._driver.run_test(driver_input, stop_when_done)
> 
>+    def do_post_tests_work(self):
>+        return self._driver.do_post_tests_work()
>+
>     def has_crashed(self):
>         return self._driver.has_crashed()
> 
>diff --git a/Tools/Scripts/webkitpy/port/test.py b/Tools/Scripts/webkitpy/port/test.py
>index df5e67d068884b8aa84d35305509c06b373f2fb6..f2d6c6ffcc34319c44b04e686136297da26c0cf6 100644
>--- a/Tools/Scripts/webkitpy/port/test.py
>+++ b/Tools/Scripts/webkitpy/port/test.py
>@@ -97,12 +97,12 @@ class TestList(object):
> #
> # These numbers may need to be updated whenever we add or delete tests.
> #
>-TOTAL_TESTS = 72
>+TOTAL_TESTS = 74
> TOTAL_SKIPS = 9
>-TOTAL_RETRIES = 14
>+TOTAL_RETRIES = 15
> 
> UNEXPECTED_PASSES = 7
>-UNEXPECTED_FAILURES = 17
>+UNEXPECTED_FAILURES = 18
> 
> 
> def unit_test_list():
>@@ -115,6 +115,7 @@ def unit_test_list():
>     tests.add('failures/expected/timeout.html', timeout=True)
>     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/image.html',
>               actual_image='image_fail-pngtEXtchecksum\x00checksum_fail',
>               expected_image='image-pngtEXtchecksum\x00checksum-png')
>@@ -176,6 +177,7 @@ layer at (0,0) size 800x34
>     tests.add('failures/unexpected/skip_pass.html')
>     tests.add('failures/unexpected/text.html', actual_text='text_fail-txt')
>     tests.add('failures/unexpected/timeout.html', timeout=True)
>+    tests.add('failures/unexpected/leak.html', leak=True)
>     tests.add('http/tests/passes/text.html')
>     tests.add('http/tests/passes/image.html')
>     tests.add('http/tests/ssl/text.html')
>@@ -280,6 +282,7 @@ def add_unit_tests_to_mock_filesystem(filesystem):
>     if not filesystem.exists(LAYOUT_TEST_DIR + '/platform/test/TestExpectations'):
>         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/image.html [ ImageOnlyFailure ]
> Bug(test) failures/expected/audio.html [ Failure ]
> Bug(test) failures/expected/image_checksum.html [ ImageOnlyFailure ]
>@@ -591,5 +594,17 @@ class TestDriver(Driver):
>             crashed_pid=crashed_pid, crash_log=crash_log,
>             test_time=time.time() - start_time, timeout=test.timeout, error=test.error, pid=self.pid)
> 
>+    def do_post_tests_work(self):
>+        if not self._port.get_option('world_leaks'):
>+            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
>+TEST: file:///test.checkout/LayoutTests/failures/unexpected/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"""
>+        return self._parse_world_leaks_output(test_world_leaks_output)
>+
>     def stop(self):
>         self.started = False
>diff --git a/Tools/WebKitTestRunner/Options.cpp b/Tools/WebKitTestRunner/Options.cpp
>index a51440b918776b58ad68d8dd5f58956cc2765313..9dd37b028b4a5b4ab4dc33734967be07a73ba867 100644
>--- a/Tools/WebKitTestRunner/Options.cpp
>+++ b/Tools/WebKitTestRunner/Options.cpp
>@@ -135,7 +135,7 @@ OptionsHandler::OptionsHandler(Options& o)
>     optionList.append(Option("--allow-any-certificate-for-allowed-hosts", "Allows any HTTPS certificate for an allowed host.", handleOptionAllowAnyHTTPSCertificateForAllowedHosts));
>     optionList.append(Option("--show-webview", "Show the WebView during test runs (for debugging)", handleOptionShowWebView));
>     optionList.append(Option("--show-touches", "Show the touches during test runs (for debugging)", handleOptionShowTouches));
>-    optionList.append(Option("--check-for-world-leaks", "Check for leaks of world objects (currently, documents)", handleOptionCheckForWorldLeaks));
>+    optionList.append(Option("--world-leaks", "Check for leaks of world objects (currently, documents)", handleOptionCheckForWorldLeaks));
> 
>     optionList.append(Option(0, 0, handleOptionUnmatched));
> }
>diff --git a/Tools/WebKitTestRunner/TestController.cpp b/Tools/WebKitTestRunner/TestController.cpp
>index 68a4d12f50692d0050e735b5467fc09b0c0d3b31..b63da018d354e5b5b67a22ab1a9f34dd2016460d 100644
>--- a/Tools/WebKitTestRunner/TestController.cpp
>+++ b/Tools/WebKitTestRunner/TestController.cpp
>@@ -950,6 +950,9 @@ void TestController::updateLiveDocumentsAfterTest()
> 
> void TestController::checkForWorldLeaks()
> {
>+    if (!TestController::singleton().mainWebView())
>+        return;
>+
>     AsyncTask([]() {
>         // This runs at the end of a series of tests. It clears caches, runs a GC and then fetches the list of documents.
>         WKRetainPtr<WKStringRef> messageName = adoptWK(WKStringCreateWithUTF8CString("CheckForWorldLeaks"));
>diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
>index 33e01c6b2565a9df9681e3e59388fe223833adee..550faa8e9f885b3bb62fa05c288264fa95505622 100644
>--- a/LayoutTests/ChangeLog
>+++ b/LayoutTests/ChangeLog
>@@ -1,3 +1,17 @@
>+2018-08-28  Simon Fraser  <simon.fraser@apple.com>
>+
>+        Teach webkitpy how to check leaks and treat leaks as test failures
>+        https://bugs.webkit.org/show_bug.cgi?id=189067
>+
>+        Reviewed by NOBODY (OOPS!).
>+        
>+        Put some fake leaks in full_results.json, and update results.html to show a table
>+        of leaks when results are expanded.
>+
>+        * fast/harness/full_results.json:
>+        * fast/harness/results-expected.txt:
>+        * fast/harness/results.html:
>+
> 2018-08-28  Youenn Fablet  <youenn@apple.com>
> 
>         WebKitMediaSession should be GC collectable when its document is being stopped
>diff --git a/LayoutTests/fast/harness/full_results.json b/LayoutTests/fast/harness/full_results.json
>index 3e85d2a00c3ddbf69b6caef132adbd53fd95206b..10f7b851b9754fbac59602df98b19b4bc674acba 100644
>--- a/LayoutTests/fast/harness/full_results.json
>+++ b/LayoutTests/fast/harness/full_results.json
>@@ -11,6 +11,26 @@ ADD_RESULTS({
>                         "report": "REGRESSION",
>                         "expected": "PASS",
>                         "actual": "TEXT"
>+                    },
>+                    "image-fail-with-leak.html": {
>+                        "report": "REGRESSION",
>+                        "expected": "PASS",
>+                        "actual": "IMAGE LEAK",
>+                        "image_diff_percent": 0.26,
>+                        "leaks": [{
>+                            "document": "http://localhost:8800/WebKit/cache-storage/image-fail-with-leak.html",
>+                        }]
>+                    },
>+                    "leaky-worker.html": {
>+                        "report": "REGRESSION",
>+                        "expected": "PASS",
>+                        "actual": "LEAK",
>+                        "has_stderr": true,
>+                        "leaks": [{
>+                            "document": "http://localhost:8800/WebKit/cache-storage/leaky-worker.html",
>+                            }, {
>+                            "document": "http://localhost:8800/WebKit/cache-storage/resources/leaky-worker-subframe.html"
>+                        }]
>                     }
>                 }
>             },
>@@ -69,6 +89,14 @@ ADD_RESULTS({
>                     "expected": "IMAGE",
>                     "actual": "PASS",
>                     "reftest_type": ["!="]
>+                },
>+                "spelling-leaky-ref.html": {
>+                    "expected": "LEAK",
>+                    "actual": "LEAK",
>+                    "reftest_type": ["!="],
>+                    "leaks": [{
>+                        "document": "file:///Volumes/Data/slave/highsierra-release-tests-wk2/build/LayoutTests/editing/spelling/spelling-leaky-ref.html"
>+                    }]
>                 }
>             }
>         },
>@@ -121,20 +149,20 @@ ADD_RESULTS({
>         }
>     },
>     "skipped": 5367,
>-    "num_regressions": 2,
>+    "num_regressions": 7,
>     "other_crashes": {
>         "DumpRenderTree-54888": {},
>         "DumpRenderTree-56804": {},
>     },
>     "interrupted": false,
>-    "num_missing": 0,
>+    "num_missing": 1,
>     "layout_tests_dir": "/Volumes/Data/slave/highsierra-release-tests-wk2/build/LayoutTests",
>     "version": 4,
>     "num_passes": 49387,
>     "pixel_tests_enabled": false,
>     "date": "05:38PM on August 15, 2018",
>     "has_pretty_patch": true,
>-    "fixable": 6360,
>+    "fixable": 5,
>     "num_flaky": 0,
>     "uses_expectations_file": true,
>     "revision": "234905"
>diff --git a/LayoutTests/fast/harness/results-expected.txt b/LayoutTests/fast/harness/results-expected.txt
>index 1b505301e731704b0558d38e942badc6e3cea26d..d5318e68d651621ba1037ae8a563798edb3ef1b2 100644
>--- a/LayoutTests/fast/harness/results-expected.txt
>+++ b/LayoutTests/fast/harness/results-expected.txt
>@@ -1,6 +1,7 @@
> Use the i, j, k and l keys to navigate, e, c to expand and collapse, and f to flag
> expand all collapse all options
> Only unexpected results
>+Toggle images
> Use newlines in flagged list
> Tests that crashed (1): flag all
> 
>@@ -9,12 +10,14 @@ Other crashes (2): flag all
> 
> +DumpRenderTree-54888	crash log
> +DumpRenderTree-56804	crash log
>-Tests that failed text/pixel/audio diff (3): flag all
>+Tests that failed text/pixel/audio diff (5): flag all
> 
>- test 	results		actual failure	expected failure	history
>+ test 	results	image results	actual failure	expected failure	history
> +css1/font_properties/font_family.html	expected actual diff pretty diff	images	text missing		history
> +http/tests/storageAccess/request-storage-access-top-frame.html	expected actual diff pretty diff		text	pass timeout	history
> +http/wpt/cache-storage/cache-put-keys.https.any.worker.html	expected actual diff pretty diff		text	pass	history
>++http/wpt/cache-storage/image-fail-with-leak.html		images diff (0.26%)	image leak	pass	history
>++http/wpt/cache-storage/leaky-worker.html			leak	pass	history
> Tests that had no expected results (probably new) (1): flag all
> 
> test	results	image results	actual failure	expected failure	history
>@@ -22,9 +25,10 @@ svg/batik/smallFonts.svg			missing		history
> Tests that timed out (1): flag all
> 
> +platform/mac/media/audio-session-category-video-paused.html	expected actual diff pretty diff	history
>-Tests that had stderr output (2): flag all
>+Tests that had stderr output (3): flag all
> 
> +http/tests/contentextensions/top-url.html	stderr	history
>++http/wpt/cache-storage/leaky-worker.html	stderr	history
> +platform/mac/media/audio-session-category-video-paused.html	stderr	history
> Flaky tests (failed the first run and passed on retry) (1): flag all
> 
>diff --git a/LayoutTests/fast/harness/results.html b/LayoutTests/fast/harness/results.html
>index 8b291add780d39083412b33522640e0af3ebe2d1..be2b7c4c5b57bd859b98adb0af81e1c3190c17a4 100644
>--- a/LayoutTests/fast/harness/results.html
>+++ b/LayoutTests/fast/harness/results.html
>@@ -40,6 +40,11 @@ td:not(:first-of-type) {
> 
> th, td {
>     padding: 1px 4px;
>+    vertical-align: top;
>+}
>+
>+td:nth-child(1) {  
>+    min-width: 35em;
> }
> 
> th:empty, td:empty {
>@@ -51,6 +56,15 @@ th {
>     -moz-user-select: none;
> }
> 
>+dt > sup {
>+    vertical-align:text-top;
>+    font-size:75%;
>+}
>+
>+sup > a {
>+    text-decoration: none;
>+}
>+
> .content-container {
>     min-height: 0;
> }
>@@ -138,6 +152,13 @@ tbody.flagged .flag {
>     vertical-align: top;
> }
> 
>+.leaks > table {
>+    margin: 4px;
>+}
>+.leaks > td {
>+    padding-left: 20px;
>+}
>+
> #options {
>     background-color: white;
> }
>@@ -369,6 +390,11 @@ class TestResult
>         return this.info.actual.indexOf('AUDIO') != -1;
>     }
> 
>+    hasLeak()
>+    {
>+        return this.info.actual.indexOf('LEAK') != -1;
>+    }
>+
>     isCrash()
>     {
>         return this.info.actual == 'CRASH';
>@@ -787,6 +813,17 @@ class TestResultsController
>         return basePath;
>     }
> 
>+    convertToLayoutTestBaseRelativeURL(fullURL)
>+    {
>+        if (fullURL.startsWith('file://')) {
>+            let urlPrefix = 'file://' + this.layoutTestsBasePath();
>+            if (fullURL.startsWith(urlPrefix))
>+                return fullURL.substring(urlPrefix.length);
>+        }
>+        
>+        return fullURL;
>+    }
>+
>     shouldUseTracLinks()
>     {
>         return !this.testResults.layoutTestsDir() || !location.toString().indexOf('file://') == 0;
>@@ -1020,9 +1057,47 @@ class TestResultsController
>             Utils.appendHTML(newCell, result);    
>         }
> 
>+        if (testResult.hasLeak())
>+            newCell.appendChild(TestResultsController._makeLeaksCell(testResult));
>+
>         newRow.appendChild(newCell);
>+
>         return newRow;
>     }
>+    
>+    static _makeLeaksCell(testResult)
>+    {
>+        let container = document.createElement('div');
>+        container.className = 'result-container leaks';
>+        
>+        let label = document.createElement('div');
>+        label.className = 'label';
>+        label.textContent = "Leaks";
>+        container.appendChild(label);
>+
>+        let leaksTable = document.createElement('table');
>+        
>+        for (let leak of testResult.info.leaks) {
>+            let leakRow = document.createElement('tr');
>+
>+            for (let leakedObjectType in leak) {
>+                let th = document.createElement('th');
>+                th.textContent = leakedObjectType;
>+                let td = document.createElement('td');
>+                
>+                let url = leak[leakedObjectType]; // FIXME: when we track leaks other than documents, this might not be a URL.
>+                td.textContent = controller.convertToLayoutTestBaseRelativeURL(url)
>+                
>+                leakRow.appendChild(th);
>+                leakRow.appendChild(td);
>+            }
>+            
>+            leaksTable.appendChild(leakRow);
>+        }
>+        
>+        container.appendChild(leaksTable);
>+        return container;
>+    }
> 
>     static _resultIframe(src)
>     {

Actions: View | Formatted Diff | Diff

Attachments on bug 189067: 348357 | 348358