Skip to content

Commit

Permalink
test: move execution of WPT to worker threads
Browse files Browse the repository at this point in the history
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>
  • Loading branch information
targos committed Aug 18, 2020
1 parent ca5ff72 commit 238104c
Show file tree
Hide file tree
Showing 10 changed files with 141 additions and 161 deletions.
2 changes: 1 addition & 1 deletion test/common/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -963,7 +963,7 @@ the original WPT harness, see [the WPT tests README][].

### Class: WPTRunner

A driver class for running WPT with the WPT harness in a vm.
A driver class for running WPT with the WPT harness in a worker thread.

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

Expand Down
146 changes: 54 additions & 92 deletions test/common/wpt.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,8 @@ const fixtures = require('../common/fixtures');
const fs = require('fs');
const fsPromises = fs.promises;
const path = require('path');
const vm = require('vm');
const { inspect } = require('util');
const { Worker } = require('worker_threads');

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

const intlRequirements = new IntlRequirement();


class StatusLoader {
/**
* @param {string} path relative path of the WPT subset
Expand Down Expand Up @@ -287,10 +286,9 @@ class WPTRunner {
constructor(path) {
this.path = path;
this.resource = new ResourceLoader(path);
this.sandbox = null;
this.context = null;

this.globals = new Map();
this.flags = [];
this.initScript = null;

this.status = new StatusLoader(path);
this.status.load();
Expand All @@ -304,28 +302,19 @@ class WPTRunner {
}

/**
* Specify that certain global descriptors from the object
* should be defined in the vm
* @param {object} obj
* @param {string[]} names
* Sets the Node.js flags passed to the worker.
* @param {Array<string>} flags
*/
copyGlobalsFromObject(obj, names) {
for (const name of names) {
const desc = Object.getOwnPropertyDescriptor(obj, name);
if (!desc) {
assert.fail(`${name} does not exist on the object`);
}
this.globals.set(name, desc);
}
setFlags(flags) {
this.flags = flags;
}

/**
* Specify that certain global descriptors should be defined in the vm
* @param {string} name
* @param {object} descriptor
* Sets a script to be run in the worker before executing the tests.
* @param {string} script
*/
defineGlobal(name, descriptor) {
this.globals.set(name, descriptor);
setInitScript(script) {
this.initScript = script;
}

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

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

for (const { code, filename } of scriptsToRun) {
try {
vm.runInContext(code, context, { filename });
} catch (err) {
this.fail(
testFileName,
{
status: NODE_UNCAUGHT,
name: 'evaluation in WPTRunner.runJsTests()',
message: err.message,
stack: inspect(err)
},
kUncaught
);
this.inProgress.delete(filename);
break;
const workerPath = path.join(__dirname, 'wpt/worker.js');
const worker = new Worker(workerPath, {
execArgv: this.flags,
workerData: {
filename: testFileName,
wptRunner: __filename,
wptPath: this.path,
initScript: this.initScript,
harness: {
code: fs.readFileSync(harnessPath, 'utf8'),
filename: harnessPath,
},
scriptsToRun,
},
});

worker.on('message', (message) => {
switch (message.type) {
case 'result':
return this.resultCallback(testFileName, message.result);
case 'completion':
return this.completionCallback(testFileName, message.status);
default:
throw new Error(`Unexpected message from worker: ${message.type}`);
}
}
});

worker.on('error', (err) => {
this.fail(
testFileName,
{
status: NODE_UNCAUGHT,
name: 'evaluation in WPTRunner.runJsTests()',
message: err.message,
stack: inspect(err)
},
kUncaught
);
this.inProgress.delete(testFileName);
});
}

process.on('exit', () => {
Expand Down Expand Up @@ -430,56 +441,6 @@ class WPTRunner {
});
}

mock(testfile) {
const resource = this.resource;
const result = {
// This is a mock, because at the moment fetch is not implemented
// in Node.js, but some tests and harness depend on this to pull
// resources.
fetch(file) {
return resource.read(testfile, file, true);
},
GLOBAL: {
isWindow() { return false; }
},
Object
};

return result;
}

// Note: this is how our global space for the WPT test should look like
getSandbox(filename) {
const result = this.mock(filename);
for (const [name, desc] of this.globals) {
Object.defineProperty(result, name, desc);
}
return result;
}

generateContext(test) {
const filename = test.filename;
const sandbox = this.sandbox = this.getSandbox(test.getRelativePath());
const context = this.context = vm.createContext(sandbox);

const harnessPath = fixtures.path('wpt', 'resources', 'testharness.js');
const harness = fs.readFileSync(harnessPath, 'utf8');
vm.runInContext(harness, context, {
filename: harnessPath
});

sandbox.add_result_callback(
this.resultCallback.bind(this, filename)
);
sandbox.add_completion_callback(
this.completionCallback.bind(this, filename)
);
sandbox.self = sandbox;
// TODO(joyeecheung): we are not a window - work with the upstream to
// add a new scope for us.
return context;
}

getTestTitle(filename) {
const spec = this.specMap.get(filename);
const title = spec.meta && spec.meta.title;
Expand Down Expand Up @@ -524,9 +485,9 @@ class WPTRunner {
* Report the status of each WPT test (one per file)
*
* @param {string} filename
* @param {Test[]} test The Test objects returned by WPT harness
* @param {object} harnessStatus - The status object returned by WPT harness.
*/
completionCallback(filename, tests, harnessStatus) {
completionCallback(filename, harnessStatus) {
// Treat it like a test case failure
if (harnessStatus.status === 2) {
const title = this.getTestTitle(filename);
Expand Down Expand Up @@ -644,5 +605,6 @@ class WPTRunner {

module.exports = {
harness: harnessMock,
ResourceLoader,
WPTRunner
};
55 changes: 55 additions & 0 deletions test/common/wpt/worker.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
/* eslint-disable node-core/required-modules,node-core/require-common-first */

'use strict';

const { runInThisContext } = require('vm');
const { parentPort, workerData } = require('worker_threads');

const { ResourceLoader } = require(workerData.wptRunner);
const resource = new ResourceLoader(workerData.wptPath);

global.self = global;
global.GLOBAL = {
isWindow() { return false; }
};
global.require = require;

// This is a mock, because at the moment fetch is not implemented
// in Node.js, but some tests and harness depend on this to pull
// resources.
global.fetch = function fetch(file) {
return resource.read(workerData.filename, file, true);
};

if (workerData.initScript) {
runInThisContext(workerData.initScript);
}

runInThisContext(workerData.harness.code, {
filename: workerData.harness.filename
});

// eslint-disable-next-line no-undef
add_result_callback((result) => {
parentPort.postMessage({
type: 'result',
result: {
status: result.status,
name: result.name,
message: result.message,
stack: result.stack,
},
});
});

// eslint-disable-next-line no-undef
add_completion_callback((_, status) => {
parentPort.postMessage({
type: 'completion',
status,
});
});

for (const scriptToRun of workerData.scriptsToRun) {
runInThisContext(scriptToRun.code, { filename: scriptToRun.filename });
}
27 changes: 12 additions & 15 deletions test/wpt/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -45,23 +45,20 @@ For example, for the URL tests, add a file `test/wpt/test-url.js`:
```js
'use strict';

// This flag is required by the WPT Runner to patch the internals
// for the tests to run in a vm.
// Flags: --expose-internals

require('../common');
const { WPTRunner } = require('../common/wpt');

const runner = new WPTRunner('url');

// Copy global descriptors from the global object
runner.copyGlobalsFromObject(global, ['URL', 'URLSearchParams']);
// Define any additional globals with descriptors
runner.defineGlobal('DOMException', {
get() {
return require('internal/domexception');
}
});
// Set Node.js flags required for the tests.
runner.setFlags(['--expose-internals']);

// Set a script that will be executed in the worker before running the tests.
runner.setInitScript(`
const { internalBinding } = require('internal/test/binding');
const { DOMException } = internalBinding('messaging');
global.DOMException = DOMException;
`);

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

```text
node --expose-internals test/wpt/test-url.js url-searchparams.any.js
node test/wpt/test-url.js url-searchparams.any.js
```

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

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

Expand Down
3 changes: 0 additions & 3 deletions test/wpt/test-console.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,4 @@ const { WPTRunner } = require('../common/wpt');

const runner = new WPTRunner('console');

// Copy global descriptors from the global object
runner.copyGlobalsFromObject(global, ['console']);

runner.runJsTests();
13 changes: 4 additions & 9 deletions test/wpt/test-encoding.js
Original file line number Diff line number Diff line change
@@ -1,16 +1,11 @@
'use strict';
require('../common');
const { MessageChannel } = require('worker_threads');
const { WPTRunner } = require('../common/wpt');
const runner = new WPTRunner('encoding');

// Copy global descriptors from the global object
runner.copyGlobalsFromObject(global, ['TextDecoder', 'TextEncoder']);

runner.defineGlobal('MessageChannel', {
get() {
return MessageChannel;
}
});
runner.setInitScript(`
const { MessageChannel } = require('worker_threads');
global.MessageChannel = MessageChannel;
`);

runner.runJsTests();
23 changes: 5 additions & 18 deletions test/wpt/test-hr-time.js
Original file line number Diff line number Diff line change
@@ -1,27 +1,14 @@
'use strict';

// Flags: --expose-internals

require('../common');
const { WPTRunner } = require('../common/wpt');
const { performance, PerformanceObserver } = require('perf_hooks');

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

runner.copyGlobalsFromObject(global, [
'setInterval',
'clearInterval',
'setTimeout',
'clearTimeout'
]);

runner.defineGlobal('performance', {
get() {
return performance;
}
});
runner.defineGlobal('PerformanceObserver', {
value: PerformanceObserver
});
runner.setInitScript(`
const { performance, PerformanceObserver } = require('perf_hooks');
global.performance = performance;
global.PerformanceObserver = PerformanceObserver;
`);

runner.runJsTests();
Loading

0 comments on commit 238104c

Please sign in to comment.