test: refactor WPTRunner and enable parallel WPT execution

PR-URL: https://github.com/nodejs/node/pull/47635
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>

Makefile

@@ -592,7 +592,8 @@ test-wpt: all
 test-wpt-report:
 	$(RM) -r out/wpt
 	mkdir -p out/wpt
-	WPT_REPORT=1 $(PYTHON) tools/test.py --shell $(NODE) $(PARALLEL_ARGS) wpt
+	-WPT_REPORT=1 $(PYTHON) tools/test.py --shell $(NODE) $(PARALLEL_ARGS) wpt
+	$(NODE) "$$PWD/tools/merge-wpt-reports.mjs"
 
 .PHONY: test-internet
 test-internet: all

test/common/wpt.js

@@ -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'));
+     * Return required and some optional properties
-      this.results = [...prev.results, ...this.results];
+     * https://github.com/web-platform-tests/wpt.fyi/blob/60da175/api/README.md?plain=1#L331-L335
-      this.time_start = prev.time_start;
+     */
-      this.time_end = Math.max(this.time_end, prev.time_end);
+    this.run_info = {
-      this.run_info = prev.run_info;
+      product: 'node.js',
-    } else {
+      ...getBrowserProperties(),
-      /**
+      revision: process.env.WPT_REVISION || 'unknown',
-       * Return required and some optional properties
+      os: getOs(),
-       * 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));
   }
 }
 
@@ -169,23 +164,27 @@ class ResourceLoader {
    * @param {string} from the path of the file loading this resource,
    *                      relative to the WPT folder.
    * @param {string} url the url of the resource being loaded.
-   * @param {boolean} asFetch if true, return the resource in a
-   *                          pseudo-Response object.
    */
-  read(from, url, asFetch = true) {
+  read(from, url) {
     const file = this.toRealFilePath(from, url);
-    if (asFetch) {
-      return fsPromises.readFile(file)
-        .then((data) => {
-          return {
-            ok: true,
-            json() { return JSON.parse(data.toString()); },
-            text() { return data.toString(); },
-          };
-        });
-    }
     return fs.readFileSync(file, 'utf8');
   }
+
+  /**
+   * Load a resource in test/fixtures/wpt specified with a URL
+   * @param {string} from the path of the file loading this resource,
+   *                      relative to the WPT folder.
+   * @param {string} url the url of the resource being loaded.
+   */
+  async readAsFetch(from, url) {
+    const file = this.toRealFilePath(from, url);
+    const data = await fsPromises.readFile(file);
+    return {
+      ok: true,
+      json() { return JSON.parse(data.toString()); },
+      text() { return data.toString(); },
+    };
+  }
 }
 
 class StatusRule {
@@ -251,16 +250,20 @@ class StatusRuleSet {
 
 // A specification of WPT test
 class WPTTestSpec {
+  #content;
+
   /**
    * @param {string} mod name of the WPT module, e.g.
    *                     'html/webappapis/microtask-queuing'
    * @param {string} filename path of the test, relative to mod, e.g.
    *                          'test.any.js'
    * @param {StatusRule[]} rules
+   * @param {string} variant test file variant
    */
-  constructor(mod, filename, rules) {
+  constructor(mod, filename, rules, variant = '') {
     this.module = mod;
     this.filename = filename;
+    this.variant = variant;
 
     this.requires = new Set();
     this.failedTests = [];
@@ -289,6 +292,17 @@ class WPTTestSpec {
     this.skipReasons = [...new Set(this.skipReasons)];
   }
 
+  /**
+   * @param {string} mod
+   * @param {string} filename
+   * @param {StatusRule[]} rules
+   */
+  static from(mod, filename, rules) {
+    const spec = new WPTTestSpec(mod, filename, rules);
+    const meta = spec.getMeta();
+    return meta.variant?.map((variant) => new WPTTestSpec(mod, filename, rules, variant)) || [spec];
+  }
+
   getRelativePath() {
     return path.join(this.module, this.filename);
   }
@@ -297,8 +311,38 @@ class WPTTestSpec {
     return fixtures.path('wpt', this.getRelativePath());
   }
 
+  /**
+   * @returns {string}
+   */
   getContent() {
-    return fs.readFileSync(this.getAbsolutePath(), 'utf8');
+    this.#content ||= fs.readFileSync(this.getAbsolutePath(), 'utf8');
+    return this.#content;
+  }
+
+  /**
+   * @returns {{ script?: string[]; variant?: string[]; [key: string]: string }} parsed META tags of a spec file
+   */
+  getMeta() {
+    const matches = this.getContent().match(/\/\/ META: .+/g);
+    if (!matches) {
+      return {};
+    }
+    const result = {};
+    for (const match of matches) {
+      const parts = match.match(/\/\/ META: ([^=]+?)=(.+)/);
+      const key = parts[1];
+      const value = parts[2];
+      if (key === 'script' || key === 'variant') {
+        if (result[key]) {
+          result[key].push(value);
+        } else {
+          result[key] = [value];
+        }
+      } else {
+        result[key] = value;
+      }
+    }
+    return result;
   }
 }
 
@@ -348,7 +392,6 @@ class StatusLoader {
    */
   constructor(path) {
     this.path = path;
-    this.loaded = false;
     this.rules = new StatusRuleSet();
     /** @type {WPTTestSpec[]} */
     this.specs = [];
@@ -388,9 +431,8 @@ class StatusLoader {
     for (const file of list) {
       const relativePath = path.relative(subDir, file);
       const match = this.rules.match(relativePath);
-      this.specs.push(new WPTTestSpec(this.path, relativePath, match));
+      this.specs.push(...WPTTestSpec.from(this.path, relativePath, match));
     }
-    this.loaded = true;
   }
 }
 
@@ -402,6 +444,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());
+        }
+      }
+    } else {
+      queue.push(fn);
+    }
+  };
+
+  return execute;
+};
+
 class WPTRunner {
   constructor(path) {
     this.path = path;
@@ -413,25 +478,21 @@ class WPTRunner {
 
     this.status = new StatusLoader(path);
     this.status.load();
-    this.specMap = new Map(
+    this.specs = new Set(this.status.specs);
-      this.status.specs.map((item) => [item.filename, item]),
-    );
 
     this.results = {};
     this.inProgress = new Set();
     this.workers = new Map();
     this.unexpectedFailures = [];
 
-    this.scriptsModifier = null;
-
     if (process.env.WPT_REPORT != null) {
-      this.report = new WPTReport();
+      this.report = new WPTReport(path);
     }
   }
 
   /**
    * Sets the Node.js flags passed to the worker.
-   * @param {Array<string>} flags
+   * @param {string[]} flags
    */
   setFlags(flags) {
     this.flags = flags;
@@ -453,13 +514,18 @@ class WPTRunner {
     this.scriptsModifier = modifier;
   }
 
-  fullInitScript(url, metaTitle) {
+  /**
+   * @param {WPTTestSpec} spec
+   */
+  fullInitScript(spec) {
+    const url = new URL(`/${spec.getRelativePath().replace(/\.js$/, '.html')}${spec.variant}`, 'http://wpt');
+    const title = spec.getMeta().title;
     let { initScript } = this;
 
     initScript = `${initScript}\n\n//===\nglobalThis.location = new URL("${url.href}");`;
 
-    if (metaTitle) {
+    if (title) {
-      initScript = `${initScript}\n\n//===\nglobalThis.META_TITLE = "${metaTitle}";`;
+      initScript = `${initScript}\n\n//===\nglobalThis.META_TITLE = "${title}";`;
     }
 
     if (this.globalThisInitScripts.length === null) {
@@ -527,47 +593,27 @@ class WPTRunner {
   // TODO(joyeecheung): work with the upstream to port more tests in .html
   // to .js.
   async runJsTests() {
-    let queue = [];
+    const queue = this.buildQueue();
 
-    // If the tests are run as `node test/wpt/test-something.js subset.any.js`,
+    const run = limit(os.availableParallelism());
-    // only `subset.any.js` will be run by the runner.
-    if (process.argv[2]) {
-      const filename = process.argv[2];
-      if (!this.specMap.has(filename)) {
-        throw new Error(`${filename} not found!`);
-      }
-      queue.push(this.specMap.get(filename));
-    } else {
-      queue = this.buildQueue();
-    }
-
-    this.inProgress = new Set(queue.map((spec) => spec.filename));
 
     for (const spec of queue) {
-      const testFileName = spec.filename;
       const content = spec.getContent();
-      const meta = spec.meta = this.getMeta(content);
+      const meta = spec.getMeta(content);
 
       const absolutePath = spec.getAbsolutePath();
       const relativePath = spec.getRelativePath();
       const harnessPath = fixtures.path('wpt', 'resources', 'testharness.js');
-      const scriptsToRun = [];
-      let needsGc = false;
 
       // Scripts specified with the `// META: script=` header
-      if (meta.script) {
+      const scriptsToRun = meta.script?.map((script) => {
-        for (const script of meta.script) {
+        const obj = {
-          if (script === '/common/gc.js') {
+          filename: this.resource.toRealFilePath(relativePath, script),
-            needsGc = true;
+          code: this.resource.read(relativePath, script),
-          }
+        };
-          const obj = {
+        this.scriptsModifier?.(obj);
-            filename: this.resource.toRealFilePath(relativePath, script),
+        return obj;
-            code: this.resource.read(relativePath, script, false),
+      }) ?? [];
-          };
-          this.scriptsModifier?.(obj);
-          scriptsToRun.push(obj);
-        }
-      }
       // The actual test
       const obj = {
         code: content,
@@ -576,53 +622,46 @@ class WPTRunner {
       this.scriptsModifier?.(obj);
       scriptsToRun.push(obj);
 
-      /**
+      run(async () => {
-       * 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 worker = new Worker(workerPath, {
           execArgv: this.flags,
           workerData: {
             testRelativePath: relativePath,
             wptRunner: __filename,
             wptPath: this.path,
-            initScript: this.fullInitScript(new URL(`/${relativePath.replace(/\.js$/, '.html')}${variant}`, 'http://wpt'), meta.title),
+            initScript: this.fullInitScript(spec),
             harness: {
               code: fs.readFileSync(harnessPath, 'utf8'),
               filename: harnessPath,
             },
             scriptsToRun,
-            needsGc,
+            needsGc: !!meta.script?.find((script) => script === '/common/gc.js'),
           },
         });
-        this.workers.set(testFileName, worker);
+        this.inProgress.add(spec);
+        this.workers.set(spec, worker);
 
         let reportResult;
         worker.on('message', (message) => {
           switch (message.type) {
             case 'result':
-              reportResult ||= this.report?.addResult(`/${relativePath}${variant}`, 'OK');
+              reportResult ||= this.report?.addResult(`/${relativePath}${spec.variant}`, 'OK');
-              return this.resultCallback(testFileName, message.result, reportResult);
+              return this.resultCallback(spec, message.result, reportResult);
             case 'completion':
-              return this.completionCallback(testFileName, message.status);
+              return this.completionCallback(spec, message.status);
             default:
               throw new Error(`Unexpected message from worker: ${message.type}`);
           }
         });
 
         worker.on('error', (err) => {
-          if (!this.inProgress.has(testFileName)) {
+          if (!this.inProgress.has(spec)) {
             // The test is already finished. Ignore errors that occur after it.
             // This can happen normally, for example in timers tests.
             return;
           }
           this.fail(
-            testFileName,
+            spec,
             {
               status: NODE_UNCAUGHT,
               name: 'evaluation in WPTRunner.runJsTests()',
@@ -631,21 +670,23 @@ class WPTRunner {
             },
             kUncaught,
           );
-          this.inProgress.delete(testFileName);
+          this.inProgress.delete(spec);
         });
 
         await events.once(worker, 'exit').catch(() => {});
-      }
+      });
     }
 
     process.on('exit', () => {
       if (this.inProgress.size > 0) {
-        for (const filename of this.inProgress) {
+        for (const id of this.inProgress) {
-          this.fail(filename, { name: 'Unknown' }, kIncomplete);
+          const spec = this.specs.get(id);
+          this.fail(spec, { name: 'Unknown' }, kIncomplete);
         }
       }
       inspect.defaultOptions.depth = Infinity;
       // Sorts the rules to have consistent output
+      console.log('');
       console.log(JSON.stringify(Object.keys(this.results).sort().reduce(
         (obj, key) => {
           obj[key] = this.results[key];
@@ -670,11 +711,11 @@ class WPTRunner {
       }
 
       const unexpectedPasses = [];
-      for (const specMap of queue) {
+      for (const specs of queue) {
-        const key = specMap.filename;
+        const key = specs.filename;
 
         // File has no expected failures
-        if (!specMap.failedTests.length) {
+        if (!specs.failedTests.length) {
           continue;
         }
 
@@ -684,8 +725,8 @@ class WPTRunner {
         }
 
         // Full check: every expected to fail test is present
-        if (specMap.failedTests.some((expectedToFail) => {
+        if (specs.failedTests.some((expectedToFail) => {
-          if (specMap.flakyTests.includes(expectedToFail)) {
+          if (specs.flakyTests.includes(expectedToFail)) {
             return false;
           }
           return this.results[key]?.fail?.expected?.includes(expectedToFail) !== true;
@@ -700,6 +741,7 @@ class WPTRunner {
       const ran = queue.length;
       const total = ran + skipped;
       const passed = ran - expectedFailures - failures.length;
+      console.log('');
       console.log(`Ran ${ran}/${total} tests, ${skipped} skipped,`,
                   `${passed} passed, ${expectedFailures} expected failures,`,
                   `${failures.length} unexpected failures,`,
@@ -719,11 +761,6 @@ class WPTRunner {
     });
   }
 
-  getTestTitle(filename) {
-    const spec = this.specMap.get(filename);
-    return spec.meta?.title || filename.split('.')[0];
-  }
-
   // Map WPT test status to strings
   getTestStatus(status) {
     switch (status) {
@@ -744,42 +781,39 @@ class WPTRunner {
    * Report the status of each specific test case (there could be multiple
    * in one test file).
    *
-   * @param {string} filename
+   * @param {WPTTestSpec} spec
    * @param {Test} test  The Test object returned by WPT harness
    */
-  resultCallback(filename, test, reportResult) {
+  resultCallback(spec, test, reportResult) {
     const status = this.getTestStatus(test.status);
-    console.log(`---- ${test.name} ----`);
     if (status !== kPass) {
-      this.fail(filename, test, status, reportResult);
+      this.fail(spec, test, status, reportResult);
     } else {
-      this.succeed(filename, test, status, reportResult);
+      this.succeed(test, status, reportResult);
     }
   }
 
   /**
    * Report the status of each WPT test (one per file)
    *
-   * @param {string} filename
+   * @param {WPTTestSpec} spec
    * @param {object} harnessStatus - The status object returned by WPT harness.
    */
-  completionCallback(filename, harnessStatus) {
+  completionCallback(spec, harnessStatus) {
     // Treat it like a test case failure
     if (harnessStatus.status === 2) {
-      const title = this.getTestTitle(filename);
+      this.resultCallback(spec, { status: 2, name: 'Unknown' });
-      console.log(`---- ${title} ----`);
-      this.resultCallback(filename, { status: 2, name: 'Unknown' });
     }
-    this.inProgress.delete(filename);
+    this.inProgress.delete(spec);
     // Always force termination of the worker. Some tests allocate resources
     // that would otherwise keep it alive.
-    this.workers.get(filename).terminate();
+    this.workers.get(spec).terminate();
   }
 
-  addTestResult(filename, item) {
+  addTestResult(spec, item) {
-    let result = this.results[filename];
+    let result = this.results[spec.filename];
     if (!result) {
-      result = this.results[filename] = {};
+      result = this.results[spec.filename] = {};
     }
     if (item.status === kSkip) {
       // { filename: { skip: 'reason' } }
@@ -801,17 +835,15 @@ class WPTRunner {
     }
   }
 
-  succeed(filename, test, status, reportResult) {
+  succeed(test, status, reportResult) {
     console.log(`[${status.toUpperCase()}] ${test.name}`);
     reportResult?.addSubtest(test.name, 'PASS');
   }
 
-  fail(filename, test, status, reportResult) {
+  fail(spec, test, status, reportResult) {
-    const spec = this.specMap.get(filename);
     const expected = spec.failedTests.includes(test.name);
     if (expected) {
       console.log(`[EXPECTED_FAILURE][${status.toUpperCase()}] ${test.name}`);
-      console.log(test.message || status);
     } else {
       console.log(`[UNEXPECTED_FAILURE][${status.toUpperCase()}] ${test.name}`);
     }
@@ -820,12 +852,12 @@ class WPTRunner {
       console.log(test.stack);
     }
     const command = `${process.execPath} ${process.execArgv}` +
-                    ` ${require.main.filename} ${filename}`;
+                    ` ${require.main.filename} '${spec.filename}${spec.variant}'`;
     console.log(`Command: ${command}\n`);
 
     reportResult?.addSubtest(test.name, 'FAIL', test.message);
 
-    this.addTestResult(filename, {
+    this.addTestResult(spec, {
       name: test.name,
       expected,
       status: kFail,
@@ -833,57 +865,52 @@ class WPTRunner {
     });
   }
 
-  skip(filename, reasons) {
+  skip(spec, reasons) {
-    const title = this.getTestTitle(filename);
-    console.log(`---- ${title} ----`);
     const joinedReasons = reasons.join('; ');
-    console.log(`[SKIPPED] ${joinedReasons}`);
+    console.log(`[SKIPPED] ${spec.filename}${spec.variant}: ${joinedReasons}`);
-    this.addTestResult(filename, {
+    this.addTestResult(spec, {
       status: kSkip,
       reason: joinedReasons,
     });
   }
 
-  getMeta(code) {
-    const matches = code.match(/\/\/ META: .+/g);
-    if (!matches) {
-      return {};
-    }
-    const result = {};
-    for (const match of matches) {
-      const parts = match.match(/\/\/ META: ([^=]+?)=(.+)/);
-      const key = parts[1];
-      const value = parts[2];
-      if (key === 'script' || key === 'variant') {
-        if (result[key]) {
-          result[key].push(value);
-        } else {
-          result[key] = [value];
-        }
-      } else {
-        result[key] = value;
-      }
-    }
-    return result;
-  }
-
   buildQueue() {
     const queue = [];
-    for (const spec of this.specMap.values()) {
+    let argFilename;
-      const filename = spec.filename;
+    let argVariant;
+    if (process.argv[2]) {
+      ([argFilename, argVariant = ''] = process.argv[2].split('?'));
+    }
+    for (const spec of this.specs) {
+      if (argFilename) {
+        if (spec.filename === argFilename && (!argVariant || spec.variant.substring(1) === argVariant)) {
+          queue.push(spec);
+        }
+        continue;
+      }
+
       if (spec.skipReasons.length > 0) {
-        this.skip(filename, spec.skipReasons);
+        this.skip(spec, spec.skipReasons);
         continue;
       }
 
       const lackingIntl = intlRequirements.isLacking(spec.requires);
       if (lackingIntl) {
-        this.skip(filename, [ `requires ${lackingIntl}` ]);
+        this.skip(spec, [ `requires ${lackingIntl}` ]);
         continue;
       }
 
       queue.push(spec);
     }
+
+    // If the tests are run as `node test/wpt/test-something.js subset.any.js`,
+    // only `subset.any.js` (all variants) will be run by the runner.
+    // If the tests are run as `node test/wpt/test-something.js 'subset.any.js?1-10'`,
+    // only the `?1-10` variant of `subset.any.js` will be run by the runner.
+    if (argFilename && queue.length === 0) {
+      throw new Error(`${process.argv[2]} not found!`);
+    }
+
     return queue;
   }
 }

test/common/wpt/worker.js

@@ -20,11 +20,11 @@ globalThis.GLOBAL = {
 };
 globalThis.require = require;
 
-// This is a mock, because at the moment fetch is not implemented
+// This is a mock for non-fetch tests that use fetch to resolve
-// in Node.js, but some tests and harness depend on this to pull
+// a relative fixture file.
-// resources.
+// Actual Fetch API WPTs are executed in nodejs/undici.
 globalThis.fetch = function fetch(file) {
-  return resource.read(workerData.testRelativePath, file, true);
+  return resource.readAsFetch(workerData.testRelativePath, file);
 };
 
 if (workerData.initScript) {

test/wpt/testcfg.py

@@ -3,4 +3,4 @@ sys.path.append(os.path.join(os.path.dirname(__file__), '..'))
 import testpy
 
 def GetConfiguration(context, root):
-  return testpy.SimpleTestConfiguration(context, root, 'wpt')
+  return testpy.ParallelTestConfiguration(context, root, 'wpt')

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));