CR

nodejs · Dec 3, 2022 · 3e3218f · 3e3218f
1 parent c1c9fa4
commit 3e3218f
Show file tree

Hide file tree

Showing 9 changed files with 105 additions and 43 deletions.
diff --git a/lib/internal/main/test_runner.js b/lib/internal/main/test_runner.js
@@ -6,7 +6,7 @@ const {
 const { getOptionValue } = require('internal/options');
 const { isUsingInspector } = require('internal/util/inspector');
 const { run } = require('internal/test_runner/runner');
-const { getTestReporter } = require('internal/test_runner/utils');
+const { setupTestReporters } = require('internal/test_runner/utils');
 const { exitCodes: { kGenericUserError } } = internalBinding('errors');
 
 prepareMainThreadExecution(false);
@@ -22,8 +22,9 @@ if (isUsingInspector()) {
   inspectPort = process.debugPort;
 }
 
-const tapStream = getTestReporter(run({ concurrency, inspectPort, watch: getOptionValue('--watch') }));
-tapStream.pipe(process.stdout);
+const tapStream = run({ concurrency, inspectPort, watch: getOptionValue('--watch') });
+const reporters = setupTestReporters(tapStream);
+reporters.pipe(process.stdout);
 tapStream.once('test:fail', () => {
   process.exitCode = kGenericUserError;
 });
diff --git a/lib/internal/test_runner/harness.js b/lib/internal/test_runner/harness.js
@@ -18,7 +18,7 @@ const { exitCodes: { kGenericUserError } } = internalBinding('errors');
 const { kEmptyObject } = require('internal/util');
 const { getOptionValue } = require('internal/options');
 const { kCancelledByParent, Test, ItTest, Suite } = require('internal/test_runner/test');
-const { getTestReporter } = require('internal/test_runner/utils');
+const { setupTestReporters } = require('internal/test_runner/utils');
 const { bigint: hrtime } = process.hrtime;
 
 const isTestRunnerCli = getOptionValue('--test');
@@ -120,7 +120,7 @@ let globalRoot;
 function getGlobalRoot() {
   if (!globalRoot) {
     globalRoot = createTestTree();
-    const reporter = getTestReporter(globalRoot.reporter);
+    const reporter = setupTestReporters(globalRoot.reporter);
     reporter.pipe(process.stdout);
     reporter.once('test:fail', () => {
       process.exitCode = kGenericUserError;

diff --git a/lib/internal/test_runner/reporter_stream.js b/lib/internal/test_runner/reporter_stream.js
@@ -0,0 +1,78 @@
+'use strict';
+const {
+  ArrayPrototypePush,
+  ArrayPrototypeShift,
+} = primordials;
+const Readable = require('internal/streams/readable');
+
+class ReporterStream extends Readable {
+  #buffer;
+  #canPush;
+
+  constructor() {
+    super({ objectMode: true });
+    this.#buffer = [];
+    this.#canPush = true;
+  }
+
+  _read() {
+    this.#canPush = true;
+
+    while (this.#buffer.length > 0) {
+      const chunk = ArrayPrototypeShift(this.#buffer);
+
+      if (!this.#tryPush(chunk)) {
+        return;
+      }
+    }
+  }
+
+  fail(nesting, testNumber, name, details, directive) {
+    this.#emit('test:fail', { __proto__: null, name, nesting, testNumber, details, ...directive });
+  }
+
+  ok(nesting, testNumber, name, details, directive) {
+    this.#emit('test:pass', { __proto__: null, name, nesting, testNumber, details, ...directive });
+  }
+
+  plan() {
+    // NOT IMPLEMENTED
+  }
+
+  getSkip(reason) {
+    return { __proto__: null, skip: reason };
+  }
+
+  getTodo(reason) {
+    return { __proto__: null, todo: reason };
+  }
+
+  subtest(nesting, name) {
+    this.#emit('test:subtest', { nesting, name });
+  }
+
+  diagnostic(nesting, message) {
+    this.#emit('test:diagnostic', { nesting, message });
+  }
+
+  version() {
+    // NOT IMPLEMENTED
+  }
+
+  #emit(type, data) {
+    this.emit(type, data);
+    this.#tryPush({ type, data });
+  }
+
+  #tryPush(message) {
+    if (this.#canPush) {
+      this.#canPush = this.push(message);
+    } else {
+      ArrayPrototypePush(this.#buffer, message);
+    }
+
+    return this.#canPush;
+  }
+}
+
+module.exports = { ReporterStream };
diff --git a/lib/internal/test_runner/runner.js b/lib/internal/test_runner/runner.js
@@ -118,9 +118,7 @@ function createTestFileList() {
 
 function filterExecArgv(arg, i, arr) {
   return !ArrayPrototypeIncludes(kFilterArgs, arg) &&
-    !ArrayPrototypeIncludes(kFilterArgValues, arg) &&
-    !ArrayPrototypeIncludes(kFilterArgValues, arr[i - 1]) &&
-    !ArrayPrototypeSome(kFilterArgValues, (p) => StringPrototypeStartsWith(arg, `${p}=`));
+  !ArrayPrototypeSome(kFilterArgValues, (p) => arg === p || (i > 0 && arr[i - 1] === p) || StringPrototypeStartsWith(arg, `${p}=`));
 }
 
 function getRunArgs({ path, inspectPort }) {

diff --git a/lib/internal/test_runner/tap_stream.js b/lib/internal/test_runner/tap_stream.js
@@ -32,8 +32,8 @@ class TapStream extends Readable {
   #buffer;
   #canPush;
 
-  constructor(options = kEmptyObject) {
-    super(options);
+  constructor() {
+    super();
     this.#buffer = [];
     this.#canPush = true;
   }
@@ -42,30 +42,30 @@ class TapStream extends Readable {
     this.#canPush = true;
 
     while (this.#buffer.length > 0) {
-      const chunk = ArrayPrototypeShift(this.#buffer);
+      const line = ArrayPrototypeShift(this.#buffer);
 
-      if (!this.#tryPush(chunk)) {
+      if (!this.#tryPush(line)) {
         return;
       }
     }
   }
 
   fail(nesting, testNumber, name, details, directive) {
-    this.#emit('test:fail', { __proto__: null, name, nesting, testNumber, details, ...directive });
+    this.emit('test:fail', { __proto__: null, name, nesting, testNumber, details, ...directive });
     this.#test(nesting, testNumber, 'not ok', name, directive);
     this.#details(nesting, details);
   }
 
   ok(nesting, testNumber, name, details, directive) {
-    this.#emit('test:pass', { __proto__: null, name, nesting, testNumber, details, ...directive });
+    this.emit('test:pass', { __proto__: null, name, nesting, testNumber, details, ...directive });
     this.#test(nesting, testNumber, 'ok', name, directive);
     this.#details(nesting, details);
   }
 
   plan(nesting, count, explanation) {
     const exp = `${explanation ? ` # ${tapEscape(explanation)}` : ''}`;
 
-    this.#tryPushString(`${this.#indent(nesting)}1..${count}${exp}\n`);
+    this.#tryPush(`${this.#indent(nesting)}1..${count}${exp}\n`);
   }
 
   getSkip(reason) {
@@ -77,8 +77,8 @@ class TapStream extends Readable {
   }
 
   subtest(nesting, name) {
-    this.#emit('test:subtest', { nesting, name });
-    this.#tryPushString(`${this.#indent(nesting)}# Subtest: ${tapEscape(name)}\n`);
+    this.emit('test:subtest', { nesting, name });
+    this.#tryPush(`${this.#indent(nesting)}# Subtest: ${tapEscape(name)}\n`);
   }
 
   #details(nesting, data = kEmptyObject) {
@@ -90,16 +90,16 @@ class TapStream extends Readable {
     details += jsToYaml(indent, 'duration_ms', duration);
     details += jsToYaml(indent, null, error);
     details += `${indent}  ...\n`;
-    this.#tryPushString(details);
+    this.#tryPush(details);
   }
 
   diagnostic(nesting, message) {
-    this.#emit('test:diagnostic', message);
-    this.#tryPushString(`${this.#indent(nesting)}# ${tapEscape(message)}\n`);
+    this.emit('test:diagnostic', { message, nesting });
+    this.#tryPush(`${this.#indent(nesting)}# ${tapEscape(message)}\n`);
   }
 
   version(spec = kDefaultTAPVersion) {
-    this.#tryPushString(`TAP version ${spec}\n`);
+    this.#tryPush(`TAP version ${spec}\n`);
   }
 
   #indent(nesting) {
@@ -123,23 +123,7 @@ class TapStream extends Readable {
 
     line += '\n';
 
-    this.#tryPushString(line);
-  }
-
-  #emit(type, data) {
-    this.emit(type, data);
-    this.#tryPushObject({ type, data });
-  }
-
-  #tryPushString(str) {
-    if (!this._readableState.objectMode) {
-      this.#tryPush(str);
-    }
-  }
-  #tryPushObject(obj) {
-    if (this._readableState.objectMode) {
-      this.#tryPush(obj);
-    }
+    this.#tryPush(line);
   }
 
   #tryPush(message) {

diff --git a/lib/internal/test_runner/test.js b/lib/internal/test_runner/test.js
@@ -35,6 +35,7 @@ const {
 const { getOptionValue } = require('internal/options');
 const { MockTracker } = require('internal/test_runner/mock');
 const { TapStream } = require('internal/test_runner/tap_stream');
+const { ReporterStream } = require('internal/test_runner/reporter_stream');
 const {
   convertStringToRegExp,
   createDeferredCallback,
@@ -174,7 +175,7 @@ class Test extends AsyncResource {
       this.concurrency = 1;
       this.nesting = 0;
       this.only = testOnlyFlag;
-      this.reporter = new TapStream({ __proto__: null, objectMode: hasReporters });
+      this.reporter = hasReporters ? new ReporterStream() : new TapStream();
       this.runOnlySubtests = this.only;
       this.testNumber = 0;
       this.timeout = kDefaultTimeout;

diff --git a/lib/internal/test_runner/utils.js b/lib/internal/test_runner/utils.js
@@ -78,7 +78,7 @@ function convertStringToRegExp(str, name) {
 }
 
 let _module;
-function getTestReporter(reporter) {
+function setupTestReporters(reporter) {
   if (getOptionValue('--test-reporter')) {
     _module ??= new Module('node:test');
     const reporters = _module.require(getOptionValue('--test-reporter'));
@@ -93,5 +93,5 @@ module.exports = {
   doesPathMatchFilter,
   isSupportedFileType,
   isTestFailureError,
-  getTestReporter,
+  setupTestReporters,
 };
diff --git a/lib/test/reporter/spec.js b/lib/test/reporter/spec.js
@@ -57,7 +57,7 @@ class SpecReporter extends Transform {
         ArrayPrototypeUnshift(this.#stack, data);
         break;
       case 'test:diagnostic':
-        return `${color}${symbol}${data}${white}\n`;
+        return `${color}${this.#indent(data.nesting)}${symbol}${data.message}${white}\n`;
     }
   }
   _transform({ type, data }, encoding, callback) {

diff --git a/src/node_options.cc b/src/node_options.cc
@@ -549,7 +549,7 @@ EnvironmentOptionsParser::EnvironmentOptionsParser() {
             "run tests whose name matches this regular expression",
             &EnvironmentOptions::test_name_pattern);
   AddOption("--test-reporter",
-            "report test using the given reporter",
+            "report test output using the given reporter",
             &EnvironmentOptions::test_reporter);
   AddOption("--test-only",
             "run tests with 'only' option set",