fix: move test reporter loading

Move the logic for handling --test-reporter out of the general module loader and into the test_runner subsystem. PR-URL: nodejs/node#45923 Reviewed-By: Moshe Atlow <[email protected]> Reviewed-By: Benjamin Gruenbaum <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Jacob Smith <[email protected]> Reviewed-By: Antoine du Hamel <[email protected]> (cherry picked from commit 12c0571c8fece32d274eaf0ae197c0eb1948fe11)
nodejs · Feb 8, 2023 · a5e0e9e · a5e0e9e
1 parent 1ec1348
commit a5e0e9e
Show file tree

Hide file tree

Showing 4 changed files with 21 additions and 8 deletions.
diff --git a/README.md b/README.md
@@ -447,7 +447,7 @@ The following built-reporters are supported:
   The `spec` reporter outputs the test results in a human-readable format.
 
 * `dot`
-  The `dot` reporter outputs the test results in a comact format,
+  The `dot` reporter outputs the test results in a compact format,
   where each passing test is represented by a `.`,
   and each failing test is represented by a `X`.
 
@@ -564,6 +564,9 @@ module.exports = async function * customReporter(source) {
 };
 ```
 
+The value provided to `--test-reporter` should be a string like one used in an
+`import()` in JavaScript code, or a value provided for [`--import`][].
+
 ### Multiple reporters
 
 The [`--test-reporter`][] flag can be specified multiple times to report test

diff --git a/lib/internal/test_runner/utils.js b/lib/internal/test_runner/utils.js
@@ -1,4 +1,4 @@
-// https://github.com/nodejs/node/blob/a1b27b25bb01aadd3fd2714e4b136db11b7eb85a/lib/internal/test_runner/utils.js
+// https://github.com/nodejs/node/blob/12c0571c8fece32d274eaf0ae197c0eb1948fe11/lib/internal/test_runner/utils.js
 'use strict'
 const {
   ArrayPrototypePush,
@@ -103,7 +103,10 @@ const kDefaultDestination = 'stdout'
 async function getReportersMap (reporters, destinations) {
   return SafePromiseAllReturnArrayLike(reporters, async (name, i) => {
     const destination = kBuiltinDestinations.get(destinations[i]) ?? createWriteStream(destinations[i])
-    let reporter = await import(kBuiltinReporters.get(name) ?? name)
+
+    // Load the test reporter passed to --test-reporter
+    const reporterSpecifier = kBuiltinReporters.get(name) ?? name
+    let reporter = await import(reporterSpecifier)
 
     if (reporter?.default) {
       reporter = reporter.default

diff --git a/test/common/fixtures.js b/test/common/fixtures.js
@@ -1,14 +1,19 @@
-// https://github.com/nodejs/node/blob/1aab13cad9c800f4121c1d35b554b78c1b17bdbd/test/common/fixtures.js
+// https://github.com/nodejs/node/blob/12c0571c8fece32d274eaf0ae197c0eb1948fe11/test/common/fixtures.js
 'use strict'
 
 const path = require('path')
+const { pathToFileURL } = require('url')
 
 const fixturesDir = path.join(__dirname, '..', 'fixtures')
 
 function fixturesPath (...args) {
   return path.join(fixturesDir, ...args)
 }
+function fixturesFileURL (...args) {
+  return pathToFileURL(fixturesPath(...args))
+}
 
 module.exports = {
-  path: fixturesPath
+  path: fixturesPath,
+  fileURL: fixturesFileURL
 }
diff --git a/test/parallel/test-runner-reporters.js b/test/parallel/test-runner-reporters.js
@@ -1,4 +1,4 @@
-// https://github.com/nodejs/node/blob/a1b27b25bb01aadd3fd2714e4b136db11b7eb85a/test/parallel/test-runner-reporters.js
+// https://github.com/nodejs/node/blob/12c0571c8fece32d274eaf0ae197c0eb1948fe11/test/parallel/test-runner-reporters.js
 'use strict'
 
 require('../common')
@@ -91,10 +91,12 @@ describe('node:test reporters', { concurrency: true }, () => {
     it(`should support a '${ext}' file as a custom reporter`, async () => {
       const filename = `custom.${ext}`
       const child = spawnSync(process.execPath,
-        ['--test', '--test-reporter', fixtures.path('test-runner/custom_reporters/', filename),
+        ['--test', '--test-reporter', fixtures.fileURL('test-runner/custom_reporters/', filename),
           testFile])
       assert.strictEqual(child.stderr.toString(), '')
-      assert.strictEqual(child.stdout.toString(), `${filename} {"test:start":5,"test:pass":2,"test:fail":3,"test:plan":3,"test:diagnostic":7}`)
+      const stdout = child.stdout.toString()
+      assert.match(stdout, /{"test:start":5,"test:pass":2,"test:fail":3,"test:plan":3,"test:diagnostic":\d+}$/)
+      assert.strictEqual(stdout.slice(0, filename.length + 2), `${filename} {`)
     })
   })
 })