Diff
Modified: trunk/Tools/ChangeLog (285131 => 285132)
--- trunk/Tools/ChangeLog 2021-11-01 21:24:24 UTC (rev 285131)
+++ trunk/Tools/ChangeLog 2021-11-01 21:30:38 UTC (rev 285132)
@@ -1,3 +1,62 @@
+2021-11-01 Tim Horton <timothy_hor...@apple.com>
+
+ Add a run-webkit-tests mode to A/B test a given feature
+ https://bugs.webkit.org/show_bug.cgi?id=232553
+
+ Reviewed by Jonathan Bedard.
+
+ Add the argument --self-compare-with-header to run-webkit-tests, which
+ can be used to test the impact of a given feature (or set of features;
+ it accepts the standard test features header format).
+
+ When tests are run in this mode, all tests are run in the ref-test
+ style, but with the `expected` and `actual` results loading the same
+ test file (ignoring the usual -expected.html or whatever); they differ
+ only in the set of features/preferences enabled.
+
+ This is especially useful for testing the impact of e.g. platform
+ graphics features, where the difference between the shipping behavior
+ and in-development behavior is more interesting than whether or not
+ it actually makes the tests, as written, fail.
+
+ * Scripts/webkitpy/layout_tests/controllers/single_test_runner.py:
+ (SingleTestRunner):
+ (SingleTestRunner._run_comparison_test):
+ Add the comparison test runner, and prefer it if requested.
+
+ One note here: the run with the options derived from the given header is
+ considered the "actual" result and the default configuration the "expected".
+
+ * Scripts/webkitpy/layout_tests/run_webkit_tests.py:
+ (parse_args):
+ * Scripts/webkitpy/port/driver.py:
+ (DriverInput.__init__):
+ (DriverInput.__repr__):
+ (Driver._command_from_driver_input):
+ Pass the comparison test header along to the test runner.
+
+ Also, fix a longstanding error where --dump-jsconsolelog-in-stderr
+ could get inserted immediately after --pixel-test, causing the test runner
+ to consume it as the expected image hash! And leave a comment so nobody
+ else has to debug this again...
+
+ * TestRunnerShared/TestCommand.cpp:
+ (WTR::parseInputLine):
+ * TestRunnerShared/TestCommand.h:
+ * TestRunnerShared/TestFeatures.cpp:
+ (WTR::parseTestHeaderString):
+ (WTR::parseTestHeader):
+ (WTR::featureDefaultsFromComparisonTestHeader):
+ Factor out the parsing of the part of the test header inside the [ ],
+ since we use this format for the value of --self-compare-with-header as well.
+
+ * TestRunnerShared/TestFeatures.h:
+ * WebKitTestRunner/Options.h:
+ * WebKitTestRunner/TestController.cpp:
+ (WTR::TestController::testOptionsForTest const):
+ Merge the comparison header's options in to the test options before
+ the test's own header, so that the comparison header wins.
+
2021-11-01 Jonathan Bedard <jbed...@apple.com>
webkitpy: Remove obsolete port name
Modified: trunk/Tools/Scripts/webkitpy/layout_tests/controllers/single_test_runner.py (285131 => 285132)
--- trunk/Tools/Scripts/webkitpy/layout_tests/controllers/single_test_runner.py 2021-11-01 21:24:24 UTC (rev 285131)
+++ trunk/Tools/Scripts/webkitpy/layout_tests/controllers/single_test_runner.py 2021-11-01 21:30:38 UTC (rev 285132)
@@ -111,6 +111,9 @@
return DriverInput(self._test_name, self._timeout, image_hash, self._should_run_pixel_test, self._should_dump_jsconsolelog_in_stderr)
def run(self):
+ self_comparison_header = self._port.get_option('self_compare_with_header')
+ if self_comparison_header:
+ return self._run_self_comparison_test(self_comparison_header)
if self._reference_files:
if self._port.get_option('no_ref_tests') or self._options.reset_results:
reftest_type = set([reference_file[0] for reference_file in self._reference_files])
@@ -336,6 +339,21 @@
reftest_type = set([reference_file[0] for reference_file in self._reference_files])
return TestResult(self._test_input, test_result.failures, total_test_time + test_result.test_run_time, test_result.has_stderr, reftest_type=reftest_type, pid=test_result.pid, references=reference_test_names)
+ def _run_self_comparison_test(self, header):
+ driver_input = self._driver_input()
+ driver_input.should_run_pixel_test = True
+
+ reference_output = self._driver.run_test(driver_input, self._stop_when_done)
+ driver_input.self_comparison_header = header
+ test_output = self._driver.run_test(driver_input, self._stop_when_done)
+
+ test_full_path = self._port.abspath_for_test(self._test_name)
+ test_result = self._compare_output_with_reference(reference_output, test_output, test_full_path, False)
+
+ assert(reference_output)
+ test_result_writer.write_test_result(self._filesystem, self._port, self._results_directory, self._test_name, test_output, reference_output, test_result.failures)
+ return TestResult(self._test_input, test_result.failures, test_result.test_run_time, test_result.has_stderr, pid=test_result.pid)
+
@staticmethod
def _relative_reference_path(test_full_path, reference_full_path):
test_dir = os.path.split(test_full_path)[0]
Modified: trunk/Tools/Scripts/webkitpy/layout_tests/run_webkit_tests.py (285131 => 285132)
--- trunk/Tools/Scripts/webkitpy/layout_tests/run_webkit_tests.py 2021-11-01 21:24:24 UTC (rev 285131)
+++ trunk/Tools/Scripts/webkitpy/layout_tests/run_webkit_tests.py 2021-11-01 21:30:38 UTC (rev 285132)
@@ -343,7 +343,8 @@
optparse.make_option(
"--prefer-integrated-gpu", action="" default=False,
help=("Prefer using the lower-power integrated GPU on a dual-GPU system. Note that other running applications and the tests themselves can override this request.")),
- optparse.make_option('--show-window', action="" default=False, help="Make the test runner window visible during testing."),
+ optparse.make_option("--show-window", action="" default=False, help="Make the test runner window visible during testing."),
+ optparse.make_option("--self-compare-with-header", help="Run all tests as A/B tests between the default configuration and the given test features header (ignoring expected results)."),
]))
option_group_definitions.append(("Web Platform Test Server Options", [
Modified: trunk/Tools/Scripts/webkitpy/port/driver.py (285131 => 285132)
--- trunk/Tools/Scripts/webkitpy/port/driver.py 2021-11-01 21:24:24 UTC (rev 285131)
+++ trunk/Tools/Scripts/webkitpy/port/driver.py 2021-11-01 21:30:38 UTC (rev 285132)
@@ -46,7 +46,7 @@
class DriverInput(object):
- def __init__(self, test_name, timeout, image_hash, should_run_pixel_test, should_dump_jsconsolelog_in_stderr=None, args=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):
self.test_name = test_name
self.timeout = timeout # in ms
self.image_hash = image_hash
@@ -53,9 +53,10 @@
self.should_run_pixel_test = should_run_pixel_test
self.should_dump_jsconsolelog_in_stderr = should_dump_jsconsolelog_in_stderr
self.args = args or []
+ self.self_comparison_header = self_comparison_header
def __repr__(self):
- return "DriverInput(test_name='{}', timeout={}, image_hash={}, should_run_pixel_test={}, should_dump_jsconsolelog_in_stderr={}'".format(self.test_name, self.timeout, self.image_hash, self.should_run_pixel_test, self.should_dump_jsconsolelog_in_stderr)
+ 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)
class DriverOutput(object):
@@ -631,12 +632,17 @@
# ' is the separator between arguments.
if self._port.supports_per_test_timeout():
command += "'--timeout'%s" % driver_input.timeout
+ if driver_input.should_dump_jsconsolelog_in_stderr:
+ command += "'--dump-jsconsolelog-in-stderr"
+ if driver_input.self_comparison_header:
+ command += "'--self-compare-with-header'%s" % driver_input.self_comparison_header
+
+ # --pixel-test must be the last argument, because the hash is optional,
+ # and any argument put in its place will be incorrectly consumed as the hash.
if driver_input.should_run_pixel_test:
command += "'--pixel-test"
- if driver_input.should_dump_jsconsolelog_in_stderr:
- command += "'--dump-jsconsolelog-in-stderr"
- if driver_input.image_hash:
- command += "'" + driver_input.image_hash
+ if driver_input.image_hash:
+ command += "'" + driver_input.image_hash
return command + "\n"
def _read_first_block(self, deadline, test_name):
Modified: trunk/Tools/TestRunnerShared/TestCommand.cpp (285131 => 285132)
--- trunk/Tools/TestRunnerShared/TestCommand.cpp 2021-11-01 21:24:24 UTC (rev 285131)
+++ trunk/Tools/TestRunnerShared/TestCommand.cpp 2021-11-01 21:30:38 UTC (rev 285132)
@@ -98,6 +98,11 @@
result.shouldDumpPixels = true;
if (tokenizer.hasNext())
result.expectedPixelHash = tokenizer.next();
+ } else if (arg == "--self-compare-with-header") {
+ if (tokenizer.hasNext())
+ result.selfComparisonHeader = tokenizer.next();
+ else
+ die(inputLine);
} else if (arg == std::string("--dump-jsconsolelog-in-stderr"))
result.dumpJSConsoleLogInStdErr = true;
else if (arg == std::string("--absolutePath"))
Modified: trunk/Tools/TestRunnerShared/TestCommand.h (285131 => 285132)
--- trunk/Tools/TestRunnerShared/TestCommand.h 2021-11-01 21:24:24 UTC (rev 285131)
+++ trunk/Tools/TestRunnerShared/TestCommand.h 2021-11-01 21:30:38 UTC (rev 285132)
@@ -35,6 +35,7 @@
std::string pathOrURL;
std::filesystem::path absolutePath;
std::string expectedPixelHash;
+ std::string selfComparisonHeader;
WTF::Seconds timeout;
bool shouldDumpPixels { false };
bool dumpJSConsoleLogInStdErr { false };
Modified: trunk/Tools/TestRunnerShared/TestFeatures.cpp (285131 => 285132)
--- trunk/Tools/TestRunnerShared/TestFeatures.cpp 2021-11-01 21:24:24 UTC (rev 285131)
+++ trunk/Tools/TestRunnerShared/TestFeatures.cpp 2021-11-01 21:30:38 UTC (rev 285132)
@@ -243,32 +243,10 @@
return false;
}
-static TestFeatures parseTestHeader(std::filesystem::path path, const std::unordered_map<std::string, TestHeaderKeyType>& keyTypeMap)
+static TestFeatures parseTestHeaderString(const std::string& pairString, std::filesystem::path path, const std::unordered_map<std::string, TestHeaderKeyType>& keyTypeMap)
{
TestFeatures features;
- std::error_code ec;
- if (!std::filesystem::exists(path, ec))
- return features;
- std::ifstream file(path);
- if (!file.good()) {
- LOG_ERROR("Could not open file to inspect test headers in %s", path.c_str());
- return features;
- }
-
- std::string options;
- getline(file, options);
- std::string beginString("webkit-test-runner [ ");
- std::string endString(" ]");
- size_t beginLocation = options.find(beginString);
- if (beginLocation == std::string::npos)
- return features;
- size_t endLocation = options.find(endString, beginLocation);
- if (endLocation == std::string::npos) {
- LOG_ERROR("Could not find end of test header in %s", path.c_str());
- return features;
- }
- std::string pairString = options.substr(beginLocation + beginString.size(), endLocation - (beginLocation + beginString.size()));
size_t pairStart = 0;
while (pairStart < pairString.size()) {
size_t pairEnd = pairString.find(" ", pairStart);
@@ -291,9 +269,44 @@
return features;
}
+static TestFeatures parseTestHeader(std::filesystem::path path, const std::unordered_map<std::string, TestHeaderKeyType>& keyTypeMap)
+{
+ std::error_code ec;
+ if (!std::filesystem::exists(path, ec))
+ return { };
+
+ std::ifstream file(path);
+ if (!file.good()) {
+ LOG_ERROR("Could not open file to inspect test headers in %s", path.c_str());
+ return { };
+ }
+
+ std::string options;
+ getline(file, options);
+ std::string beginString("webkit-test-runner [ ");
+ std::string endString(" ]");
+ size_t beginLocation = options.find(beginString);
+ if (beginLocation == std::string::npos)
+ return { };
+ size_t endLocation = options.find(endString, beginLocation);
+ if (endLocation == std::string::npos) {
+ LOG_ERROR("Could not find end of test header in %s", path.c_str());
+ return { };
+ }
+ std::string pairString = options.substr(beginLocation + beginString.size(), endLocation - (beginLocation + beginString.size()));
+ return parseTestHeaderString(pairString, path, keyTypeMap);
+}
+
TestFeatures featureDefaultsFromTestHeaderForTest(const TestCommand& command, const std::unordered_map<std::string, TestHeaderKeyType>& keyTypeMap)
{
return parseTestHeader(command.absolutePath, keyTypeMap);
}
+TestFeatures featureDefaultsFromSelfComparisonHeader(const TestCommand& command, const std::unordered_map<std::string, TestHeaderKeyType>& keyTypeMap)
+{
+ if (command.selfComparisonHeader.empty())
+ return { };
+ return parseTestHeaderString(command.selfComparisonHeader, command.absolutePath, keyTypeMap);
}
+
+} // namespace WTF
Modified: trunk/Tools/TestRunnerShared/TestFeatures.h (285131 => 285132)
--- trunk/Tools/TestRunnerShared/TestFeatures.h 2021-11-01 21:24:24 UTC (rev 285131)
+++ trunk/Tools/TestRunnerShared/TestFeatures.h 2021-11-01 21:30:38 UTC (rev 285132)
@@ -69,5 +69,6 @@
Unknown
};
TestFeatures featureDefaultsFromTestHeaderForTest(const TestCommand&, const std::unordered_map<std::string, TestHeaderKeyType>&);
+TestFeatures featureDefaultsFromSelfComparisonHeader(const TestCommand&, const std::unordered_map<std::string, TestHeaderKeyType>&);
}
Modified: trunk/Tools/WebKitTestRunner/TestController.cpp (285131 => 285132)
--- trunk/Tools/WebKitTestRunner/TestController.cpp 2021-11-01 21:24:24 UTC (rev 285131)
+++ trunk/Tools/WebKitTestRunner/TestController.cpp 2021-11-01 21:30:38 UTC (rev 285132)
@@ -1311,6 +1311,7 @@
merge(features, m_globalFeatures);
merge(features, hardcodedFeaturesBasedOnPathForTest(command));
merge(features, platformSpecificFeatureDefaultsForTest(command));
+ merge(features, featureDefaultsFromSelfComparisonHeader(command, TestOptions::keyTypeMapping()));
merge(features, featureDefaultsFromTestHeaderForTest(command, TestOptions::keyTypeMapping()));
merge(features, platformSpecificFeatureOverridesDefaultsForTest(command));
