Skip to content

Commit 80c1bf7

Browse files
panvaaduh95
authored andcommitted
test: deflake user timing WPT assertions
The fixed 20 ms tolerance in mark.any.js can fail when execution pauses between creating a mark and reading the clock. Adapt the test in memory to check the mark timestamp against readings before and after mark(). Keep the vendored fixture and subtest names unchanged. Apply script modifiers to worker entry scripts as well so the check covers both WPT globals, and remove the suite-wide flaky expectation. Signed-off-by: Filip Skokan <panva.ip@gmail.com> Assisted-by: Codex PR-URL: #66036 Fixes: #40449 Refs: #41203 Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com> Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
1 parent 13eb484 commit 80c1bf7

5 files changed

Lines changed: 79 additions & 14 deletions

File tree

test/common/wpt.js

Lines changed: 6 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1039,15 +1039,10 @@ class WPTRunner {
10391039
this.scriptsModifier?.(obj);
10401040
return obj;
10411041
}) ?? [];
1042-
if (!isWebWorkerTest) {
1043-
// The actual test
1044-
const obj = {
1045-
code: content,
1046-
filename: absolutePath,
1047-
};
1048-
this.scriptsModifier?.(obj);
1049-
scriptsToRun.push(obj);
1050-
}
1042+
// The actual test, including tests imported by a Web Worker.
1043+
const testScript = { code: content, filename: absolutePath };
1044+
this.scriptsModifier?.(testScript);
1045+
if (!isWebWorkerTest) scriptsToRun.push(testScript);
10511046

10521047
jobs.push(run(async () => {
10531048
this.inProgress.add(spec);
@@ -1066,6 +1061,8 @@ class WPTRunner {
10661061
// Set when the test runs inside an actual Web Worker.
10671062
webWorker: isWebWorkerTest ? {
10681063
path: absolutePath,
1064+
modifiedScript: testScript.code !== content || testScript.filename !== absolutePath ?
1065+
testScript : undefined,
10691066
isAnyTest,
10701067
initScript: this.initScript,
10711068
title: meta.title,

test/common/wpt/webworker.js

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -123,11 +123,21 @@ globalThis.onmessage = ({ data }) => {
123123
for (const script of data.scripts) {
124124
globalThis.importScripts(pathToFileURL(script).href);
125125
}
126+
}
127+
128+
if (data.modifiedScript) {
129+
runInThisContext(data.modifiedScript.code, {
130+
filename: data.modifiedScript.filename,
131+
importModuleDynamically: USE_MAIN_CONTEXT_DEFAULT_LOADER,
132+
});
133+
} else {
134+
// Keep unmodified tests on the native script loading path.
126135
globalThis.importScripts(pathToFileURL(data.path).href);
136+
}
137+
138+
if (data.isAnyTest) {
139+
// *.worker.js tests import testharness.js and call done() themselves.
127140
// eslint-disable-next-line no-undef
128141
done();
129-
} else {
130-
// *.worker.js tests import testharness.js and call done() themselves.
131-
globalThis.importScripts(pathToFileURL(data.path).href);
132142
}
133143
};
Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
1+
'use strict';
2+
3+
const common = require('../common');
4+
const assert = require('assert');
5+
const fixtures = require('../common/fixtures');
6+
const { spawnSync } = require('child_process');
7+
8+
if (process.env.NODE_TEST_WPT_MODIFIER_PROBE === '1') {
9+
const { WPTRunner } = require('../common/wpt');
10+
const runner = new WPTRunner('html/webappapis/atob');
11+
runner.setScriptModifier(common.mustCall((script) => {
12+
assert.strictEqual(script.filename,
13+
fixtures.path('wpt', 'html/webappapis/atob', 'base64.any.js'));
14+
script.filename += '.modified';
15+
script.code = `test(() => {
16+
assert_true(new Error().stack.includes(${JSON.stringify(`${script.filename}:`)}));
17+
}, 'modified script retains its filename');`;
18+
}, 2));
19+
runner.runJsTests();
20+
} else {
21+
// eslint-disable-next-line no-unused-vars
22+
const { NODE_TEST_WPT, WPT_REPORT, WPT_INSPECT, ...env } = { ...process.env, NODE_TEST_WPT_MODIFIER_PROBE: '1' };
23+
for (const backend of ['thread', 'process']) {
24+
const result = spawnSync(process.execPath, [__filename, 'base64.any.js'], {
25+
env: { ...env, WPT_BACKEND: backend },
26+
encoding: 'utf8',
27+
timeout: common.platformTimeout(10_000),
28+
});
29+
const { error, status, stdout, stderr } = result;
30+
assert.ifError(error);
31+
assert.strictEqual(status, 0, stdout + stderr);
32+
const results = stdout.split('\n').filter((line) => line.startsWith('[PASS]'));
33+
assert.deepStrictEqual(results.sort(), [
34+
'[PASS] html/webappapis/atob/base64.any.html: modified script retains its filename',
35+
'[PASS] html/webappapis/atob/base64.any.worker.html: modified script retains its filename',
36+
]);
37+
}
38+
}

test/wpt/test-user-timing.js

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,31 @@
11
'use strict';
22

3+
const assert = require('assert');
4+
const { basename } = require('path');
35
const { WPTRunner } = require('../common/wpt');
46

57
const runner = new WPTRunner('user-timing');
68

79
runner.pretendGlobalThisAs('Window');
810

11+
runner.setScriptModifier((script) => {
12+
if (basename(script.filename) !== 'mark.any.js') return;
13+
14+
// A scheduling pause between mark() and now() can exceed any fixed
15+
// tolerance. Check that the mark falls between the surrounding clock
16+
// readings instead, leaving the upstream fixture unchanged.
17+
// https://github.com/nodejs/node/issues/40449
18+
function replace(from, to) {
19+
assert(script.code.includes(from), `Unexpected contents of ${script.filename}`);
20+
script.code = script.code.replaceAll(from, to);
21+
}
22+
23+
replace('var expectedTimes = new Array();',
24+
'var beforeTimes = [];\nvar expectedTimes = new Array();');
25+
replace('self.performance.mark("mark");',
26+
'beforeTimes.push(self.performance.now());\n self.performance.mark("mark");');
27+
replace('assert_approx_equals(entries[index].startTime, expectedTimes[index], testThreshold);',
28+
'assert_between_inclusive(entries[index].startTime, beforeTimes[index], expectedTimes[index]);');
29+
});
30+
931
runner.runJsTests();

test/wpt/wpt.status

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,8 +5,6 @@ prefix wpt
55
# sample-test : PASS,FLAKY
66

77
[true] # This section applies to all platforms
8-
# https://github.com/nodejs/node/issues/40449
9-
test-user-timing: PASS,FLAKY
108

119
[$system==win32]
1210

0 commit comments

Comments
 (0)