Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

test: run WPT files in parallel again #47283

Merged
merged 1 commit into from
Mar 31, 2023
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
test: run WPT files in parallel again
panva committed Mar 30, 2023
commit 313c51e36f3be46cf30e60316c35a97ee44e4b9f
1 change: 1 addition & 0 deletions Makefile
Original file line number Diff line number Diff line change
@@ -603,6 +603,7 @@ test-wpt-report:
$(RM) -r out/wpt
mkdir -p out/wpt
WPT_REPORT=1 $(PYTHON) tools/test.py --shell $(NODE) $(PARALLEL_ARGS) wpt
$(NODE) "$$PWD/tools/merge-wpt-reports.mjs"

.PHONY: test-simple
test-simple: | cctest # Depends on 'all'.
83 changes: 53 additions & 30 deletions test/common/wpt.js
Original file line number Diff line number Diff line change
@@ -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));
}
}

@@ -402,6 +397,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());
}
Comment on lines +411 to +413
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is not returned or awaited. Is there a risk of unhandled rejection?

Copy link
Member Author

@panva panva Mar 28, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It behaves as expected even with the WPT suite updated (like the daily WPT job does).

This is the await in the queue.

await events.once(worker, 'exit').catch(() => {});

That being said, I am aware it's just a basic concurrency limit function implementation...

}
} else {
queue.push(fn);
}
};

return execute;
};

class WPTRunner {
constructor(path) {
this.path = path;
@@ -425,7 +443,7 @@ class WPTRunner {
this.scriptsModifier = null;

if (process.env.WPT_REPORT != null) {
this.report = new WPTReport();
this.report = new WPTReport(path);
}
}

@@ -543,6 +561,8 @@ class WPTRunner {

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();
@@ -576,15 +596,7 @@ 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');
const runWorker = async (variant) => {
const worker = new Worker(workerPath, {
execArgv: this.flags,
workerData: {
@@ -635,6 +647,17 @@ class WPTRunner {
});

await events.once(worker, 'exit').catch(() => {});
};

/**
* 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 || ['']) {
run(() => runWorker(variant));
}
}

2 changes: 1 addition & 1 deletion test/wpt/testcfg.py
Original file line number Diff line number Diff line change
@@ -3,4 +3,4 @@
import testpy

def GetConfiguration(context, root):
return testpy.SimpleTestConfiguration(context, root, 'wpt')
return testpy.ParallelTestConfiguration(context, root, 'wpt')
32 changes: 32 additions & 0 deletions tools/merge-wpt-reports.mjs
Original file line number Diff line number Diff line change
@@ -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));