Skip to content

Commit

Permalink
Improve developer experience for web platform tests
Browse files Browse the repository at this point in the history
* Fix process shutdown on Windows to work consistently.

* Filter the logs a bit to make them more readable.

* Don't require manually marking .worker/.sharedworker/serviceworker variants as skipped in to-run.yaml.
  • Loading branch information
domenic committed May 2, 2023
1 parent 13ac01e commit 64db3ce
Show file tree
Hide file tree
Showing 7 changed files with 136 additions and 110 deletions.
4 changes: 2 additions & 2 deletions test/web-platform-tests/run-single-wpt.js
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,8 @@ module.exports = urlPrefixFactory => {
title,
expectPromise: true,
// WPT also takes care of timeouts (maximum 60 seconds), this is an extra failsafe:
timeout: 70000,
slow: 10000,
timeout: 70_000,
slow: 10_000,
fn() {
return createJSDOM(urlPrefixFactory(), testPath, expectFail);
}
Expand Down
8 changes: 4 additions & 4 deletions test/web-platform-tests/run-tuwpts.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ const path = require("path");
const { describe, before, after } = require("mocha-sugar-free");
const { spawnSync } = require("child_process");
const { readManifest, getPossibleTestFilePaths } = require("./wpt-manifest-utils.js");
const startWPTServer = require("./start-wpt-server.js");
const wptServer = require("./wpt-server.js");

const wptPath = path.resolve(__dirname, "tests");
const testsPath = path.resolve(__dirname, "to-upstream");
Expand All @@ -20,14 +20,14 @@ const possibleTestFilePaths = getPossibleTestFilePaths(manifest);

let wptServerURL, serverProcess;
const runSingleWPT = require("./run-single-wpt.js")(() => wptServerURL);
before({ timeout: 30 * 1000 }, async () => {
const { urls, subprocess } = await startWPTServer({ toUpstream: true });
before({ timeout: 30_000 }, async () => {
const { urls, subprocess } = await wptServer.start({ toUpstream: true });
wptServerURL = urls[0];
serverProcess = subprocess;
});

after(() => {
serverProcess.kill();
wptServer.kill(serverProcess);
});

describe("Local tests in web-platform-test format (to-upstream)", () => {
Expand Down
8 changes: 4 additions & 4 deletions test/web-platform-tests/run-wpts.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ const jsYAML = require("js-yaml");
const { Minimatch } = require("minimatch");
const { describe, specify, before, after } = require("mocha-sugar-free");
const { readManifest, getPossibleTestFilePaths } = require("./wpt-manifest-utils.js");
const startWPTServer = require("./start-wpt-server.js");
const wptServer = require("./wpt-server.js");
const { resolveReason } = require("./utils.js");

const validInnerReasons = new Set([
Expand Down Expand Up @@ -39,14 +39,14 @@ checkToRun();

let wptServerURL, serverProcess;
const runSingleWPT = require("./run-single-wpt.js")(() => wptServerURL);
before({ timeout: 30 * 1000 }, async () => {
const { urls, subprocess } = await startWPTServer({ toUpstream: false });
before({ timeout: 30_000 }, async () => {
const { urls, subprocess } = await wptServer.start({ toUpstream: false });
wptServerURL = urls[0];
serverProcess = subprocess;
});

after(() => {
serverProcess.kill("SIGINT");
wptServer.kill(serverProcess);
});

describe("web-platform-tests", () => {
Expand Down
91 changes: 0 additions & 91 deletions test/web-platform-tests/start-wpt-server.js

This file was deleted.

6 changes: 0 additions & 6 deletions test/web-platform-tests/to-run.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -1402,25 +1402,19 @@ idlharness.any.html: [fail, Depends on fetch]
percent-encoding.window.html: [fail, Depends on fetch]
toascii.window.html: [fail, Depends on fetch]
url-constructor.any.html**: [fail, Depends on fetch]
url-constructor.any.worker.html*: [fail, Depends on Worker]
url-origin.any.html: [fail, Depends on fetch]
url-setters-a-area.window.html**: [fail, Depends on fetch]
url-setters.any.html**: [fail, Depends on fetch]
url-setters.any.worker.html*: [fail, Depends on Worker]
urlencoded-parser.any.html: [fail, Depends on fetch]

---

DIR: websockets

"**/**?wpt_flags=h2": [fail-slow, HTTP/2 web sockets not implemented]
"*.any.sharedworker.html?wss": [fail-slow, Needs SharedWorker implementation]
"*.any.worker.html?wss": [fail-slow, Needs Worker implementation]
Create-blocked-port.any.html: [fail-slow, Not implemented]
Create-blocked-port.any.html?wss: [fail-slow, Not implemented]
Create-on-worker-shutdown.any.html: [fail, Needs Worker implementation]
Send-data.worker.html?wss: [fail-slow, Needs Worker implementation]
basic-auth.any.serviceworker.html?wss: [fail-slow, Unknown]
cookies/third-party-cookie-accepted.https.html: [fail, 'https://github.com/salesforce/tough-cookie/issues/80']
idlharness.any.html: [fail, Depends on fetch]
interfaces/WebSocket/close/close-connecting.html*: [fail, Potentially buggy test as Chrome fails it too]
Expand Down
4 changes: 1 addition & 3 deletions test/web-platform-tests/wpt-manifest-utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,7 @@ exports.getPossibleTestFilePaths = manifest => {
const testPath = curPath === null ? fallbackPath : curPath;

// Globally disable worker tests
if (testPath.endsWith(".worker.html") ||
testPath.endsWith(".serviceworker.html") ||
testPath.endsWith(".sharedworker.html")) {
if (/\.(?:shared|service)?worker\.html(?:\?.*)?$/.test(testPath)) {
continue;
}
// Globally disable testdriver tests
Expand Down
125 changes: 125 additions & 0 deletions test/web-platform-tests/wpt-server.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,125 @@
"use strict";
/* eslint-disable no-console, global-require */
const dns = require("dns").promises;
const path = require("path");
const childProcess = require("child_process");
const http = require("http");
const https = require("https");
const os = require("os");

const wptDir = path.resolve(__dirname, "tests");

const configPaths = {
default: path.resolve(__dirname, "wpt-config.json"),
toUpstream: path.resolve(__dirname, "tuwpt-config.json")
};

const configs = {
default: require(configPaths.default),
toUpstream: require(configPaths.toUpstream)
};

exports.start = async ({ toUpstream = false } = {}) => {
const configType = toUpstream ? "toUpstream" : "default";
const configPath = configPaths[configType];
const config = configs[configType];

try {
await dns.lookup("web-platform.test");
} catch {
throw new Error("Host entries not present for web platform tests. See " +
"https://web-platform-tests.org/running-tests/from-local-system.html#system-setup");
}

const configArg = path.relative(path.resolve(wptDir), configPath);
const args = ["./wpt.py", "serve", "--config", configArg];
const subprocess = childProcess.spawn("python", args, {
cwd: wptDir,
stdio: ["inherit", "pipe", "pipe"]
});

subprocess.stdout.filter(nonSpammyWPTLog).pipe(process.stdout);
subprocess.stderr.filter(nonSpammyWPTLog).pipe(process.stderr);

return new Promise((resolve, reject) => {
subprocess.on("error", e => {
reject(new Error("Error starting python server process:", e.message));
});

resolve(Promise.all([
pollForServer(`http://${config.browser_host}:${config.ports.http[0]}/`),
pollForServer(`https://${config.browser_host}:${config.ports.https[0]}/`),
pollForServer(`http://${config.browser_host}:${config.ports.ws[0]}/`),
pollForServer(`https://${config.browser_host}:${config.ports.wss[0]}/`)
]).then(urls => ({ urls, subprocess })));
});
};

exports.kill = subprocess => {
if (os.platform() === "win32") {
// subprocess.kill() doesn't seem to be able to kill descendant processes on Windows, at least with whatever's going
// on inside the web-platform-tests Python. Use this technique instead.
childProcess.spawnSync("taskkill", ["/F", "/T", "/PID", subprocess.pid], { detached: true, windowsHide: true });
} else {
// SIGINT is necessary so that the Python script can clean up its subprocesses.
subprocess.kill("SIGINT");
}
};

function pollForServer(url, lastLogTime = Date.now()) {
const agent = url.startsWith("https") ? new https.Agent({ rejectUnauthorized: false }) : null;
const { request } = url.startsWith("https") ? https : http;

// Using raw Node.js http/https modules is gross, but it's not worth pulling in something like node-fetch for just
// this one part of the test codebase.
return new Promise((resolve, reject) => {
const req = request(url, { method: "HEAD", agent }, res => {
if (res.statusCode < 200 || res.statusCode > 299) {
reject(new Error(`Unexpected status=${res.statusCode}`));
} else {
resolve(url);
}
});

req.on("error", reject);
req.end();
}).catch(err => {
// Only log every 5 seconds to be less spammy.
if (Date.now() - lastLogTime >= 5000) {
console.log(`WPT server at ${url} is not up yet (${err.message}); trying again`);
lastLogTime = Date.now();
}

return new Promise(resolve => {
setTimeout(() => resolve(pollForServer(url, lastLogTime)), 500);
});
});
}

function nonSpammyWPTLog(buffer) {
const string = buffer.toString("utf-8");

// Subprocess shutdown is uninteresting.
if (string.includes("Status of subprocess")) {
return false;
}
if (string.includes("INFO - Stopped")) {
return false;
}

// We'll get one message for each server startup. We don't need four more.
if (string.includes("INFO - Close on:") ||
string.includes("INFO - Bind on:") ||
string.includes("INFO - Listen on:") ||
string.includes("INFO - Create socket on:")) {
return false;
}

// Probing / on the ws and wss ports will cause a fallback, and log some messages about it.
// Those are expected and are uninteresting.
if (/wss? on port \d+\] INFO - (No handler|Fallback to|"HEAD \/ HTTP)/.test(string)) {
return false;
}

return true;
}

0 comments on commit 64db3ce

Please sign in to comment.