From 80c1bf7fb534e90f1fe7ba051bd85b93e0b80436 Mon Sep 17 00:00:00 2001 From: Filip Skokan Date: Fri, 18 Sep 2026 12:54:52 +0200 Subject: [PATCH] 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 Assisted-by: Codex PR-URL: https://github.com/nodejs/node/pull/66036 Fixes: https://github.com/nodejs/node/issues/40449 Refs: https://github.com/nodejs/node/pull/41203 Reviewed-By: Antoine du Hamel Reviewed-By: Matteo Collina --- test/common/wpt.js | 15 +++----- test/common/wpt/webworker.js | 18 +++++++-- .../test-common-wpt-script-modifier.js | 38 +++++++++++++++++++ test/wpt/test-user-timing.js | 22 +++++++++++ test/wpt/wpt.status | 2 - 5 files changed, 80 insertions(+), 15 deletions(-) create mode 100644 test/parallel/test-common-wpt-script-modifier.js diff --git a/test/common/wpt.js b/test/common/wpt.js index e2668d75da7..29180fdeb35 100644 --- a/test/common/wpt.js +++ b/test/common/wpt.js @@ -1039,15 +1039,10 @@ class WPTRunner { this.scriptsModifier?.(obj); return obj; }) ?? []; - if (!isWebWorkerTest) { - // The actual test - const obj = { - code: content, - filename: absolutePath, - }; - this.scriptsModifier?.(obj); - scriptsToRun.push(obj); - } + // The actual test, including tests imported by a Web Worker. + const testScript = { code: content, filename: absolutePath }; + this.scriptsModifier?.(testScript); + if (!isWebWorkerTest) scriptsToRun.push(testScript); jobs.push(run(async () => { this.inProgress.add(spec); @@ -1066,6 +1061,8 @@ class WPTRunner { // Set when the test runs inside an actual Web Worker. webWorker: isWebWorkerTest ? { path: absolutePath, + modifiedScript: testScript.code !== content || testScript.filename !== absolutePath ? + testScript : undefined, isAnyTest, initScript: this.initScript, title: meta.title, diff --git a/test/common/wpt/webworker.js b/test/common/wpt/webworker.js index 1f94c82c02a..e9a8ebe53d7 100644 --- a/test/common/wpt/webworker.js +++ b/test/common/wpt/webworker.js @@ -123,11 +123,21 @@ globalThis.onmessage = ({ data }) => { for (const script of data.scripts) { globalThis.importScripts(pathToFileURL(script).href); } - globalThis.importScripts(pathToFileURL(data.path).href); - // eslint-disable-next-line no-undef - done(); + } + + if (data.modifiedScript) { + runInThisContext(data.modifiedScript.code, { + filename: data.modifiedScript.filename, + importModuleDynamically: USE_MAIN_CONTEXT_DEFAULT_LOADER, + }); } else { - // *.worker.js tests import testharness.js and call done() themselves. + // Keep unmodified tests on the native script loading path. globalThis.importScripts(pathToFileURL(data.path).href); } + + if (data.isAnyTest) { + // *.worker.js tests import testharness.js and call done() themselves. + // eslint-disable-next-line no-undef + done(); + } }; diff --git a/test/parallel/test-common-wpt-script-modifier.js b/test/parallel/test-common-wpt-script-modifier.js new file mode 100644 index 00000000000..cbda2e6e825 --- /dev/null +++ b/test/parallel/test-common-wpt-script-modifier.js @@ -0,0 +1,38 @@ +'use strict'; + +const common = require('../common'); +const assert = require('assert'); +const fixtures = require('../common/fixtures'); +const { spawnSync } = require('child_process'); + +if (process.env.NODE_TEST_WPT_MODIFIER_PROBE === '1') { + const { WPTRunner } = require('../common/wpt'); + const runner = new WPTRunner('html/webappapis/atob'); + runner.setScriptModifier(common.mustCall((script) => { + assert.strictEqual(script.filename, + fixtures.path('wpt', 'html/webappapis/atob', 'base64.any.js')); + script.filename += '.modified'; + script.code = `test(() => { + assert_true(new Error().stack.includes(${JSON.stringify(`${script.filename}:`)})); + }, 'modified script retains its filename');`; + }, 2)); + runner.runJsTests(); +} else { + // eslint-disable-next-line no-unused-vars + const { NODE_TEST_WPT, WPT_REPORT, WPT_INSPECT, ...env } = { ...process.env, NODE_TEST_WPT_MODIFIER_PROBE: '1' }; + for (const backend of ['thread', 'process']) { + const result = spawnSync(process.execPath, [__filename, 'base64.any.js'], { + env: { ...env, WPT_BACKEND: backend }, + encoding: 'utf8', + timeout: common.platformTimeout(10_000), + }); + const { error, status, stdout, stderr } = result; + assert.ifError(error); + assert.strictEqual(status, 0, stdout + stderr); + const results = stdout.split('\n').filter((line) => line.startsWith('[PASS]')); + assert.deepStrictEqual(results.sort(), [ + '[PASS] html/webappapis/atob/base64.any.html: modified script retains its filename', + '[PASS] html/webappapis/atob/base64.any.worker.html: modified script retains its filename', + ]); + } +} diff --git a/test/wpt/test-user-timing.js b/test/wpt/test-user-timing.js index 151c06a86d0..57db5837bbe 100644 --- a/test/wpt/test-user-timing.js +++ b/test/wpt/test-user-timing.js @@ -1,9 +1,31 @@ 'use strict'; +const assert = require('assert'); +const { basename } = require('path'); const { WPTRunner } = require('../common/wpt'); const runner = new WPTRunner('user-timing'); runner.pretendGlobalThisAs('Window'); +runner.setScriptModifier((script) => { + if (basename(script.filename) !== 'mark.any.js') return; + + // A scheduling pause between mark() and now() can exceed any fixed + // tolerance. Check that the mark falls between the surrounding clock + // readings instead, leaving the upstream fixture unchanged. + // https://github.com/nodejs/node/issues/40449 + function replace(from, to) { + assert(script.code.includes(from), `Unexpected contents of ${script.filename}`); + script.code = script.code.replaceAll(from, to); + } + + replace('var expectedTimes = new Array();', + 'var beforeTimes = [];\nvar expectedTimes = new Array();'); + replace('self.performance.mark("mark");', + 'beforeTimes.push(self.performance.now());\n self.performance.mark("mark");'); + replace('assert_approx_equals(entries[index].startTime, expectedTimes[index], testThreshold);', + 'assert_between_inclusive(entries[index].startTime, beforeTimes[index], expectedTimes[index]);'); +}); + runner.runJsTests(); diff --git a/test/wpt/wpt.status b/test/wpt/wpt.status index d20d108ac99..de3b3024627 100644 --- a/test/wpt/wpt.status +++ b/test/wpt/wpt.status @@ -5,8 +5,6 @@ prefix wpt # sample-test : PASS,FLAKY [true] # This section applies to all platforms -# https://github.com/nodejs/node/issues/40449 -test-user-timing: PASS,FLAKY [$system==win32]