Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

Commit 7ed7ef7

Browse filesBrowse files
targosBethGriggs
targos
authored and
BethGriggs
committed
test: move execution of WPT to worker threads
Running outside of the main Node.js context prevents us from upgrading the WPT harness because new versions more aggressively check the identity of globals like error constructors. Instead of exposing globals used by the tests on vm sandboxes, use worker threads to run everything. PR-URL: #34796 Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
1 parent 47f2f45 commit 7ed7ef7
Copy full SHA for 7ed7ef7

File tree

Expand file treeCollapse file tree

10 files changed

+141
-161
lines changed
Open diff view settings
Filter options
Expand file treeCollapse file tree

10 files changed

+141
-161
lines changed
Open diff view settings
Collapse file

‎test/common/README.md‎

Copy file name to clipboardExpand all lines: test/common/README.md
+1-1Lines changed: 1 addition & 1 deletion
  • Display the source diff
  • Display the rich diff
Original file line numberDiff line numberDiff line change
@@ -962,7 +962,7 @@ the original WPT harness, see [the WPT tests README][].
962962

963963
### Class: WPTRunner
964964

965-
A driver class for running WPT with the WPT harness in a vm.
965+
A driver class for running WPT with the WPT harness in a worker thread.
966966

967967
See [the WPT tests README][] for details.
968968

Collapse file

‎test/common/wpt.js‎

Copy file name to clipboardExpand all lines: test/common/wpt.js
+54-92Lines changed: 54 additions & 92 deletions
Original file line numberDiff line numberDiff line change
@@ -6,8 +6,8 @@ const fixtures = require('../common/fixtures');
66
const fs = require('fs');
77
const fsPromises = fs.promises;
88
const path = require('path');
9-
const vm = require('vm');
109
const { inspect } = require('util');
10+
const { Worker } = require('worker_threads');
1111

