Skip to content

Commit 64db3ce

Browse files
committed
Improve developer experience for web platform tests
* 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.
1 parent 13ac01e commit 64db3ce

7 files changed

Lines changed: 136 additions & 110 deletions

File tree

test/web-platform-tests/run-single-wpt.js

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -21,8 +21,8 @@ module.exports = urlPrefixFactory => {
2121
title,
2222
expectPromise: true,
2323
// WPT also takes care of timeouts (maximum 60 seconds), this is an extra failsafe:
24-
timeout: 70000,
25-
slow: 10000,
24+
timeout: 70_000,
25+
slow: 10_000,
2626
fn() {
2727
return createJSDOM(urlPrefixFactory(), testPath, expectFail);
2828
}

test/web-platform-tests/run-tuwpts.js

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ const path = require("path");
33
const { describe, before, after } = require("mocha-sugar-free");
44
const { spawnSync } = require("child_process");
55
const { readManifest, getPossibleTestFilePaths } = require("./wpt-manifest-utils.js");
6-
const startWPTServer = require("./start-wpt-server.js");
6+
const wptServer = require("./wpt-server.js");
77

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

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

2929
after(() => {
30-
serverProcess.kill();
30+
wptServer.kill(serverProcess);
3131
});
3232

3333
describe("Local tests in web-platform-test format (to-upstream)", () => {

test/web-platform-tests/run-wpts.js

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ const jsYAML = require("js-yaml");
55
const { Minimatch } = require("minimatch");
66
const { describe, specify, before, after } = require("mocha-sugar-free");
77
const { readManifest, getPossibleTestFilePaths } = require("./wpt-manifest-utils.js");
8-
const startWPTServer = require("./start-wpt-server.js");
8+
const wptServer = require("./wpt-server.js");
99
const { resolveReason } = require("./utils.js");
1010

1111
const validInnerReasons = new Set([
@@ -39,14 +39,14 @@ checkToRun();
3939

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

4848
after(() => {
49-
serverProcess.kill("SIGINT");
49+
wptServer.kill(serverProcess);
5050
});
5151

5252
describe("web-platform-tests", () => {

test/web-platform-tests/start-wpt-server.js

Lines changed: 0 additions & 91 deletions
This file was deleted.

test/web-platform-tests/to-run.yaml

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1402,25 +1402,19 @@ idlharness.any.html: [fail, Depends on fetch]
14021402
percent-encoding.window.html: [fail, Depends on fetch]
14031403
toascii.window.html: [fail, Depends on fetch]
14041404
url-constructor.any.html**: [fail, Depends on fetch]
1405-
url-constructor.any.worker.html*: [fail, Depends on Worker]
14061405
url-origin.any.html: [fail, Depends on fetch]
14071406
url-setters-a-area.window.html**: [fail, Depends on fetch]
14081407
url-setters.any.html**: [fail, Depends on fetch]
1409-
url-setters.any.worker.html*: [fail, Depends on Worker]
14101408
urlencoded-parser.any.html: [fail, Depends on fetch]
14111409

14121410
---
14131411

14141412
DIR: websockets
14151413

14161414
"**/**?wpt_flags=h2": [fail-slow, HTTP/2 web sockets not implemented]
1417-
"*.any.sharedworker.html?wss": [fail-slow, Needs SharedWorker implementation]
1418-
"*.any.worker.html?wss": [fail-slow, Needs Worker implementation]
14191415
Create-blocked-port.any.html: [fail-slow, Not implemented]
14201416
Create-blocked-port.any.html?wss: [fail-slow, Not implemented]
14211417
Create-on-worker-shutdown.any.html: [fail, Needs Worker implementation]
1422-
Send-data.worker.html?wss: [fail-slow, Needs Worker implementation]
1423-
basic-auth.any.serviceworker.html?wss: [fail-slow, Unknown]
14241418
cookies/third-party-cookie-accepted.https.html: [fail, 'https://github.com/salesforce/tough-cookie/issues/80']
14251419
idlharness.any.html: [fail, Depends on fetch]
14261420
interfaces/WebSocket/close/close-connecting.html*: [fail, Potentially buggy test as Chrome fails it too]

test/web-platform-tests/wpt-manifest-utils.js

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -17,9 +17,7 @@ exports.getPossibleTestFilePaths = manifest => {
1717
const testPath = curPath === null ? fallbackPath : curPath;
1818

1919
// Globally disable worker tests
20-
if (testPath.endsWith(".worker.html") ||
21-
testPath.endsWith(".serviceworker.html") ||
22-
testPath.endsWith(".sharedworker.html")) {
20+
if (/\.(?:shared|service)?worker\.html(?:\?.*)?$/.test(testPath)) {
2321
continue;
2422
}
2523
// Globally disable testdriver tests
Lines changed: 125 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,125 @@
1+
"use strict";
2+
/* eslint-disable no-console, global-require */
3+
const dns = require("dns").promises;
4+
const path = require("path");
5+
const childProcess = require("child_process");
6+
const http = require("http");
7+
const https = require("https");
8+
const os = require("os");
9+
10+
const wptDir = path.resolve(__dirname, "tests");
11+
12+
const configPaths = {
13+
default: path.resolve(__dirname, "wpt-config.json"),
14+
toUpstream: path.resolve(__dirname, "tuwpt-config.json")
15+
};
16+
17+
const configs = {
18+
default: require(configPaths.default),
19+
toUpstream: require(configPaths.toUpstream)
20+
};
21+
22+
exports.start = async ({ toUpstream = false } = {}) => {
23+
const configType = toUpstream ? "toUpstream" : "default";
24+
const configPath = configPaths[configType];
25+
const config = configs[configType];
26+
27+
try {
28+
await dns.lookup("web-platform.test");
29+
} catch {
30+
throw new Error("Host entries not present for web platform tests. See " +
31+
"https://web-platform-tests.org/running-tests/from-local-system.html#system-setup");
32+
}
33+
34+
const configArg = path.relative(path.resolve(wptDir), configPath);
35+
const args = ["./wpt.py", "serve", "--config", configArg];
36+
const subprocess = childProcess.spawn("python", args, {
37+
cwd: wptDir,
38+
stdio: ["inherit", "pipe", "pipe"]
39+
});
40+
41+
subprocess.stdout.filter(nonSpammyWPTLog).pipe(process.stdout);
42+
subprocess.stderr.filter(nonSpammyWPTLog).pipe(process.stderr);
43+
44+
return new Promise((resolve, reject) => {
45+
subprocess.on("error", e => {
46+
reject(new Error("Error starting python server process:", e.message));
47+
});
48+
49+
resolve(Promise.all([
50+
pollForServer(`http://${config.browser_host}:${config.ports.http[0]}/`),
51+
pollForServer(`https://${config.browser_host}:${config.ports.https[0]}/`),
52+
pollForServer(`http://${config.browser_host}:${config.ports.ws[0]}/`),
53+
pollForServer(`https://${config.browser_host}:${config.ports.wss[0]}/`)
54+
]).then(urls => ({ urls, subprocess })));
55+
});
56+
};
57+
58+
exports.kill = subprocess => {
59+
if (os.platform() === "win32") {
60+
// subprocess.kill() doesn't seem to be able to kill descendant processes on Windows, at least with whatever's going
61+
// on inside the web-platform-tests Python. Use this technique instead.
62+
childProcess.spawnSync("taskkill", ["/F", "/T", "/PID", subprocess.pid], { detached: true, windowsHide: true });
63+
} else {
64+
// SIGINT is necessary so that the Python script can clean up its subprocesses.
65+
subprocess.kill("SIGINT");
66+
}
67+
};
68+
69+
function pollForServer(url, lastLogTime = Date.now()) {
70+
const agent = url.startsWith("https") ? new https.Agent({ rejectUnauthorized: false }) : null;
71+
const { request } = url.startsWith("https") ? https : http;
72+
73+
// Using raw Node.js http/https modules is gross, but it's not worth pulling in something like node-fetch for just
74+
// this one part of the test codebase.
75+
return new Promise((resolve, reject) => {
76+
const req = request(url, { method: "HEAD", agent }, res => {
77+
if (res.statusCode < 200 || res.statusCode > 299) {
78+
reject(new Error(`Unexpected status=${res.statusCode}`));
79+
} else {
80+
resolve(url);
81+
}
82+
});
83+
84+
req.on("error", reject);
85+
req.end();
86+
}).catch(err => {
87+
// Only log every 5 seconds to be less spammy.
88+
if (Date.now() - lastLogTime >= 5000) {
89+
console.log(`WPT server at ${url} is not up yet (${err.message}); trying again`);
90+
lastLogTime = Date.now();
91+
}
92+
93+
return new Promise(resolve => {
94+
setTimeout(() => resolve(pollForServer(url, lastLogTime)), 500);
95+
});
96+
});
97+
}
98+
99+
function nonSpammyWPTLog(buffer) {
100+
const string = buffer.toString("utf-8");
101+
102+
// Subprocess shutdown is uninteresting.
103+
if (string.includes("Status of subprocess")) {
104+
return false;
105+
}
106+
if (string.includes("INFO - Stopped")) {
107+
return false;
108+
}
109+
110+
// We'll get one message for each server startup. We don't need four more.
111+
if (string.includes("INFO - Close on:") ||
112+
string.includes("INFO - Bind on:") ||
113+
string.includes("INFO - Listen on:") ||
114+
string.includes("INFO - Create socket on:")) {
115+
return false;
116+
}
117+
118+
// Probing / on the ws and wss ports will cause a fallback, and log some messages about it.
119+
// Those are expected and are uninteresting.
120+
if (/wss? on port \d+\] INFO - (No handler|Fallback to|"HEAD \/ HTTP)/.test(string)) {
121+
return false;
122+
}
123+
124+
return true;
125+
}

0 commit comments

Comments
 (0)