test: deflake user timing WPT assertions · nodejs/node@80c1bf7 · GitHub
Skip to content

Commit 80c1bf7

Browse files
panvaaduh95
panva
authored and
aduh95
committed
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

‎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
};

‎test/parallel/test-common-wpt-script-modifier.js‎

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

0 commit comments

Comments
 (0)