From 505b768a6ebf2d7ee8dcfa50957b4b511dc1075c Mon Sep 17 00:00:00 2001 From: Filip Skokan Date: Wed, 19 Apr 2023 23:42:26 +0200 Subject: [PATCH] test: refactor WPTRunner and enable parallel WPT execution --- Makefile | 3 +- test/common/wpt.js | 337 ++++++++++++++++++++---------------- test/common/wpt/worker.js | 8 +- test/wpt/testcfg.py | 2 +- tools/merge-wpt-reports.mjs | 32 ++++ 5 files changed, 223 insertions(+), 159 deletions(-) create mode 100644 tools/merge-wpt-reports.mjs diff --git a/Makefile b/Makefile index 85950cb95ca7c5..25e54f109ef58b 100644 --- a/Makefile +++ b/Makefile @@ -592,7 +592,8 @@ test-wpt: all test-wpt-report: $(RM) -r out/wpt mkdir -p out/wpt - WPT_REPORT=1 $(PYTHON) tools/test.py --shell $(NODE) $(PARALLEL_ARGS) wpt + -WPT_REPORT=1 $(PYTHON) tools/test.py --shell $(NODE) $(PARALLEL_ARGS) wpt + $(NODE) "$$PWD/tools/merge-wpt-reports.mjs" .PHONY: test-internet test-internet: all diff --git a/test/common/wpt.js b/test/common/wpt.js index 57c409f5cc8a0f..2212af43ba1d85 100644 --- a/test/common/wpt.js +++ b/test/common/wpt.js @@ -10,6 +10,8 @@ const os = require('os'); const { inspect } = require('util'); const { Worker } = require('worker_threads'); +const workerPath = path.join(__dirname, 'wpt/worker.js'); + function getBrowserProperties() { const { node: version } = process.versions; // e.g. 18.13.0, 20.0.0-nightly202302078e6e215481 const release = /^\d+\.\d+\.\d+$/.test(version); @@ -57,7 +59,8 @@ function codeUnitStr(char) { } class WPTReport { - constructor() { + constructor(path) { + this.filename = `report-${path.replaceAll('/', '-')}.json`; this.results = []; this.time_start = Date.now(); } @@ -96,26 +99,18 @@ class WPTReport { return result; }); - if (fs.existsSync('out/wpt/wptreport.json')) { - const prev = JSON.parse(fs.readFileSync('out/wpt/wptreport.json')); - this.results = [...prev.results, ...this.results]; - this.time_start = prev.time_start; - this.time_end = Math.max(this.time_end, prev.time_end); - this.run_info = prev.run_info; - } else { - /** - * Return required and some optional properties - * https://github.com/web-platform-tests/wpt.fyi/blob/60da175/api/README.md?plain=1#L331-L335 - */ - this.run_info = { - product: 'node.js', - ...getBrowserProperties(), - revision: process.env.WPT_REVISION || 'unknown', - os: getOs(), - }; - } + /** + * Return required and some optional properties + * https://github.com/web-platform-tests/wpt.fyi/blob/60da175/api/README.md?plain=1#L331-L335 + */ + this.run_info = { + product: 'node.js', + ...getBrowserProperties(), + revision: process.env.WPT_REVISION || 'unknown', + os: getOs(), + }; - fs.writeFileSync('out/wpt/wptreport.json', JSON.stringify(this)); + fs.writeFileSync(`out/wpt/${this.filename}`, JSON.stringify(this)); } } @@ -169,23 +164,27 @@ class ResourceLoader { * @param {string} from the path of the file loading this resource, * relative to the WPT folder. * @param {string} url the url of the resource being loaded. - * @param {boolean} asFetch if true, return the resource in a - * pseudo-Response object. */ - read(from, url, asFetch = true) { + read(from, url) { const file = this.toRealFilePath(from, url); - if (asFetch) { - return fsPromises.readFile(file) - .then((data) => { - return { - ok: true, - json() { return JSON.parse(data.toString()); }, - text() { return data.toString(); }, - }; - }); - } return fs.readFileSync(file, 'utf8'); } + + /** + * Load a resource in test/fixtures/wpt specified with a URL + * @param {string} from the path of the file loading this resource, + * relative to the WPT folder. + * @param {string} url the url of the resource being loaded. + */ + async readAsFetch(from, url) { + const file = this.toRealFilePath(from, url); + const data = await fsPromises.readFile(file); + return { + ok: true, + json() { return JSON.parse(data.toString()); }, + text() { return data.toString(); }, + }; + } } class StatusRule { @@ -251,16 +250,21 @@ class StatusRuleSet { // A specification of WPT test class WPTTestSpec { + #content; + /** * @param {string} mod name of the WPT module, e.g. * 'html/webappapis/microtask-queuing' * @param {string} filename path of the test, relative to mod, e.g. * 'test.any.js' * @param {StatusRule[]} rules + * @param {string} variant test file variant */ - constructor(mod, filename, rules) { + constructor(mod, filename, rules, variant = '') { + this.id = Symbol(); this.module = mod; this.filename = filename; + this.variant = variant; this.requires = new Set(); this.failedTests = []; @@ -289,6 +293,17 @@ class WPTTestSpec { this.skipReasons = [...new Set(this.skipReasons)]; } + /** + * @param {string} mod + * @param {string} filename + * @param {StatusRule[]} rules + */ + static from(mod, filename, rules) { + const spec = new WPTTestSpec(mod, filename, rules); + const meta = spec.getMeta(); + return meta.variant?.map((variant) => new WPTTestSpec(mod, filename, rules, variant)) || [spec]; + } + getRelativePath() { return path.join(this.module, this.filename); } @@ -297,8 +312,38 @@ class WPTTestSpec { return fixtures.path('wpt', this.getRelativePath()); } + /** + * @returns {string} + */ getContent() { - return fs.readFileSync(this.getAbsolutePath(), 'utf8'); + this.#content ||= fs.readFileSync(this.getAbsolutePath(), 'utf8'); + return this.#content; + } + + /** + * @returns {{ script?: string[]; variant?: string[]; [key: string]: string }} parsed META tags of a spec file + */ + getMeta() { + const matches = this.getContent().match(/\/\/ META: .+/g); + if (!matches) { + return {}; + } + const result = {}; + for (const match of matches) { + const parts = match.match(/\/\/ META: ([^=]+?)=(.+)/); + const key = parts[1]; + const value = parts[2]; + if (key === 'script' || key === 'variant') { + if (result[key]) { + result[key].push(value); + } else { + result[key] = [value]; + } + } else { + result[key] = value; + } + } + return result; } } @@ -348,7 +393,6 @@ class StatusLoader { */ constructor(path) { this.path = path; - this.loaded = false; this.rules = new StatusRuleSet(); /** @type {WPTTestSpec[]} */ this.specs = []; @@ -388,9 +432,8 @@ class StatusLoader { for (const file of list) { const relativePath = path.relative(subDir, file); const match = this.rules.match(relativePath); - this.specs.push(new WPTTestSpec(this.path, relativePath, match)); + this.specs.push(...WPTTestSpec.from(this.path, relativePath, match)); } - this.loaded = true; } } @@ -402,6 +445,29 @@ const kIncomplete = 'incomplete'; const kUncaught = 'uncaught'; const NODE_UNCAUGHT = 100; +const limit = (concurrency) => { + let running = 0; + const queue = []; + + const execute = async (fn) => { + if (running < concurrency) { + running++; + try { + await fn(); + } finally { + running--; + if (queue.length > 0) { + execute(queue.shift()); + } + } + } else { + queue.push(fn); + } + }; + + return execute; +}; + class WPTRunner { constructor(path) { this.path = path; @@ -414,7 +480,7 @@ class WPTRunner { this.status = new StatusLoader(path); this.status.load(); this.specMap = new Map( - this.status.specs.map((item) => [item.filename, item]), + this.status.specs.map((item) => [item.id, item]), ); this.results = {}; @@ -422,16 +488,14 @@ class WPTRunner { this.workers = new Map(); this.unexpectedFailures = []; - this.scriptsModifier = null; - if (process.env.WPT_REPORT != null) { - this.report = new WPTReport(); + this.report = new WPTReport(path); } } /** * Sets the Node.js flags passed to the worker. - * @param {Array} flags + * @param {string[]} flags */ setFlags(flags) { this.flags = flags; @@ -453,13 +517,18 @@ class WPTRunner { this.scriptsModifier = modifier; } - fullInitScript(url, metaTitle) { + /** + * @param {WPTTestSpec} spec + */ + fullInitScript(spec) { + const url = new URL(`/${spec.getRelativePath().replace(/\.js$/, '.html')}${spec.variant}`, 'http://wpt'); + const title = spec.getMeta().title; let { initScript } = this; initScript = `${initScript}\n\n//===\nglobalThis.location = new URL("${url.href}");`; - if (metaTitle) { - initScript = `${initScript}\n\n//===\nglobalThis.META_TITLE = "${metaTitle}";`; + if (title) { + initScript = `${initScript}\n\n//===\nglobalThis.META_TITLE = "${title}";`; } if (this.globalThisInitScripts.length === null) { @@ -527,47 +596,28 @@ class WPTRunner { // TODO(joyeecheung): work with the upstream to port more tests in .html // to .js. async runJsTests() { - let queue = []; + const queue = this.buildQueue(); - // If the tests are run as `node test/wpt/test-something.js subset.any.js`, - // only `subset.any.js` will be run by the runner. - if (process.argv[2]) { - const filename = process.argv[2]; - if (!this.specMap.has(filename)) { - throw new Error(`${filename} not found!`); - } - queue.push(this.specMap.get(filename)); - } else { - queue = this.buildQueue(); - } - - this.inProgress = new Set(queue.map((spec) => spec.filename)); + const run = limit(os.availableParallelism()); for (const spec of queue) { - const testFileName = spec.filename; const content = spec.getContent(); - const meta = spec.meta = this.getMeta(content); + const meta = spec.getMeta(content); const absolutePath = spec.getAbsolutePath(); const relativePath = spec.getRelativePath(); const harnessPath = fixtures.path('wpt', 'resources', 'testharness.js'); const scriptsToRun = []; - let needsGc = false; // Scripts specified with the `// META: script=` header - if (meta.script) { - for (const script of meta.script) { - if (script === '/common/gc.js') { - needsGc = true; - } - const obj = { - filename: this.resource.toRealFilePath(relativePath, script), - code: this.resource.read(relativePath, script, false), - }; - this.scriptsModifier?.(obj); - scriptsToRun.push(obj); - } - } + meta.script?.forEach((script) => { + const obj = { + filename: this.resource.toRealFilePath(relativePath, script), + code: this.resource.read(relativePath, script), + }; + this.scriptsModifier?.(obj); + scriptsToRun.push(obj); + }); // The actual test const obj = { code: content, @@ -576,53 +626,46 @@ class WPTRunner { this.scriptsModifier?.(obj); scriptsToRun.push(obj); - /** - * Example test with no META variant - * https://github.com/nodejs/node/blob/03854f6/test/fixtures/wpt/WebCryptoAPI/sign_verify/hmac.https.any.js#L1-L4 - * - * Example test with multiple META variants - * https://github.com/nodejs/node/blob/03854f6/test/fixtures/wpt/WebCryptoAPI/generateKey/successes_RSASSA-PKCS1-v1_5.https.any.js#L1-L9 - */ - for (const variant of meta.variant || ['']) { - const workerPath = path.join(__dirname, 'wpt/worker.js'); + run(async () => { const worker = new Worker(workerPath, { execArgv: this.flags, workerData: { testRelativePath: relativePath, wptRunner: __filename, wptPath: this.path, - initScript: this.fullInitScript(new URL(`/${relativePath.replace(/\.js$/, '.html')}${variant}`, 'http://wpt'), meta.title), + initScript: this.fullInitScript(spec), harness: { code: fs.readFileSync(harnessPath, 'utf8'), filename: harnessPath, }, scriptsToRun, - needsGc, + needsGc: !!meta.script?.find((script) => script === '/common/gc.js'), }, }); - this.workers.set(testFileName, worker); + this.inProgress.add(spec.id); + this.workers.set(spec.id, worker); let reportResult; worker.on('message', (message) => { switch (message.type) { case 'result': - reportResult ||= this.report?.addResult(`/${relativePath}${variant}`, 'OK'); - return this.resultCallback(testFileName, message.result, reportResult); + reportResult ||= this.report?.addResult(`/${relativePath}${spec.variant}`, 'OK'); + return this.resultCallback(spec, message.result, reportResult); case 'completion': - return this.completionCallback(testFileName, message.status); + return this.completionCallback(spec, message.status); default: throw new Error(`Unexpected message from worker: ${message.type}`); } }); worker.on('error', (err) => { - if (!this.inProgress.has(testFileName)) { + if (!this.inProgress.has(spec.id)) { // The test is already finished. Ignore errors that occur after it. // This can happen normally, for example in timers tests. return; } this.fail( - testFileName, + spec, { status: NODE_UNCAUGHT, name: 'evaluation in WPTRunner.runJsTests()', @@ -631,21 +674,23 @@ class WPTRunner { }, kUncaught, ); - this.inProgress.delete(testFileName); + this.inProgress.delete(spec.id); }); await events.once(worker, 'exit').catch(() => {}); - } + }); } process.on('exit', () => { if (this.inProgress.size > 0) { - for (const filename of this.inProgress) { - this.fail(filename, { name: 'Unknown' }, kIncomplete); + for (const id of this.inProgress) { + const spec = this.specMap.get(id); + this.fail(spec, { name: 'Unknown' }, kIncomplete); } } inspect.defaultOptions.depth = Infinity; // Sorts the rules to have consistent output + console.log(''); console.log(JSON.stringify(Object.keys(this.results).sort().reduce( (obj, key) => { obj[key] = this.results[key]; @@ -700,6 +745,7 @@ class WPTRunner { const ran = queue.length; const total = ran + skipped; const passed = ran - expectedFailures - failures.length; + console.log(''); console.log(`Ran ${ran}/${total} tests, ${skipped} skipped,`, `${passed} passed, ${expectedFailures} expected failures,`, `${failures.length} unexpected failures,`, @@ -719,11 +765,6 @@ class WPTRunner { }); } - getTestTitle(filename) { - const spec = this.specMap.get(filename); - return spec.meta?.title || filename.split('.')[0]; - } - // Map WPT test status to strings getTestStatus(status) { switch (status) { @@ -744,42 +785,39 @@ class WPTRunner { * Report the status of each specific test case (there could be multiple * in one test file). * - * @param {string} filename + * @param {WPTTestSpec} spec * @param {Test} test The Test object returned by WPT harness */ - resultCallback(filename, test, reportResult) { + resultCallback(spec, test, reportResult) { const status = this.getTestStatus(test.status); - console.log(`---- ${test.name} ----`); if (status !== kPass) { - this.fail(filename, test, status, reportResult); + this.fail(spec, test, status, reportResult); } else { - this.succeed(filename, test, status, reportResult); + this.succeed(test, status, reportResult); } } /** * Report the status of each WPT test (one per file) * - * @param {string} filename + * @param {WPTTestSpec} spec * @param {object} harnessStatus - The status object returned by WPT harness. */ - completionCallback(filename, harnessStatus) { + completionCallback(spec, harnessStatus) { // Treat it like a test case failure if (harnessStatus.status === 2) { - const title = this.getTestTitle(filename); - console.log(`---- ${title} ----`); - this.resultCallback(filename, { status: 2, name: 'Unknown' }); + this.resultCallback(spec, { status: 2, name: 'Unknown' }); } - this.inProgress.delete(filename); + this.inProgress.delete(spec.id); // Always force termination of the worker. Some tests allocate resources // that would otherwise keep it alive. - this.workers.get(filename).terminate(); + this.workers.get(spec.id).terminate(); } - addTestResult(filename, item) { - let result = this.results[filename]; + addTestResult(spec, item) { + let result = this.results[spec.filename]; if (!result) { - result = this.results[filename] = {}; + result = this.results[spec.filename] = {}; } if (item.status === kSkip) { // { filename: { skip: 'reason' } } @@ -801,17 +839,15 @@ class WPTRunner { } } - succeed(filename, test, status, reportResult) { + succeed(test, status, reportResult) { console.log(`[${status.toUpperCase()}] ${test.name}`); reportResult?.addSubtest(test.name, 'PASS'); } - fail(filename, test, status, reportResult) { - const spec = this.specMap.get(filename); + fail(spec, test, status, reportResult) { const expected = spec.failedTests.includes(test.name); if (expected) { console.log(`[EXPECTED_FAILURE][${status.toUpperCase()}] ${test.name}`); - console.log(test.message || status); } else { console.log(`[UNEXPECTED_FAILURE][${status.toUpperCase()}] ${test.name}`); } @@ -820,12 +856,12 @@ class WPTRunner { console.log(test.stack); } const command = `${process.execPath} ${process.execArgv}` + - ` ${require.main.filename} ${filename}`; + ` ${require.main.filename} '${spec.filename}${spec.variant}'`; console.log(`Command: ${command}\n`); reportResult?.addSubtest(test.name, 'FAIL', test.message); - this.addTestResult(filename, { + this.addTestResult(spec, { name: test.name, expected, status: kFail, @@ -833,57 +869,52 @@ class WPTRunner { }); } - skip(filename, reasons) { - const title = this.getTestTitle(filename); - console.log(`---- ${title} ----`); + skip(spec, reasons) { const joinedReasons = reasons.join('; '); - console.log(`[SKIPPED] ${joinedReasons}`); - this.addTestResult(filename, { + console.log(`[SKIPPED] ${spec.filename}${spec.variant}: ${joinedReasons}`); + this.addTestResult(spec, { status: kSkip, reason: joinedReasons, }); } - getMeta(code) { - const matches = code.match(/\/\/ META: .+/g); - if (!matches) { - return {}; + buildQueue() { + const queue = []; + let argFilename; + let argVariant; + if (process.argv[2]) { + ([argFilename, argVariant = ''] = process.argv[2].split('?')); } - const result = {}; - for (const match of matches) { - const parts = match.match(/\/\/ META: ([^=]+?)=(.+)/); - const key = parts[1]; - const value = parts[2]; - if (key === 'script' || key === 'variant') { - if (result[key]) { - result[key].push(value); - } else { - result[key] = [value]; + for (const spec of this.specMap.values()) { + if (argFilename) { + if (spec.filename === argFilename && (!argVariant || spec.variant.substring(1) === argVariant)) { + queue.push(spec); } - } else { - result[key] = value; + continue; } - } - return result; - } - buildQueue() { - const queue = []; - for (const spec of this.specMap.values()) { - const filename = spec.filename; if (spec.skipReasons.length > 0) { - this.skip(filename, spec.skipReasons); + this.skip(spec, spec.skipReasons); continue; } const lackingIntl = intlRequirements.isLacking(spec.requires); if (lackingIntl) { - this.skip(filename, [ `requires ${lackingIntl}` ]); + this.skip(spec, [ `requires ${lackingIntl}` ]); continue; } queue.push(spec); } + + // If the tests are run as `node test/wpt/test-something.js subset.any.js`, + // only `subset.any.js` (all variants) will be run by the runner. + // If the tests are run as `node test/wpt/test-something.js 'subset.any.js?1-10'`, + // only the `?1-10` variant of `subset.any.js` will be run by the runner. + if (argFilename && queue.length === 0) { + throw new Error(`${process.argv[2]} not found!`); + } + return queue; } } diff --git a/test/common/wpt/worker.js b/test/common/wpt/worker.js index 14d3d887aad5eb..80b32bf5b912e7 100644 --- a/test/common/wpt/worker.js +++ b/test/common/wpt/worker.js @@ -20,11 +20,11 @@ globalThis.GLOBAL = { }; globalThis.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. +// This is a mock for non-fetch tests that use fetch to resolve +// a relative fixture file. +// Actual Fetch API WPTs are executed in nodejs/undici. globalThis.fetch = function fetch(file) { - return resource.read(workerData.testRelativePath, file, true); + return resource.readAsFetch(workerData.testRelativePath, file); }; if (workerData.initScript) { diff --git a/test/wpt/testcfg.py b/test/wpt/testcfg.py index 3c356cf474d83c..db235699ddfe57 100644 --- a/test/wpt/testcfg.py +++ b/test/wpt/testcfg.py @@ -3,4 +3,4 @@ import testpy def GetConfiguration(context, root): - return testpy.SimpleTestConfiguration(context, root, 'wpt') + return testpy.ParallelTestConfiguration(context, root, 'wpt') diff --git a/tools/merge-wpt-reports.mjs b/tools/merge-wpt-reports.mjs new file mode 100644 index 00000000000000..9199714f4b1032 --- /dev/null +++ b/tools/merge-wpt-reports.mjs @@ -0,0 +1,32 @@ +import * as fs from 'node:fs'; +import * as path from 'node:path'; +import * as url from 'node:url'; + +const __dirname = path.dirname(url.fileURLToPath(import.meta.url)); + +const outDir = path.resolve(__dirname, '../out/wpt'); +const files = fs.readdirSync(outDir); +const reports = files.filter((file) => file.endsWith('.json')); + +const startTimes = []; +const endTimes = []; +const results = []; +let runInfo; + +for (const file of reports) { + const report = JSON.parse(fs.readFileSync(path.resolve(outDir, file))); + fs.unlinkSync(path.resolve(outDir, file)); + results.push(...report.results); + startTimes.push(report.time_start); + endTimes.push(report.time_end); + runInfo ||= report.run_info; +} + +const mergedReport = { + time_start: Math.min(...startTimes), + time_end: Math.max(...endTimes), + run_info: runInfo, + results, +}; + +fs.writeFileSync(path.resolve(outDir, 'wptreport.json'), JSON.stringify(mergedReport));