1212
// https://github.com/w3c/testharness.js/blob/master/testharness.js
1313
// TODO: get rid of this half-baked harness in favor of the one
@@ -222,7 +222,6 @@ class IntlRequirement {
222222

223223
const intlRequirements = new IntlRequirement();
224224

225-
226225
class StatusLoader {
227226
/**
228227
* @param {string} path relative path of the WPT subset
@@ -287,10 +286,9 @@ class WPTRunner {
287286
constructor(path) {
288287
this.path = path;
289288
this.resource = new ResourceLoader(path);
290-
this.sandbox = null;
291-
this.context = null;
292289

293-
this.globals = new Map();
290+
this.flags = [];
291+
this.initScript = null;
294292

295293
this.status = new StatusLoader(path);
296294
this.status.load();
@@ -304,28 +302,19 @@ class WPTRunner {
304302
}
305303

306304
/**
307-
* Specify that certain global descriptors from the object
308-
* should be defined in the vm
309-
* @param {object} obj
310-
* @param {string[]} names
305+
* Sets the Node.js flags passed to the worker.
306+
* @param {Array<string>} flags
311307
*/
312-
copyGlobalsFromObject(obj, names) {
313-
for (const name of names) {
314-
const desc = Object.getOwnPropertyDescriptor(obj, name);
315-
if (!desc) {
316-
assert.fail(`${name} does not exist on the object`);
317-
}
318-
this.globals.set(name, desc);
319-
}
308+
setFlags(flags) {
309+
this.flags = flags;
320310
}
321311

322312
/**
323-
* Specify that certain global descriptors should be defined in the vm
324-
* @param {string} name
325-
* @param {object} descriptor
313+
* Sets a script to be run in the worker before executing the tests.
314+
* @param {string} script
326315
*/
327-
defineGlobal(name, descriptor) {
328-
this.globals.set(name, descriptor);
316+
setInitScript(script) {
317+
this.initScript = script;
329318
}
330319

331320
// TODO(joyeecheung): work with the upstream to port more tests in .html
@@ -353,8 +342,8 @@ class WPTRunner {
353342
const meta = spec.title = this.getMeta(content);
354343

355344
const absolutePath = spec.getAbsolutePath();
356-
const context = this.generateContext(spec);
357345
const relativePath = spec.getRelativePath();
346+
const harnessPath = fixtures.path('wpt', 'resources', 'testharness.js');
358347
const scriptsToRun = [];
359348
// Scripts specified with the `// META: script=` header
360349
if (meta.script) {
@@ -371,24 +360,46 @@ class WPTRunner {
371360
filename: absolutePath
372361
});
373362

374-
for (const { code, filename } of scriptsToRun) {
375-
try {
376-
vm.runInContext(code, context, { filename });
377-
} catch (err) {
378-
this.fail(
379-
testFileName,
380-
{
381-
status: NODE_UNCAUGHT,
382-
name: 'evaluation in WPTRunner.runJsTests()',
383-
message: err.message,
384-
stack: inspect(err)
385-
},
386-
kUncaught
387-
);
388-
this.inProgress.delete(filename);
389-
break;
363+
const workerPath = path.join(__dirname, 'wpt/worker.js');
364+
const worker = new Worker(workerPath, {
365+
execArgv: this.flags,
366+
workerData: {
367+
filename: testFileName,
368+
wptRunner: __filename,
369+
wptPath: this.path,
370+
initScript: this.initScript,
371+
harness: {
372+
code: fs.readFileSync(harnessPath, 'utf8'),
373+
filename: harnessPath,
374+
},
375+
scriptsToRun,
376+
},
377+
});
378+
379+
worker.on('message', (message) => {
380+
switch (message.type) {
381+
case 'result':
382+
return this.resultCallback(testFileName, message.result);
383+
case 'completion':
384+
return this.completionCallback(testFileName, message.status);
385+
default:
386+
throw new Error(`Unexpected message from worker: ${message.type}`);
390387
}
391-
}
388+
});
389+
390+
worker.on('error', (err) => {
391+
this.fail(
392+
testFileName,
393+
{
394+
status: NODE_UNCAUGHT,
395+
name: 'evaluation in WPTRunner.runJsTests()',
396+
message: err.message,
397+
stack: inspect(err)
398+
},
399+
kUncaught
400+
);
401+
this.inProgress.delete(testFileName);
402+
});
392403
}
393404

394405
process.on('exit', () => {
@@ -430,56 +441,6 @@ class WPTRunner {
430441
});
431442
}
432443

433-
mock(testfile) {
434-
const resource = this.resource;
435-
const result = {
436-
// This is a mock, because at the moment fetch is not implemented
437-
// in Node.js, but some tests and harness depend on this to pull
438-
// resources.
439-
fetch(file) {
440-
return resource.read(testfile, file, true);
441-
},
442-
GLOBAL: {
443-
isWindow() { return false; }
444-
},
445-
Object
446-
};
447-
448-
return result;
449-
}
450-
451-
// Note: this is how our global space for the WPT test should look like
452-
getSandbox(filename) {
453-
const result = this.mock(filename);
454-
for (const [name, desc] of this.globals) {
455-
Object.defineProperty(result, name, desc);
456-
}
457-
return result;
458-
}
459-
460-
generateContext(test) {
461-
const filename = test.filename;
462-
const sandbox = this.sandbox = this.getSandbox(test.getRelativePath());
463-
const context = this.context = vm.createContext(sandbox);
464-
465-
const harnessPath = fixtures.path('wpt', 'resources', 'testharness.js');
466-
const harness = fs.readFileSync(harnessPath, 'utf8');
467-
vm.runInContext(harness, context, {
468-
filename: harnessPath
469-
});
470-
471-
sandbox.add_result_callback(
472-
this.resultCallback.bind(this, filename)
473-
);
474-
sandbox.add_completion_callback(
475-
this.completionCallback.bind(this, filename)
476-
);
477-
sandbox.self = sandbox;
478-
// TODO(joyeecheung): we are not a window - work with the upstream to
479-
// add a new scope for us.
480-
return context;
481-
}
482-
483444
getTestTitle(filename) {
484445
const spec = this.specMap.get(filename);
485446
const title = spec.meta && spec.meta.title;
@@ -524,9 +485,9 @@ class WPTRunner {
524485
* Report the status of each WPT test (one per file)
525486
*
526487
* @param {string} filename
527-
* @param {Test[]} test The Test objects returned by WPT harness
488+
* @param {object} harnessStatus - The status object returned by WPT harness.
528489
*/
529-
completionCallback(filename, tests, harnessStatus) {
490+
completionCallback(filename, harnessStatus) {
530491
// Treat it like a test case failure
531492
if (harnessStatus.status === 2) {
532493
const title = this.getTestTitle(filename);
@@ -644,5 +605,6 @@ class WPTRunner {
644605

645606
module.exports = {
646607
harness: harnessMock,
608+
ResourceLoader,
647609
WPTRunner
648610
};
Collapse file

‎test/common/wpt/worker.js‎

Copy file name to clipboard
+55Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,55 @@
1+
/* eslint-disable node-core/required-modules,node-core/require-common-first */
2+
3+
'use strict';
4+
5+
const { runInThisContext } = require('vm');
6+
const { parentPort, workerData } = require('worker_threads');
7+
8+
const { ResourceLoader } = require(workerData.wptRunner);
9+
const resource = new ResourceLoader(workerData.wptPath);
10+
11+
global.self = global;
12+
global.GLOBAL = {
13+
isWindow() { return false; }
14+
};
15+
global.require = require;
16+
17+
// This is a mock, because at the moment fetch is not implemented
18+
// in Node.js, but some tests and harness depend on this to pull
19+
// resources.
20+
global.fetch = function fetch(file) {
21+
return resource.read(workerData.filename, file, true);
22+
};
23+
24+
if (workerData.initScript) {
25+
runInThisContext(workerData.initScript);
26+
}
27+
28+
runInThisContext(workerData.harness.code, {
29+
filename: workerData.harness.filename
30+
});
31+
32+
// eslint-disable-next-line no-undef
33+
add_result_callback((result) => {
34+
parentPort.postMessage({
35+
type: 'result',
36+
result: {
37+
status: result.status,
38+
name: result.name,
39+
message: result.message,
40+
stack: result.stack,
41+
},
42+
});
43+
});
44+
45+
// eslint-disable-next-line no-undef
46+
add_completion_callback((_, status) => {
47+
parentPort.postMessage({
48+
type: 'completion',
49+
status,
50+
});
51+
});
52+
53+
for (const scriptToRun of workerData.scriptsToRun) {
54+
runInThisContext(scriptToRun.code, { filename: scriptToRun.filename });
55+
}
Collapse file

‎test/wpt/README.md‎

Copy file name to clipboardExpand all lines: test/wpt/README.md
+12-15Lines changed: 12 additions & 15 deletions
  • Display the source diff
  • Display the rich diff
Original file line numberDiff line numberDiff line change
@@ -45,23 +45,20 @@ For example, for the URL tests, add a file `test/wpt/test-url.js`:
4545
```js
4646
'use strict';
4747

48-
// This flag is required by the WPT Runner to patch the internals
49-
// for the tests to run in a vm.
50-
// Flags: --expose-internals
51-
5248
require('../common');
5349
const { WPTRunner } = require('../common/wpt');
5450

5551
const runner = new WPTRunner('url');
5652

57-
// Copy global descriptors from the global object
58-
runner.copyGlobalsFromObject(global, ['URL', 'URLSearchParams']);
59-
// Define any additional globals with descriptors
60-
runner.defineGlobal('DOMException', {
61-
get() {
62-
return require('internal/domexception');
63-
}
64-
});
53+
// Set Node.js flags required for the tests.
54+
runner.setFlags(['--expose-internals']);
55+
56+
// Set a script that will be executed in the worker before running the tests.
57+
runner.setInitScript(`
58+
const { internalBinding } = require('internal/test/binding');
59+
const { DOMException } = internalBinding('messaging');
60+
global.DOMException = DOMException;
61+
`);
6562

6663
runner.runJsTests();
6764
```
@@ -82,7 +79,7 @@ To run a specific test in WPT, for example, `url/url-searchparams.any.js`,
8279
pass the file name as argument to the corresponding test driver:
8380

8481
```text
85-
node --expose-internals test/wpt/test-url.js url-searchparams.any.js
82+
node test/wpt/test-url.js url-searchparams.any.js
8683
```
8784

8885
If there are any failures, update the corresponding status file
@@ -138,9 +135,9 @@ loads:
138135
* Status file (for example, `test/wpt/status/url.json` for `url`)
139136
* The WPT harness
140137

141-
Then, for each test, it creates a vm with the globals and mocks,
138+
Then, for each test, it creates a worker thread with the globals and mocks,
142139
sets up the harness result hooks, loads the metadata in the test (including
143-
loading extra resources), and runs all the tests in that vm,
140+
loading extra resources), and runs all the tests in that worker thread,
144141
skipping tests that cannot be run because of lack of dependency or
145142
expected failures.
146143

Collapse file

‎test/wpt/test-console.js‎

Copy file name to clipboardExpand all lines: test/wpt/test-console.js
-3Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,4 @@ const { WPTRunner } = require('../common/wpt');
44

55
const runner = new WPTRunner('console');
66

7-
// Copy global descriptors from the global object
8-
runner.copyGlobalsFromObject(global, ['console']);
9-
107
runner.runJsTests();
Collapse file

‎test/wpt/test-encoding.js‎

Copy file name to clipboard
+4-9Lines changed: 4 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,16 +1,11 @@
11
'use strict';
22
require('../common');
3-
const { MessageChannel } = require('worker_threads');
43
const { WPTRunner } = require('../common/wpt');
54
const runner = new WPTRunner('encoding');
65

7-
// Copy global descriptors from the global object
8-
runner.copyGlobalsFromObject(global, ['TextDecoder', 'TextEncoder']);
9-
10-
runner.defineGlobal('MessageChannel', {
11-
get() {
12-
return MessageChannel;
13-
}
14-
});
6+
runner.setInitScript(`
7+
const { MessageChannel } = require('worker_threads');
8+
global.MessageChannel = MessageChannel;
9+
`);
1510

1611
runner.runJsTests();
Collapse file

‎test/wpt/test-hr-time.js‎

Copy file name to clipboard
+5-18Lines changed: 5 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -1,27 +1,14 @@
11
'use strict';
22

3-
// Flags: --expose-internals
4-
53
require('../common');
64
const { WPTRunner } = require('../common/wpt');
7-
const { performance, PerformanceObserver } = require('perf_hooks');
85

96
const runner = new WPTRunner('hr-time');
107

11-
runner.copyGlobalsFromObject(global, [
12-
'setInterval',
13-
'clearInterval',
14-
'setTimeout',
15-
'clearTimeout'
16-
]);
17-
18-
runner.defineGlobal('performance', {
19-
get() {
20-
return performance;
21-
}
22-
});
23-
runner.defineGlobal('PerformanceObserver', {
24-
value: PerformanceObserver
25-
});
8+
runner.setInitScript(`
9+
const { performance, PerformanceObserver } = require('perf_hooks');
10+
global.performance = performance;
11+
global.PerformanceObserver = PerformanceObserver;
12+
`);
2613

2714
runner.runJsTests();

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.