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
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions Makefile
Expand Up @@ -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'.
Expand Down
83 changes: 53 additions & 30 deletions test/common/wpt.js
Expand Up @@ -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);
Expand Down Expand Up @@ -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();
}
Expand Down Expand Up @@ -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));
}
}

Expand Down Expand Up @@ -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;
Expand All @@ -425,7 +443,7 @@ class WPTRunner {
this.scriptsModifier = null;

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

Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -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: {
Expand Down Expand Up @@ -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));
}
}

Expand Down
2 changes: 1 addition & 1 deletion test/wpt/testcfg.py
Expand Up @@ -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
@@ -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));