CR

Author: MoLow

lib/internal/test_runner/runner.js

@@ -10,6 +10,7 @@ const {
   ArrayPrototypeSort,
   Number,
   ObjectAssign,
+  ObjectKeys,
   PromisePrototypeThen,
   SafePromiseAllReturnVoid,
   SafePromiseAllSettledReturnVoid,
@@ -133,9 +134,10 @@ function getRunArgs({ path, inspectPort }) {
 
 class FileTest extends Test {
   #buffer = [];
-  #hasSubtests = false;
-  get skipReporting() {
-    return this.#hasSubtests && (!this.error || this.error.failureType === kSubtestsFailed);
+  #counters = { __proto__: null, all: 0, failed: 0, passed: 0, cancelled: 0, skipped: 0, todo: 0, totalFailed: 0 };
+  failedSubtests = false;
+  #skipReporting() {
+    return this.#counters.all > 0 && (!this.error || this.error.failureType === kSubtestsFailed);
   }
   #checkNestedComment({ comment }) {
     const firstSpaceIndex = StringPrototypeIndexOf(comment, ' ');
@@ -155,7 +157,7 @@ class FileTest extends Test {
         break;
 
       case TokenKind.TAP_PLAN:
-        if (nesting === 0 && this.skipReporting) {
+        if (nesting === 0 && this.#skipReporting()) {
           break;
         }
         this.reporter.plan(nesting, this.name, node.end - node.start + 1);
@@ -179,15 +181,15 @@ class FileTest extends Test {
           directive = kEmptyObject;
         }
 
-        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 });
+          super.countSubtest
+            .call({ finished: true, skipped: skip, isTodo: todo, passed: pass, cancelled }, this.#counters);
+          this.failedSubtests ||= !pass;
         }
         break;
 
@@ -213,13 +215,22 @@ class FileTest extends Test {
     this.reportStarted();
     this.#handleReportItem(ast);
   }
+  countSubtest(counters) {
+    if (this.#counters.all === 0) {
+      return super.countSubtest(counters);
+    }
+    ArrayPrototypeForEach(ObjectKeys(counters), (key) => {
+      counters[key] += this.#counters[key];
+    });
+  }
   reportStarted() {}
   report() {
-    if (!this.skipReporting) {
+    const skipReporting = this.#skipReporting();
+    if (!skipReporting) {
       super.reportStarted();
     }
     ArrayPrototypeForEach(this.#buffer, (ast) => this.#handleReportItem(ast));
-    if (!this.skipReporting) {
+    if (!skipReporting) {
       super.report();
     }
   }
@@ -282,7 +293,7 @@ function runTestFile(path, root, inspectPort, filesWatcher) {
     runningSubtests.delete(path);
     if (code !== 0 || signal !== null) {
       if (!err) {
-        const failureType = subtest.completedCounters.totalFailed > 0 ? kSubtestsFailed : kTestCodeFailure;
+        const failureType = subtest.failedSubtests ? kSubtestsFailed : kTestCodeFailure;
         err = ObjectAssign(new ERR_TEST_FAILURE('test failed', failureType), {
           __proto__: null,
           exitCode: code,

lib/internal/test_runner/test.js

@@ -286,10 +286,6 @@ 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;
   }
@@ -582,32 +578,31 @@ class Test extends AsyncResource {
     this.postRun();
   }
 
-  countSubtest(subtest) {
-    if (subtest.skipReporting || subtest.counted) {
-      return;
-    }
-    subtest.counted = true;
+  countSubtest(counters) {
     // 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++;
+    if (this.skipped) {
+      counters.skipped++;
+    } else if (this.isTodo) {
+      counters.todo++;
+    } else if (this.cancelled) {
+      counters.cancelled++;
+    } else if (!this.passed) {
+      counters.failed++;
     } else {
-      this.completedCounters.passed++;
+      counters.passed++;
     }
 
-    if (!subtest.passed) {
-      this.completedCounters.totalFailed++;
+    if (!this.passed) {
+      counters.totalFailed++;
     }
-    this.completedCounters.all++;
+    counters.all++;
   }
 
   postRun(pendingSubtestsError) {
+    const counters = {
+      __proto__: null, all: 0, failed: 0, passed: 0, cancelled: 0, skipped: 0, todo: 0, totalFailed: 0
+    };
     // 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) {
@@ -620,15 +615,17 @@ 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);
       }
+      subtest.countSubtest(counters);
     }
 
-    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`;
+    if ((this.passed || this.parent === null) && counters.totalFailed > 0) {
+      const subtestString = `subtest${counters.totalFailed > 1 ? 's' : ''}`;
+      const msg = `${counters.totalFailed} ${subtestString} failed`;
 
       this.fail(new ERR_TEST_FAILURE(msg, kSubtestsFailed));
     }
@@ -641,21 +638,20 @@ 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.completedCounters.all);
+      this.reporter.plan(this.nesting, kFilename, counters.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.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, `tests ${counters.all}`);
+      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, `duration_ms ${this.#duration()}`);
 
       if (this.coverage) {

test/parallel/test-runner-extraneous-async-activity.js

@@ -11,8 +11,8 @@ const { spawnSync } = require('child_process');
   ]);
   const stdout = child.stdout.toString();
   assert.match(stdout, /^# Warning: Test "extraneous async activity test" generated asynchronous activity after the test ended/m);
-  assert.match(stdout, /^# pass 1$/m);
-  assert.match(stdout, /^# fail 1$/m);
+  assert.match(stdout, /^# pass 1/m);
+  assert.match(stdout, /^# fail 0$/m);
   assert.match(stdout, /^# cancelled 0$/m);
   assert.strictEqual(child.status, 1);
   assert.strictEqual(child.signal, null);
@@ -26,7 +26,7 @@ const { spawnSync } = require('child_process');
   const stdout = child.stdout.toString();
   assert.match(stdout, /^# Warning: Test "extraneous async activity test" generated asynchronous activity after the test ended/m);
   assert.match(stdout, /^# pass 1$/m);
-  assert.match(stdout, /^# fail 1$/m);
+  assert.match(stdout, /^# fail 0$/m);
   assert.match(stdout, /^# cancelled 0$/m);
   assert.strictEqual(child.status, 1);
   assert.strictEqual(child.signal, null);