test_runner: fix global after not failing the tests

PR-URL: #48913 Backport-PR-URL: #49225 Fixes: #48867 Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> Reviewed-By: Moshe Atlow <moshe@atlow.co.il> Reviewed-By: Chemi Atlow <chemi@atlow.co.il>
nodejs · Nov 27, 2023 · 71f7ed7 · 71f7ed7
1 parent 0fea550
commit 71f7ed7
Show file tree

Hide file tree

Showing 6 changed files with 103 additions and 34 deletions.
diff --git a/lib/internal/test_runner/test.js b/lib/internal/test_runner/test.js
@@ -688,6 +688,14 @@ class Test extends AsyncResource {
       this.parent.processReadySubtestRange(false);
       this.parent.processPendingSubtests();
     } else if (!this.reported) {
+      if (!this.passed && failed === 0 && this.error) {
+        this.reporter.fail(0, kFilename, this.subtests.length + 1, kFilename, {
+          __proto__: null,
+          duration_ms: this.#duration(),
+          error: this.error,
+        }, undefined);
+      }
+
       this.reported = true;
       this.reporter.plan(this.nesting, kFilename, this.root.harness.counters.topLevel);
 

diff --git a/test/common/assertSnapshot.js b/test/common/assertSnapshot.js
@@ -24,6 +24,10 @@ function replaceWindowsPaths(str) {
   return common.isWindows ? str.replaceAll(path.win32.sep, path.posix.sep) : str;
 }
 
+function replaceFullPaths(str) {
+  return str.replaceAll(process.cwd(), '');
+}
+
 function transform(...args) {
   return (str) => args.reduce((acc, fn) => fn(acc), str);
 }
@@ -73,6 +77,7 @@ async function spawnAndAssert(filename, transform = (x) => x, { tty = false, ...
 module.exports = {
   assertSnapshot,
   getSnapshotPath,
+  replaceFullPaths,
   replaceNodeVersion,
   replaceStackTrace,
   replaceWindowsLineEndings,

diff --git a/test/fixtures/test-runner/output/global_after_should_fail_the_test.js b/test/fixtures/test-runner/output/global_after_should_fail_the_test.js
@@ -0,0 +1,10 @@
+'use strict';
+const { it, after } = require('node:test');
+
+after(() => {
+  throw new Error('this should fail the test')
+});
+
+it('this is a test', () => {
+  console.log('this is a test')
+});
diff --git a/test/fixtures/test-runner/output/global_after_should_fail_the_test.snapshot b/test/fixtures/test-runner/output/global_after_should_fail_the_test.snapshot
@@ -0,0 +1,34 @@
+this is a test
+TAP version 13
+# Subtest: this is a test
+ok 1 - this is a test
+  ---
+  duration_ms: *
+  ...
+not ok 2 - /test/fixtures/test-runner/output/global_after_should_fail_the_test.js
+  ---
+  duration_ms: *
+  failureType: 'hookFailed'
+  error: 'this should fail the test'
+  code: 'ERR_TEST_FAILURE'
+  stack: |-
+    *
+    *
+    *
+    *
+    *
+    *
+    *
+    *
+    *
+    *
+  ...
+1..1
+# tests 1
+# suites 0
+# pass 1
+# fail 0
+# cancelled 0
+# skipped 0
+# todo 0
+# duration_ms *
diff --git a/test/fixtures/test-runner/output/hooks-with-no-global-test.js b/test/fixtures/test-runner/output/hooks-with-no-global-test.js
@@ -9,42 +9,36 @@ before(() => testArr.push('global before'));
 after(() => {
   testArr.push('global after');
 
-  try {
-    assert.deepStrictEqual(testArr, [
-      'global before',
-      'describe before',
+  assert.deepStrictEqual(testArr, [
+    'global before',
+    'describe before',
 
-      'describe beforeEach',
-      'describe it 1',
-      'describe afterEach',
+    'describe beforeEach',
+    'describe it 1',
+    'describe afterEach',
 
-      'describe beforeEach',
-      'describe test 2',
-      'describe afterEach',
+    'describe beforeEach',
+    'describe test 2',
+    'describe afterEach',
 
-      'describe nested before',
+    'describe nested before',
 
-      'describe beforeEach',
-      'describe nested beforeEach',
-      'describe nested it 1',
-      'describe afterEach',
-      'describe nested afterEach',
+    'describe beforeEach',
+    'describe nested beforeEach',
+    'describe nested it 1',
+    'describe afterEach',
+    'describe nested afterEach',
 
-      'describe beforeEach',
-      'describe nested beforeEach',
-      'describe nested test 2',
-      'describe afterEach',
-      'describe nested afterEach',
+    'describe beforeEach',
+    'describe nested beforeEach',
+    'describe nested test 2',
+    'describe afterEach',
+    'describe nested afterEach',
 
-      'describe nested after',
-      'describe after',
-      'global after',
-    ]);
-  } catch (e) {
-    // TODO(rluvaton): remove the try catch after #48867 is fixed
-    console.error(e);
-    process.exit(1);
-  }
+    'describe nested after',
+    'describe after',
+    'global after',
+  ]);
 });
 
 describe('describe hooks with no global tests', () => {

diff --git a/test/parallel/test-runner-output.mjs b/test/parallel/test-runner-output.mjs
@@ -24,10 +24,27 @@ function replaceSpecDuration(str) {
     .replaceAll(/duration_ms [0-9.]+/g, 'duration_ms *')
     .replace(stackTraceBasePath, '$3');
 }
-const defaultTransform = snapshot
-  .transform(snapshot.replaceWindowsLineEndings, snapshot.replaceStackTrace, replaceTestDuration);
-const specTransform = snapshot
-  .transform(replaceSpecDuration, snapshot.replaceWindowsLineEndings, snapshot.replaceStackTrace);
+
+function removeWindowsPathEscaping(str) {
+  return common.isWindows ? str.replaceAll(/\\\\/g, '\\') : str;
+}
+
+const defaultTransform = snapshot.transform(
+  snapshot.replaceWindowsLineEndings,
+  snapshot.replaceStackTrace,
+  replaceTestDuration,
+);
+const specTransform = snapshot.transform(
+  replaceSpecDuration,
+  snapshot.replaceWindowsLineEndings,
+  snapshot.replaceStackTrace,
+);
+const withFileNameTransform = snapshot.transform(
+  defaultTransform,
+  removeWindowsPathEscaping,
+  snapshot.replaceFullPaths,
+  snapshot.replaceWindowsPaths,
+);
 
 
 const tests = [
@@ -41,6 +58,7 @@ const tests = [
   { name: 'test-runner/output/hooks-with-no-global-test.js' },
   { name: 'test-runner/output/before-and-after-each-too-many-listeners.js' },
   { name: 'test-runner/output/before-and-after-each-with-timeout-too-many-listeners.js' },
+  { name: 'test-runner/output/global_after_should_fail_the_test.js', transform: withFileNameTransform },
   { name: 'test-runner/output/no_refs.js' },
   { name: 'test-runner/output/no_tests.js' },
   { name: 'test-runner/output/only_tests.js' },