Changeset 285168 in webkit

Timestamp:

Nov 2, 2021, 10:50:02 AM (5 years ago)

Author:

timothy_horton@apple.com

Message:

dumpAsText() tests don't get the ref-test treatment when using --self-compare-with-header
​https://bugs.webkit.org/show_bug.cgi?id=232611

Reviewed by Jonathan Bedard.

One oversight in r285132: a test can disable pixel dumping by calling
dumpAsText(). This causes --self-compare-with-header to fall over because
it expects every test to have pixel results (and that is the whole point).

Add an un-overrideable --force-dump-pixels TestCommand argument,
and adopt it for self-comparison tests.

• Scripts/webkitpy/layout_tests/controllers/single_test_runner.py:

(SingleTestRunner._run_self_comparison_test):

• Scripts/webkitpy/port/driver.py:

(DriverInput.init):
(DriverInput.repr):
(Driver._command_from_driver_input):

• TestRunnerShared/TestCommand.cpp:

(WTR::parseInputLine):

• TestRunnerShared/TestCommand.h:
• WebKitTestRunner/TestController.cpp:

(WTR::TestController::runTest):

• WebKitTestRunner/TestInvocation.cpp:

(WTR::TestInvocation::dumpResults):
(WTR::TestInvocation::didReceiveSynchronousMessageFromInjectedBundle):

• WebKitTestRunner/TestInvocation.h:

Location:

trunk/Tools

Files:

• ChangeLog (modified) (1 diff)
• Scripts/webkitpy/layout_tests/controllers/single_test_runner.py (modified) (1 diff)
• Scripts/webkitpy/port/driver.py (modified) (3 diffs)
• TestRunnerShared/TestCommand.cpp (modified) (1 diff)
• TestRunnerShared/TestCommand.h (modified) (1 diff)
• WebKitTestRunner/TestController.cpp (modified) (1 diff)
• WebKitTestRunner/TestInvocation.cpp (modified) (1 diff)
• WebKitTestRunner/TestInvocation.h (modified) (2 diffs)

trunk/Tools/ChangeLog

@@ +1 @@
+2021-11-02  Tim Horton  <timothy_horton@apple.com>
+
+        dumpAsText() tests don't get the ref-test treatment when using --self-compare-with-header
+        https://bugs.webkit.org/show_bug.cgi?id=232611
+
+        Reviewed by Jonathan Bedard.
+
+        One oversight in r285132: a test can disable pixel dumping by calling
+        dumpAsText(). This causes --self-compare-with-header to fall over because
+        it expects every test to have pixel results (and that is the whole point).
+
+        Add an un-overrideable `--force-dump-pixels` TestCommand argument,
+        and adopt it for self-comparison tests.
+
+        * Scripts/webkitpy/layout_tests/controllers/single_test_runner.py:
+        (SingleTestRunner._run_self_comparison_test):
+        * Scripts/webkitpy/port/driver.py:
+        (DriverInput.__init__):
+        (DriverInput.__repr__):
+        (Driver._command_from_driver_input):
+        * TestRunnerShared/TestCommand.cpp:
+        (WTR::parseInputLine):
+        * TestRunnerShared/TestCommand.h:
+        * WebKitTestRunner/TestController.cpp:
+        (WTR::TestController::runTest):
+        * WebKitTestRunner/TestInvocation.cpp:
+        (WTR::TestInvocation::dumpResults):
+        (WTR::TestInvocation::didReceiveSynchronousMessageFromInjectedBundle):
+        * WebKitTestRunner/TestInvocation.h:
+
 2021-11-02  Kate Cheney  <katherine_cheney@apple.com>
 

trunk/Tools/Scripts/webkitpy/layout_tests/controllers/single_test_runner.py

@@ -343 +343 @@
         driver_input = self._driver_input()
         driver_input.should_run_pixel_test = True
