FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

test: move execution of WPT to worker threads · nodejs/node@7ed7ef7 · GitHub

/ node Public

Commit 7ed7ef7

Browse files
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

10 files changed

Lines changed: 141 additions & 161 deletions

File tree

‎test/common/README.md‎

Lines changed: 1 addition & 1 deletion
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

‎test/common/wpt.js‎

Lines 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
};

‎test/common/wpt/worker.js‎

Lines 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+
}

‎test/wpt/README.md‎

Lines changed: 12 additions & 15 deletions
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

‎test/wpt/test-console.js‎

Lines 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();

‎test/wpt/test-encoding.js‎

Lines 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();

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

Lines 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)
Back | FazBrowse Home | New Git URL