test_runner: flatten TAP output when running using --test

lib/internal/test_runner/runner.js

@@ -8,9 +8,9 @@ const {
   ArrayPrototypeSlice,
   ArrayPrototypeSome,
   ArrayPrototypeSort,
+  Number,
   ObjectAssign,
   PromisePrototypeThen,
-  SafePromiseAll,
   SafePromiseAllReturnVoid,
   SafePromiseAllSettledReturnVoid,
   SafeMap,
@@ -35,7 +35,7 @@ const { validateArray, validateBoolean } = require('internal/validators');
 const { getInspectPort, isUsingInspector, isInspectorMessage } = require('internal/util/inspector');
 const { kEmptyObject } = require('internal/util');
 const { createTestTree } = require('internal/test_runner/harness');
-const { kSubtestsFailed, Test } = require('internal/test_runner/test');
+const { kSubtestsFailed, kTestCodeFailure, kCancelledByParent, Test } = require('internal/test_runner/test');
 const { TapParser } = require('internal/test_runner/tap_parser');
 const { YAMLToJs } = require('internal/test_runner/yaml_to_js');
 const { TokenKind } = require('internal/test_runner/tap_lexer');
@@ -133,6 +133,10 @@ function getRunArgs({ path, inspectPort }) {
 
 class FileTest extends Test {
   #buffer = [];
+  #hasSubtests = false;
+  get skipReporting() {
+    return this.#hasSubtests && (!this.error || this.error.failureType === kSubtestsFailed);
+  }
   #checkNestedComment({ comment }) {
     const firstSpaceIndex = StringPrototypeIndexOf(comment, ' ');
     if (firstSpaceIndex === -1) return false;
@@ -141,8 +145,6 @@ class FileTest extends Test {
           ArrayPrototypeIncludes(kDiagnosticsFilterArgs, StringPrototypeSlice(comment, 0, firstSpaceIndex));
   }
   #handleReportItem({ kind, node, comments, nesting = 0 }) {
-    nesting += 1;
-
     if (comments) {
       ArrayPrototypeForEach(comments, (comment) => this.reporter.diagnostic(nesting, this.name, comment));
     }
@@ -153,17 +155,20 @@ class FileTest extends Test {
         break;
 
       case TokenKind.TAP_PLAN:
+        if (nesting === 0 && this.skipReporting) {
+          break;
+        }
         this.reporter.plan(nesting, this.name, node.end - node.start + 1);
         break;
 
       case TokenKind.TAP_SUBTEST_POINT:
         this.reporter.start(nesting, this.name, node.name);
         break;
 
-      case TokenKind.TAP_TEST_POINT:
-        // eslint-disable-next-line no-case-declarations
+      case TokenKind.TAP_TEST_POINT: {
+
         const { todo, skip, pass } = node.status;
-        // eslint-disable-next-line no-case-declarations
+
         let directive;
 
         if (skip) {
@@ -174,29 +179,21 @@ class FileTest extends Test {
           directive = kEmptyObject;
         }
 
-        if (pass) {
-          this.reporter.ok(
-            nesting,
-            this.name,
-            node.id,
-            node.description,
-            YAMLToJs(node.diagnostics),
-            directive
-          );
-        } else {
-          this.reporter.fail(
-            nesting,
-            this.name,
-            node.id,
-            node.description,
-            YAMLToJs(node.diagnostics),
-            directive
-          );
+        this.#hasSubtests = true;
+        const diagnostics = YAMLToJs(node.diagnostics);
+        const cancelled = diagnostics.error?.failureType === kCancelledByParent;
+        const testNumber = nesting === 0 ? (Number(node.id) + this.testNumber - 1) : node.id;
+        const method = pass ? 'ok' : 'fail';
+        this.reporter[method](nesting, this.name, testNumber, node.description, diagnostics, directive);
+        if (nesting === 0) {
+          this.parent.countSubtest({ finished: true, skipped: skip, isTodo: todo, passed: pass, cancelled });
+          this.countSubtest({ finished: true, skipped: skip, isTodo: todo, passed: pass, cancelled });
         }
         break;
 
+      }
       case TokenKind.COMMENT:
-        if (nesting === 1 && this.#checkNestedComment(node)) {
+        if (nesting === 0 && this.#checkNestedComment(node)) {
           // Ignore file top level diagnostics
           break;
         }
@@ -216,10 +213,15 @@ class FileTest extends Test {
     this.reportStarted();
     this.#handleReportItem(ast);
   }
+  reportStarted() {}
   report() {
-    this.reportStarted();
+    if (!this.skipReporting) {
+      super.reportStarted();
+    }
     ArrayPrototypeForEach(this.#buffer, (ast) => this.#handleReportItem(ast));
-    super.report();
+    if (!this.skipReporting) {
+      super.report();
+    }
   }
 }
 
@@ -274,16 +276,14 @@ function runTestFile(path, root, inspectPort, filesWatcher) {
       subtest.addToReport(ast);
     });
 
-    const { 0: { 0: code, 1: signal } } = await SafePromiseAll([
-      once(child, 'exit', { signal: t.signal }),
-      child.stdout.toArray({ signal: t.signal }),
-    ]);
+    const { 0: code, 1: signal } = await once(child, 'exit', { signal: t.signal });
 
     runningProcesses.delete(path);
     runningSubtests.delete(path);
     if (code !== 0 || signal !== null) {
       if (!err) {
-        err = ObjectAssign(new ERR_TEST_FAILURE('test failed', kSubtestsFailed), {
+        const failureType = subtest.completedCounters.totalFailed > 0 ? kSubtestsFailed : kTestCodeFailure;
+        err = ObjectAssign(new ERR_TEST_FAILURE('test failed', failureType), {
           __proto__: null,
           exitCode: code,
           signal: signal,

lib/internal/test_runner/test.js

@@ -286,6 +286,10 @@ class Test extends AsyncResource {
       beforeEach: [],
       afterEach: [],
     };
+    this.completedCounters = {
+      __proto__: null,
+      all: 0, failed: 0, passed: 0, cancelled: 0, skipped: 0, todo: 0, totalFailed: 0
+    };
     this.waitingOn = 0;
     this.finished = false;
   }
@@ -404,6 +408,7 @@ class Test extends AsyncResource {
         kCancelledByParent
       )
     );
+    this.error.failureType = kCancelledByParent;
     this.startTime = this.startTime || this.endTime; // If a test was canceled before it was started, e.g inside a hook
     this.cancelled = true;
     this.#abortController.abort();
@@ -577,9 +582,32 @@ class Test extends AsyncResource {
     this.postRun();
   }
 
-  postRun(pendingSubtestsError) {
-    const counters = { __proto__: null, failed: 0, passed: 0, cancelled: 0, skipped: 0, todo: 0, totalFailed: 0 };
+  countSubtest(subtest) {
+    if (subtest.skipReporting || subtest.counted) {
+      return;
+    }
+    subtest.counted = true;
+    // Check SKIP and TODO tests first, as those should not be counted as
+    // failures.
+    if (subtest.skipped) {
+      this.completedCounters.skipped++;
+    } else if (subtest.isTodo) {
+      this.completedCounters.todo++;
+    } else if (subtest.cancelled) {
+      this.completedCounters.cancelled++;
+    } else if (!subtest.passed) {
+      this.completedCounters.failed++;
+    } else {
+      this.completedCounters.passed++;
+    }
+
+    if (!subtest.passed) {
+      this.completedCounters.totalFailed++;
+    }
+    this.completedCounters.all++;
+  }
 
+  postRun(pendingSubtestsError) {
     // If the test was failed before it even started, then the end time will
     // be earlier than the start time. Correct that here.
     if (this.endTime < this.startTime) {
@@ -592,34 +620,15 @@ class Test extends AsyncResource {
     this.pendingSubtests = [];
     for (let i = 0; i < this.subtests.length; i++) {
       const subtest = this.subtests[i];
-
       if (!subtest.finished) {
         subtest.cancel(pendingSubtestsError);
         subtest.postRun(pendingSubtestsError);
       }
-
-      // Check SKIP and TODO tests first, as those should not be counted as
-      // failures.
-      if (subtest.skipped) {
-        counters.skipped++;
-      } else if (subtest.isTodo) {
-        counters.todo++;
-      } else if (subtest.cancelled) {
-        counters.cancelled++;
-      } else if (!subtest.passed) {
-        counters.failed++;
-      } else {
-        counters.passed++;
-      }
-
-      if (!subtest.passed) {
-        counters.totalFailed++;
-      }
     }
 
-    if ((this.passed || this.parent === null) && counters.totalFailed > 0) {
-      const subtestString = `subtest${counters.totalFailed > 1 ? 's' : ''}`;
-      const msg = `${counters.totalFailed} ${subtestString} failed`;
+    if ((this.passed || this.parent === null) && this.completedCounters.totalFailed > 0) {
+      const subtestString = `subtest${this.completedCounters.totalFailed > 1 ? 's' : ''}`;
+      const msg = `${this.completedCounters.totalFailed} ${subtestString} failed`;
 
       this.fail(new ERR_TEST_FAILURE(msg, kSubtestsFailed));
     }
@@ -632,20 +641,21 @@ class Test extends AsyncResource {
       this.parent.addReadySubtest(this);
       this.parent.processReadySubtestRange(false);
       this.parent.processPendingSubtests();
+      this.parent.countSubtest(this);
     } else if (!this.reported) {
       this.reported = true;
-      this.reporter.plan(this.nesting, kFilename, this.subtests.length);
+      this.reporter.plan(this.nesting, kFilename, this.completedCounters.all);
 
       for (let i = 0; i < this.diagnostics.length; i++) {
         this.reporter.diagnostic(this.nesting, kFilename, this.diagnostics[i]);
       }
 
-      this.reporter.diagnostic(this.nesting, kFilename, `tests ${this.subtests.length}`);
-      this.reporter.diagnostic(this.nesting, kFilename, `pass ${counters.passed}`);
-      this.reporter.diagnostic(this.nesting, kFilename, `fail ${counters.failed}`);
-      this.reporter.diagnostic(this.nesting, kFilename, `cancelled ${counters.cancelled}`);
-      this.reporter.diagnostic(this.nesting, kFilename, `skipped ${counters.skipped}`);
-      this.reporter.diagnostic(this.nesting, kFilename, `todo ${counters.todo}`);
+      this.reporter.diagnostic(this.nesting, kFilename, `tests ${this.completedCounters.all}`);
+      this.reporter.diagnostic(this.nesting, kFilename, `pass ${this.completedCounters.passed}`);
+      this.reporter.diagnostic(this.nesting, kFilename, `fail ${this.completedCounters.failed}`);
+      this.reporter.diagnostic(this.nesting, kFilename, `cancelled ${this.completedCounters.cancelled}`);
+      this.reporter.diagnostic(this.nesting, kFilename, `skipped ${this.completedCounters.skipped}`);
+      this.reporter.diagnostic(this.nesting, kFilename, `todo ${this.completedCounters.todo}`);
       this.reporter.diagnostic(this.nesting, kFilename, `duration_ms ${this.#duration()}`);
 
       if (this.coverage) {

test/message/test_runner_abort.out

@@ -40,6 +40,7 @@ TAP version 13
     not ok 7 - not ok 3
       ---
       duration_ms: *
+      failureType: 'cancelledByParent'
       error: 'This operation was aborted'
       code: 20
       stack: |-
@@ -58,6 +59,7 @@ TAP version 13
     not ok 8 - not ok 4
       ---
       duration_ms: *
+      failureType: 'cancelledByParent'
       error: 'This operation was aborted'
       code: 20
       stack: |-
@@ -76,6 +78,7 @@ TAP version 13
     not ok 9 - not ok 5
       ---
       duration_ms: *
+      failureType: 'cancelledByParent'
       error: 'This operation was aborted'
       code: 20
       stack: |-
@@ -94,6 +97,7 @@ TAP version 13
 not ok 1 - promise timeout signal
   ---
   duration_ms: *
+  failureType: 'cancelledByParent'
   error: 'The operation was aborted due to timeout'
   code: 23
   stack: |-
@@ -106,6 +110,7 @@ not ok 1 - promise timeout signal
 not ok 2 - promise abort signal
   ---
   duration_ms: *
+  failureType: 'cancelledByParent'
   error: 'This operation was aborted'
   code: 20
   stack: |-
@@ -160,6 +165,7 @@ not ok 2 - promise abort signal
     not ok 7 - not ok 3
       ---
       duration_ms: *
+      failureType: 'cancelledByParent'
       error: 'This operation was aborted'
       code: 20
       stack: |-
@@ -178,6 +184,7 @@ not ok 2 - promise abort signal
     not ok 8 - not ok 4
       ---
       duration_ms: *
+      failureType: 'cancelledByParent'
       error: 'This operation was aborted'
       code: 20
       stack: |-
@@ -196,6 +203,7 @@ not ok 2 - promise abort signal
     not ok 9 - not ok 5
       ---
       duration_ms: *
+      failureType: 'cancelledByParent'
       error: 'This operation was aborted'
       code: 20
       stack: |-
@@ -214,6 +222,7 @@ not ok 2 - promise abort signal
 not ok 3 - callback timeout signal
   ---
   duration_ms: *
+  failureType: 'cancelledByParent'
   error: 'The operation was aborted due to timeout'
   code: 23
   stack: |-
@@ -226,6 +235,7 @@ not ok 3 - callback timeout signal
 not ok 4 - callback abort signal
   ---
   duration_ms: *
+  failureType: 'cancelledByParent'
   error: 'This operation was aborted'
   code: 20
   stack: |-

test/message/test_runner_abort_suite.out

@@ -64,6 +64,7 @@ TAP version 13
 not ok 1 - describe timeout signal
   ---
   duration_ms: *
+  failureType: 'cancelledByParent'
   error: 'The operation was aborted due to timeout'
   code: 23
   stack: |-
@@ -76,6 +77,7 @@ not ok 1 - describe timeout signal
 not ok 2 - describe abort signal
   ---
   duration_ms: *
+  failureType: 'cancelledByParent'
   error: 'This operation was aborted'
   code: 20
   stack: |-

test/message/test_runner_describe_it.out

@@ -548,7 +548,7 @@ not ok 55 - describe async throw fails
     not ok 1 - timed out async test
       ---
       duration_ms: *
-      failureType: 'testTimeoutFailure'
+      failureType: 'cancelledByParent'
       error: 'test timed out after 5ms'
       code: 'ERR_TEST_FAILURE'
       stack: |-
@@ -558,7 +558,7 @@ not ok 55 - describe async throw fails
     not ok 2 - timed out callback test
       ---
       duration_ms: *
-      failureType: 'testTimeoutFailure'
+      failureType: 'cancelledByParent'
       error: 'test timed out after 5ms'
       code: 'ERR_TEST_FAILURE'
       ...

test/message/test_runner_output.out

@@ -561,15 +561,15 @@ not ok 56 - subtest sync throw fails
 not ok 57 - timed out async test
   ---
   duration_ms: *
-  failureType: 'testTimeoutFailure'
+  failureType: 'cancelledByParent'
   error: 'test timed out after 5ms'
   code: 'ERR_TEST_FAILURE'
   ...
 # Subtest: timed out callback test
 not ok 58 - timed out callback test
   ---
   duration_ms: *
-  failureType: 'testTimeoutFailure'
+  failureType: 'cancelledByParent'
   error: 'test timed out after 5ms'
   code: 'ERR_TEST_FAILURE'
   ...