Skip to content

Commit 5e8ea43

Browse files
committed
test: add opt-in process WPT runner
To hopefully get to the bottom of WPT crashes that have no traces. Signed-off-by: Filip Skokan <panva.ip@gmail.com>
1 parent 81a445c commit 5e8ea43

10 files changed

Lines changed: 559 additions & 265 deletions

test/common/wpt.js

Lines changed: 168 additions & 69 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ const os = require('os');
1010
const { inspect } = require('util');
1111
const { pathToFileURL } = require('url');
1212
const { Worker } = require('worker_threads');
13+
const { fork } = require('child_process');
1314

1415
const workerPath = path.join(__dirname, 'wpt/worker.js');
1516

@@ -697,8 +698,107 @@ function getHarnessErrorName(harnessStatus) {
697698
return harnessStatus.message || 'WPT test harness error';
698699
}
699700

701+
/**
702+
* @typedef {object} SpecHandlers
703+
* @property {(message: object) => void} message Handles a message from the spec.
704+
* @property {(failure: { name: string, message: string, stack: string })
705+
* => boolean} failure Reports a spec that died without completing. Returns
706+
* false when the spec had already finished and the failure was ignored.
707+
*/
708+
709+
/**
710+
* @typedef {object} SpecHandle
711+
* @property {() => void} kill Forces the spec to stop running.
712+
* @property {Promise<unknown>} finished Settles once the spec has stopped.
713+
*/
714+
715+
/**
716+
* Run a spec on a worker thread.
717+
* @param {string[]} execArgv
718+
* @param {object} workerData
719+
* @param {SpecHandlers} handlers
720+
* @returns {SpecHandle}
721+
*/
722+
function runSpecOnThread(execArgv, workerData, handlers) {
723+
const worker = new Worker(workerPath, { execArgv, workerData });
724+
worker.on('message', handlers.message);
725+
worker.on('error', (err) => handlers.failure({
726+
name: `${err}`,
727+
message: err.message,
728+
stack: inspect(err),
729+
}));
730+
return {
731+
kill: () => worker.terminate(),
732+
finished: events.once(worker, 'exit').catch(() => {}),
733+
};
734+
}
735+
736+
/**
737+
* Run a spec in a child process, so that a spec crashing the process only
738+
* takes down its own run and the runner can attribute the crash to it.
739+
* @param {string[]} execArgv
740+
* @param {object} workerData
741+
* @param {SpecHandlers} handlers
742+
* @returns {SpecHandle}
743+
*/
744+
function runSpecInProcess(execArgv, workerData, handlers) {
745+
const child = fork(workerPath, {
746+
execArgv,
747+
// Status files may skip subtests by regular expression, which JSON
748+
// serialization would not preserve.
749+
serialization: 'advanced',
750+
stdio: ['ignore', 'inherit', 'pipe', 'ipc'],
751+
});
752+
child.send(workerData);
753+
754+
let stderr = '';
755+
child.stderr.setEncoding('utf8');
756+
child.stderr.on('data', (chunk) => {
757+
stderr += chunk;
758+
});
759+
760+
child.on('message', (message) => {
761+
// The spec reports uncaught errors itself so that they are named the same
762+
// way as they would be on the worker thread backend.
763+
if (message.type === 'uncaught') {
764+
handlers.failure(message.error);
765+
return;
766+
}
767+
handlers.message(message);
768+
});
769+
child.on('error', (err) => handlers.failure({
770+
name: `${err}`,
771+
message: err.message,
772+
stack: inspect(err),
773+
}));
774+
// `close` rather than `exit` so that everything the process wrote to stderr
775+
// on its way out is part of the reported failure.
776+
child.on('close', (code, signal) => {
777+
const name = signal ?
778+
`Test process was killed by signal ${signal}` :
779+
`Test process exited with code ${code}`;
780+
if (!handlers.failure({ name, message: name, stack: stderr }) && stderr) {
781+
process.stderr.write(stderr);
782+
}
783+
});
784+
785+
return {
786+
kill: () => child.kill('SIGKILL'),
787+
finished: events.once(child, 'close').catch(() => {}),
788+
};
789+
}
790+
791+
const backends = {
792+
__proto__: null,
793+
thread: runSpecOnThread,
794+
process: runSpecInProcess,
795+
};
796+
700797
class WPTRunner {
701-
constructor(path, { concurrency = os.availableParallelism() - 1 || 1 } = {}) {
798+
constructor(path, {
799+
concurrency = os.availableParallelism() - 1 || 1,
800+
backend = 'thread',
801+
} = {}) {
702802
// RISC-V has very limited virtual address space in the currently common
703803
// sv39 mode, in which we can only create a very limited number of wasm
704804
// memories(27 from a fresh node repl). Limit the concurrency to avoid
@@ -707,6 +807,15 @@ class WPTRunner {
707807
concurrency = Math.min(10, concurrency);
708808
}
709809

810+
// The override exists so that every suite can be run either way without
811+
// editing the drivers, which is how the two backends are kept compatible.
812+
backend = process.env.WPT_BACKEND || backend;
813+
this.runSpec = backends[backend];
814+
if (this.runSpec === undefined) {
815+
throw new Error(`Invalid WPT backend ${backend}, expected one of ` +
816+
`${Object.keys(backends).join(', ')}`);
817+
}
818+
710819
this.path = path;
711820
this.resource = new ResourceLoader(path);
712821
this.concurrency = concurrency;
@@ -726,7 +835,7 @@ class WPTRunner {
726835

727836
this.results = {};
728837
this.inProgress = new Set();
729-
this.workers = new Map();
838+
this.handles = new Map();
730839
this.unexpectedFailures = [];
731840
this.skippedSpecCount = 0;
732841

@@ -858,75 +967,64 @@ class WPTRunner {
858967
}
859968

860969
run(async () => {
861-
const worker = new Worker(workerPath, {
862-
execArgv: this.flags,
863-
workerData: {
864-
testRelativePath: relativePath,
865-
wptRunner: __filename,
866-
wptPath: this.path,
867-
initScript: this.fullInitScript(spec),
868-
harness: {
869-
code: fs.readFileSync(harnessPath, 'utf8'),
870-
filename: harnessPath,
871-
},
872-
scriptsToRun,
873-
// Set when the test runs inside an actual Web Worker.
874-
webWorker: isWebWorkerTest ? {
875-
path: absolutePath,
876-
isAnyTest,
877-
initScript: this.initScript,
878-
variant: spec.variant,
879-
scripts: meta.script?.map(
880-
(script) => this.resource.toRealFilePath(relativePath, script),
881-
) ?? [],
882-
skippedTests: spec.skippedTests,
883-
} : undefined,
884-
needsGc: !!meta.script?.find((script) => script === '/common/gc.js'),
885-
skippedTests: spec.skippedTests,
886-
},
887-
});
888970
this.inProgress.add(spec);
889-
this.workers.set(spec, worker);
890-
891971
const reportResult = this.report?.getResult(spec);
892-
worker.on('message', (message) => {
893-
switch (message.type) {
894-
case 'result':
895-
return this.resultCallback(spec, message.result, reportResult);
896-
case 'skip':
897-
return this.skipTest(spec, { name: message.name }, reportResult);
898-
case 'completion':
899-
return this.completionCallback(spec, message.status, reportResult);
900-
default:
901-
throw new Error(`Unexpected message from worker: ${message.type}`);
902-
}
903-
});
904972

905-
worker.on('error', (err) => {
906-
if (!this.inProgress.has(spec)) {
907-
// The test is already finished. Ignore errors that occur after it.
908-
// This can happen normally, for example in timers tests.
909-
return;
910-
}
911-
// Generate a subtest failure for visibility.
912-
// No need to record this synthetic failure with wpt.fyi.
913-
this.fail(
914-
spec,
915-
{
916-
status: NODE_UNCAUGHT,
917-
name: `${err}`,
918-
message: err.message,
919-
stack: inspect(err),
920-
},
921-
kUncaught,
922-
);
923-
// Mark the whole test as failed in wpt.fyi report.
924-
reportResult?.finish('ERROR');
925-
this.inProgress.delete(spec);
926-
this.report?.write();
973+
const handle = this.runSpec(this.flags, {
974+
testRelativePath: relativePath,
975+
wptRunner: __filename,
976+
wptPath: this.path,
977+
initScript: this.fullInitScript(spec),
978+
harness: {
979+
code: fs.readFileSync(harnessPath, 'utf8'),
980+
filename: harnessPath,
981+
},
982+
scriptsToRun,
983+
// Set when the test runs inside an actual Web Worker.
984+
webWorker: isWebWorkerTest ? {
985+
path: absolutePath,
986+
isAnyTest,
987+
initScript: this.initScript,
988+
variant: spec.variant,
989+
scripts: meta.script?.map(
990+
(script) => this.resource.toRealFilePath(relativePath, script),
991+
) ?? [],
992+
skippedTests: spec.skippedTests,
993+
} : undefined,
994+
needsGc: !!meta.script?.find((script) => script === '/common/gc.js'),
995+
skippedTests: spec.skippedTests,
996+
}, {
997+
message: (message) => {
998+
switch (message.type) {
999+
case 'result':
1000+
return this.resultCallback(spec, message.result, reportResult);
1001+
case 'skip':
1002+
return this.skipTest(spec, { name: message.name }, reportResult);
1003+
case 'completion':
1004+
return this.completionCallback(spec, message.status, reportResult);
1005+
default:
1006+
throw new Error(`Unexpected message from spec runner: ${message.type}`);
1007+
}
1008+
},
1009+
failure: (failure) => {
1010+
if (!this.inProgress.has(spec)) {
1011+
// The test is already finished. Ignore anything that happens
1012+
// after it, including the runner terminating it itself.
1013+
return false;
1014+
}
1015+
// Generate a subtest failure for visibility.
1016+
// No need to record this synthetic failure with wpt.fyi.
1017+
this.fail(spec, { status: NODE_UNCAUGHT, ...failure }, kUncaught);
1018+
// Mark the whole test as failed in wpt.fyi report.
1019+
reportResult?.finish('ERROR');
1020+
this.inProgress.delete(spec);
1021+
this.report?.write();
1022+
return true;
1023+
},
9271024
});
1025+
this.handles.set(spec, handle);
9281026

929-
await events.once(worker, 'exit').catch(() => {});
1027+
await handle.finished;
9301028
});
9311029
}
9321030

@@ -1061,9 +1159,9 @@ class WPTRunner {
10611159
// Write report incrementally so results survive even if the process
10621160
// is killed before the exit handler runs.
10631161
this.report?.write();
1064-
// Always force termination of the worker. Some tests allocate resources
1065-
// that would otherwise keep it alive.
1066-
this.workers.get(spec).terminate();
1162+
// Always force termination of the spec runner. Some tests allocate
1163+
// resources that would otherwise keep it alive.
1164+
this.handles.get(spec).kill();
10671165
}
10681166

10691167
addTestResult(spec, item) {
@@ -1201,6 +1299,7 @@ class WPTRunner {
12011299
}
12021300

12031301
module.exports = {
1302+
backends,
12041303
getHarnessErrorName,
12051304
getUnexpectedPasses,
12061305
harness: harnessMock,

0 commit comments

Comments
 (0)