Skip to content

Commit

Permalink
[wptrunner] Downgrade URL validation in content shell driver (#39036)
Browse files Browse the repository at this point in the history
See the comment in `testharnessreport-content-shell.js`. URL mismatches
will no longer be treated as a harness `ERROR`.

For the URLs to match as much as possible, this change partially undoes
#39007, which broke validation for final URLs with
fragments. Instead, we use the standard `URL(...)` instead of `<a>` to
parse the original `location.href`.
  • Loading branch information
jonathan-j-lee authored Mar 16, 2023
1 parent 286d2ea commit 071134c
Show file tree
Hide file tree
Showing 2 changed files with 52 additions and 21 deletions.
11 changes: 10 additions & 1 deletion tools/wptrunner/wptrunner/executors/executorcontentshell.py
Original file line number Diff line number Diff line change
Expand Up @@ -264,6 +264,15 @@ def do_test(self, test):
if not text:
return (test.result_cls("ERROR", errors), [])

return self.convert_result(test, json.loads(text))
result_url, status, message, stack, subtest_results = json.loads(text)
if result_url != test.url:
# Suppress `convert_result`'s URL validation.
# See `testharnessreport-content-shell.js` for details.
self.logger.warning('Got results from %s, expected %s' % (result_url, test.url))
self.logger.warning('URL mismatch may be a false positive '
'if the test navigates')
result_url = test.url
raw_result = result_url, status, message, stack, subtest_results
return self.convert_result(test, raw_result)
except BaseException as exception:
return _convert_exception(test, exception, self.protocol.content_shell_errors.read_errors())
62 changes: 42 additions & 20 deletions tools/wptrunner/wptrunner/testharnessreport-content-shell.js
Original file line number Diff line number Diff line change
@@ -1,23 +1,45 @@
var props = {output:%(output)d, debug: %(debug)s};
setup(props);
(function() {
var props = {output:%(output)d, debug: %(debug)s};
setup(props);

testRunner.dumpAsText();
testRunner.waitUntilDone();
testRunner.setPopupBlockingEnabled(false);
testRunner.setDumpJavaScriptDialogs(false);
// Some tests navigate away from the original URL as part of the
// functionality they exercise. In that case, `add_completion_callback(...)`
// uses the final `window.location` to report the test ID, which may not be
// correct [1].
//
// Persisting the original `window.location` with standard web platform APIs
// (e.g., `localStorage`) could interfere with the their tests, so this must
// be avoided. Unfortunately, there doesn't appear to be anything in content
// shell's protocol mode or Blink-specific `window.testRunner` or
// `window.internals` [2] that could help with this. As such, the driver
// simply downgrades a mismatched test ID to a logged warning instead of a
// harness error.
//
// [1] crbug.com/1418753
// [2] https://chromium.googlesource.com/chromium/src/+/refs/heads/main/docs/testing/writing_web_tests.md#Relying-on-Blink_Specific-Testing-APIs
const url = new URL(location.href);

add_completion_callback(function (tests, harness_status) {
var id = decodeURIComponent(location.pathname) + decodeURIComponent(location.search) + decodeURIComponent(location.hash);
var result_string = JSON.stringify([
id,
harness_status.status,
harness_status.message,
harness_status.stack,
tests.map(function(t) {
return [t.name, t.status, t.message, t.stack]
}),
]);
testRunner.dumpAsText();
testRunner.waitUntilDone();
testRunner.setPopupBlockingEnabled(false);
testRunner.setDumpJavaScriptDialogs(false);
// Show `CONSOLE MESSAGE:` and `CONSOLE ERROR:` in stderr.
if (props.debug) {
testRunner.setDumpConsoleMessages(true);
}

testRunner.setCustomTextOutput(result_string);
testRunner.notifyDone();
});
add_completion_callback(function (tests, harness_status) {
const test_id = decodeURIComponent(url.pathname) + decodeURIComponent(url.search) + decodeURIComponent(url.hash);
const result_string = JSON.stringify([
test_id,
harness_status.status,
harness_status.message,
harness_status.stack,
tests.map(function(t) {
return [t.name, t.status, t.message, t.stack]
}),
]);
testRunner.setCustomTextOutput(result_string);
testRunner.notifyDone();
});
})();

0 comments on commit 071134c

Please sign in to comment.