+        driver_input.force_dump_pixels = True
 
         reference_output = self._driver.run_test(driver_input, self._stop_when_done)

trunk/Tools/Scripts/webkitpy/port/driver.py

@@ -47 +47 @@
 
 class DriverInput(object):
-    def __init__(self, test_name, timeout, image_hash, should_run_pixel_test, should_dump_jsconsolelog_in_stderr=None, args=None, self_comparison_header=None):
+    def __init__(self, test_name, timeout, image_hash, should_run_pixel_test, should_dump_jsconsolelog_in_stderr=None, args=None, self_comparison_header=None, force_dump_pixels=False):
         self.test_name = test_name
         self.timeout = timeout  # in ms
@@ -55 +55 @@
         self.args = args or []
         self.self_comparison_header = self_comparison_header
+        self.force_dump_pixels = force_dump_pixels
 
     def __repr__(self):
-        return "DriverInput(test_name='{}', timeout={}, image_hash={}, should_run_pixel_test={}, should_dump_jsconsolelog_in_stderr={}, self_comparison_header={}'".format(self.test_name, self.timeout, self.image_hash, self.should_run_pixel_test, self.should_dump_jsconsolelog_in_stderr, self.self_comparison_header)
+        return "DriverInput(test_name='{}', timeout={}, image_hash={}, should_run_pixel_test={}, should_dump_jsconsolelog_in_stderr={}, self_comparison_header={}, force_dump_pixels={}'".format(self.test_name, self.timeout, self.image_hash, self.should_run_pixel_test, self.should_dump_jsconsolelog_in_stderr, self.self_comparison_header, self.force_dump_pixels)
 
 
@@ -637 +638 @@
         if driver_input.self_comparison_header:
             command += "'--self-compare-with-header'%s" % driver_input.self_comparison_header
+        if driver_input.force_dump_pixels:
+            command += "'--force-dump-pixels"
 
         # --pixel-test must be the last argument, because the hash is optional,

trunk/Tools/TestRunnerShared/TestCommand.cpp

@@ -108 +108 @@
         else if (arg == std::string("--absolutePath"))
             result.absolutePath = tokenizer.next();
+        else if (arg == "--force-dump-pixels")
+            result.forceDumpPixels = true;
         else
             die(inputLine);

trunk/Tools/TestRunnerShared/TestCommand.h

@@ -39 +39 @@
     WTF::Seconds timeout;
     bool shouldDumpPixels { false };
+    bool forceDumpPixels { false };
     bool dumpJSConsoleLogInStdErr { false };
 };

trunk/Tools/WebKitTestRunner/TestController.cpp

@@ -1481 +1481 @@
         m_currentInvocation->setIsPixelTest(command.expectedPixelHash);
 
+    if (command.forceDumpPixels)
+        m_currentInvocation->setForceDumpPixels(true);
+
     if (command.timeout > 0_s)
         m_currentInvocation->setCustomTimeout(command.timeout);

trunk/Tools/WebKitTestRunner/TestInvocation.cpp

@@ -830 +830 @@
 
     if (WKStringIsEqualToUTF8CString(messageName, "SetDumpPixels")) {
-        m_dumpPixels = booleanValue(messageBody);
+        m_dumpPixels = booleanValue(messageBody) || m_forceDumpPixels;
         return nullptr;
     }

trunk/Tools/WebKitTestRunner/TestInvocation.h

@@ -52 +52 @@
 
     void setIsPixelTest(const std::string& expectedPixelHash);
+    void setForceDumpPixels(bool forceDumpPixels) { m_forceDumpPixels = forceDumpPixels; }
 
     void setCustomTimeout(Seconds duration) { m_timeout = duration; }
@@ -165 +166 @@
     bool m_dumpFrameLoadCallbacks { false };
     bool m_dumpPixels { false };
+    bool m_forceDumpPixels { false };
     bool m_pixelResultIsPending { false };
     bool m_shouldDumpResourceLoadStatistics { false };