test_runner: fix spec skip detection

PR-URL: https://github.com/nodejs/node/pull/47537
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>

lib/internal/test_runner/runner.js

@@ -190,9 +190,9 @@ class FileTest extends Test {
         let directive;
 
         if (skip) {
-          directive = this.reporter.getSkip(node.reason);
+          directive = this.reporter.getSkip(node.reason || true);
         } else if (todo) {
-          directive = this.reporter.getTodo(node.reason);
+          directive = this.reporter.getTodo(node.reason || true);
         } else {
           directive = kEmptyObject;
         }

test/fixtures/test-runner/output/spec_reporter.snapshot

@@ -150,7 +150,7 @@
  test with a name and options provided (*ms) # SKIP
  functionAndOptions (*ms) # SKIP
  escaped description \ # \#\ 
- 	    
 (*ms)
+ 	     (*ms)
  escaped skip message (*ms) # SKIP
  escaped todo message (*ms)
  escaped diagnostic (*ms)

test/fixtures/test-runner/output/spec_reporter_cli.js

@@ -0,0 +1,12 @@
+// Flags: --no-warnings
+'use strict';
+require('../../../common');
+const fixtures = require('../../../common/fixtures');
+const spawn = require('node:child_process').spawn;
+
+const child = spawn(process.execPath,
+                    ['--no-warnings', '--test', '--test-reporter', 'spec', fixtures.path('test-runner/output/output.js')],
+                    { stdio: 'pipe' });
+// eslint-disable-next-line no-control-regex
+child.stdout.on('data', (d) => process.stdout.write(d.toString().replace(/[^\x00-\x7F]/g, '').replace(/\u001b\[\d+m/g, '')));
+child.stderr.pipe(process.stderr);

test/fixtures/test-runner/output/spec_reporter_cli.snapshot

@@ -0,0 +1,559 @@
+ sync pass todo (*ms)
+ sync pass todo with message (*ms)
+ sync fail todo (*ms)
+  Error: thrown from sync fail todo
+      *
+      *
+      *
+      *
+      *
+      *
+      *
+
+ sync fail todo with message (*ms)
+  Error: thrown from sync fail todo with message
+      *
+      *
+      *
+      *
+      *
+      *
+      *
+
+ sync skip pass (*ms) # SKIP
+ sync skip pass with message (*ms) # SKIP
+ sync pass (*ms)
+ this test should pass
+ sync throw fail (*ms)
+  Error: thrown from sync throw fail
+      *
+      *
+      *
+      *
+      *
+      *
+      *
+
+ async skip pass (*ms) # SKIP
+ async pass (*ms)
+ async throw fail (*ms)
+  Error: thrown from async throw fail
+      *
+      *
+      *
+      *
+      *
+      *
+      *
+
+ async skip fail (*ms) # SKIP
+  Error: thrown from async throw fail
+      *
+      *
+      *
+      *
+      *
+      *
+      *
+
+ async assertion fail (*ms)
+  AssertionError: Expected values to be strictly equal:
+  
+  true !== false
+  
+      *
+      *
+      *
+      *
+      *
+      *
+      * {
+    generatedMessage: false,
+    code: 'ERR_ASSERTION',
+    actual: true,
+    expected: false,
+    operator: 'strictEqual'
+  }
+
+ resolve pass (*ms)
+ reject fail (*ms)
+  Error: rejected from reject fail
+      *
+      *
+      *
+      *
+      *
+      *
+      *
+
+ unhandled rejection - passes but warns (*ms)
+ async unhandled rejection - passes but warns (*ms)
+ immediate throw - passes but warns (*ms)
+ immediate reject - passes but warns (*ms)
+ immediate resolve pass (*ms)
+ subtest sync throw fail
+   +sync throw fail (*ms)
+    Error: thrown from subtest sync throw fail
+        *
+        *
+        *
+        *
+        *
+        *
+        *
+        *
+        *
+        *
+
+   this subtest should make its parent test fail
+ subtest sync throw fail (*ms)
+
+ sync throw non-error fail (*ms)
+  [Error: Symbol(thrown symbol from sync throw non-error fail)
+  ]
+
+ level 0a
+   level 1a (*ms)
+   level 1b (*ms)
+   level 1c (*ms)
+   level 1d (*ms)
+ level 0a (*ms)
+
+ top level
+   +long running (*ms)
+    [Error: test did not finish before its parent and was cancelled
+    ]
+
+   +short running
+     ++short running (*ms)
+   +short running (*ms)
+
+ top level (*ms)
+
+ invalid subtest - pass but subtest fails (*ms)
+ sync skip option (*ms) # SKIP
+ sync skip option with message (*ms) # SKIP
+ sync skip option is false fail (*ms)
+  Error: this should be executed
+      *
+      *
+      *
+      *
+      *
+      *
+      *
+
+ <anonymous> (*ms)
+ functionOnly (*ms)
+ <anonymous> (*ms)
+ test with only a name provided (*ms)
+ <anonymous> (*ms)
+ <anonymous> (*ms) # SKIP
+ test with a name and options provided (*ms) # SKIP
+ functionAndOptions (*ms) # SKIP
+ escaped description \ # \#\ \n \t \f \v \b \r (*ms)
+ escaped skip message (*ms) # SKIP
+ escaped todo message (*ms)
+ escaped diagnostic (*ms)
+ #diagnostic
+ callback pass (*ms)
+ callback fail (*ms)
+  Error: callback failure
+      *
+      *
+
+ sync t is this in test (*ms)
+ async t is this in test (*ms)
+ callback t is this in test (*ms)
+ callback also returns a Promise (*ms)
+  [Error: passed a callback but also returned a Promise
+  ]
+
+ callback throw (*ms)
+  Error: thrown from callback throw
+      *
+      *
+      *
+      *
+      *
+      *
+      *
+
+ callback called twice (*ms)
+  Error: callback invoked multiple times
+      *
+      *
+
+ callback called twice in different ticks (*ms)
+ callback called twice in future tick (*ms)
+  Error: callback invoked multiple times
+      *
+
+ callback async throw (*ms)
+  Error: thrown from callback async throw
+      *
+      *
+
+ callback async throw after done (*ms)
+ only is set but not in only mode
+   running subtest 1 (*ms)
+   running subtest 2 (*ms)
+   'only' and 'runOnly' require the --test-only command-line option.
+   running subtest 3 (*ms)
+   'only' and 'runOnly' require the --test-only command-line option.
+   running subtest 4 (*ms)
+ only is set but not in only mode (*ms)
+
+ 'only' and 'runOnly' require the --test-only command-line option.
+ custom inspect symbol fail (*ms)
+  [Error: customized
+  ]
+
+ custom inspect symbol that throws fail (*ms)
+  [Error: {
+    foo: 1,
+    [Symbol(nodejs.util.inspect.custom)]: [Function: [nodejs.util.inspect.custom]]
+  }
+  ]
+
+ subtest sync throw fails
+   sync throw fails at first (*ms)
+    Error: thrown from subtest sync throw fails at first
+        *
+        *
+        *
+        *
+        *
+        *
+        *
+        *
+        *
+        *
+
+   sync throw fails at second (*ms)
+    Error: thrown from subtest sync throw fails at second
+        *
+        *
+        *
+        *
+        *
+        *
+        *
+        *
+        *
+        *
+
+ subtest sync throw fails (*ms)
+
+ timed out async test (*ms)
+  [Error: test timed out after *ms
+  ]
+
+ timed out callback test (*ms)
+  [Error: test timed out after *ms
+  ]
+
+ large timeout async test is ok (*ms)
+ large timeout callback test is ok (*ms)
+ successful thenable (*ms)
+ rejected thenable (*ms)
+  [Error: custom error
+  ]
+
+ unfinished test with uncaughtException (*ms)
+  Error: foo
+      *
+      *
+      *
+
+ unfinished test with unhandledRejection (*ms)
+  Error: bar
+      *
+      *
+      *
+
+ assertion errors display actual and expected properly (*ms)
+  AssertionError: Expected values to be loosely deep-equal:
+  
+  {
+    bar: 1,
+    foo: 1
+  }
+  
+  should loosely deep-equal
+  
+  <ref *1> {
+    bar: 2,
+    c: [Circular *1]
+  }
+      * {
+    generatedMessage: false,
+    code: 'ERR_ASSERTION',
+    actual: [Object],
+    expected: [Object],
+    operator: 'deepEqual'
+  }
+
+ invalid subtest fail (*ms)
+  Error: test could not be started because its parent finished
+      *
+
+ Warning: Test "unhandled rejection - passes but warns" generated asynchronous activity after the test ended. This activity created the error "Error: rejected from unhandled rejection fail" and would have caused the test to fail, but instead triggered an unhandledRejection event.
+ Warning: Test "async unhandled rejection - passes but warns" generated asynchronous activity after the test ended. This activity created the error "Error: rejected from async unhandled rejection fail" and would have caused the test to fail, but instead triggered an unhandledRejection event.
+ Warning: A resource generated asynchronous activity after the test ended. This activity created the error "Error: uncaught from outside of a test" which triggered an uncaughtException event, caught by the test runner.
+ Warning: Test "immediate throw - passes but warns" generated asynchronous activity after the test ended. This activity created the error "Error: thrown from immediate throw fail" and would have caused the test to fail, but instead triggered an uncaughtException event.
+ Warning: Test "immediate reject - passes but warns" generated asynchronous activity after the test ended. This activity created the error "Error: rejected from immediate reject fail" and would have caused the test to fail, but instead triggered an unhandledRejection event.
+ Warning: Test "callback called twice in different ticks" generated asynchronous activity after the test ended. This activity created the error "Error [ERR_TEST_FAILURE]: callback invoked multiple times" and would have caused the test to fail, but instead triggered an uncaughtException event.
+ Warning: Test "callback async throw after done" generated asynchronous activity after the test ended. This activity created the error "Error: thrown from callback async throw after done" and would have caused the test to fail, but instead triggered an uncaughtException event.
+ tests 80
+ suites 0
+ pass 37
+ fail 25
+ cancelled 3
+ skipped 10
+ todo 5
+ duration_ms *
+
+ failing tests:
+
+ sync fail todo (*ms)
+  Error: thrown from sync fail todo
+      *
+      *
+      *
+      *
+      *
+      *
+      *
+
+ sync fail todo with message (*ms)
+  Error: thrown from sync fail todo with message
+      *
+      *
+      *
+      *
+      *
+      *
+      *
+
+ sync throw fail (*ms)
+  Error: thrown from sync throw fail
+      *
+      *
+      *
+      *
+      *
+      *
+      *
+
+ async throw fail (*ms)
+  Error: thrown from async throw fail
+      *
+      *
+      *
+      *
+      *
+      *
+      *
+
+ async skip fail (*ms)
+  Error: thrown from async throw fail
+      *
+      *
+      *
+      *
+      *
+      *
+      *
+
+ async assertion fail (*ms)
+  AssertionError: Expected values to be strictly equal:
+  
+  true !== false
+  
+      *
+      *
+      *
+      *
+      *
+      *
+      * {
+    generatedMessage: false,
+    code: 'ERR_ASSERTION',
+    actual: true,
+    expected: false,
+    operator: 'strictEqual'
+  }
+
+ reject fail (*ms)
+  Error: rejected from reject fail
+      *
+      *
+      *
+      *
+      *
+      *
+      *
+
+ +sync throw fail (*ms)
+  Error: thrown from subtest sync throw fail
+      *
+      *
+      *
+      *
+      *
+      *
+      *
+      *
+      *
+      *
+
+ subtest sync throw fail (*ms)
+  [Error: 1 subtest failed
+  ]
+
+ sync throw non-error fail (*ms)
+  [Error: Symbol(thrown symbol from sync throw non-error fail)
+  ]
+
+ +long running (*ms)
+  [Error: test did not finish before its parent and was cancelled
+  ]
+
+ top level (*ms)
+  [Error: 1 subtest failed
+  ]
+
+ sync skip option is false fail (*ms)
+  Error: this should be executed
+      *
+      *
+      *
+      *
+      *
+      *
+      *
+
+ callback fail (*ms)
+  Error: callback failure
+      *
+      *
+
+ callback also returns a Promise (*ms)
+  [Error: passed a callback but also returned a Promise
+  ]
+
+ callback throw (*ms)
+  Error: thrown from callback throw
+      *
+      *
+      *
+      *
+      *
+      *
+      *
+
+ callback called twice (*ms)
+  Error: callback invoked multiple times
+      *
+      *
+
+ callback called twice in future tick (*ms)
+  Error: callback invoked multiple times
+      *
+
+ callback async throw (*ms)
+  Error: thrown from callback async throw
+      *
+      *
+
+ custom inspect symbol fail (*ms)
+  [Error: customized
+  ]
+
+ custom inspect symbol that throws fail (*ms)
+  [Error: {
+    foo: 1,
+    [Symbol(nodejs.util.inspect.custom)]: [Function: [nodejs.util.inspect.custom]]
+  }
+  ]
+
+ sync throw fails at first (*ms)
+  Error: thrown from subtest sync throw fails at first
+      *
+      *
+      *
+      *
+      *
+      *
+      *
+      *
+      *
+      *
+
+ sync throw fails at second (*ms)
+  Error: thrown from subtest sync throw fails at second
+      *
+      *
+      *
+      *
+      *
+      *
+      *
+      *
+      *
+      *
+
+ subtest sync throw fails (*ms)
+  [Error: 2 subtests failed
+  ]
+
+ timed out async test (*ms)
+  [Error: test timed out after *ms
+  ]
+
+ timed out callback test (*ms)
+  [Error: test timed out after *ms
+  ]
+
+ rejected thenable (*ms)
+  [Error: custom error
+  ]
+
+ unfinished test with uncaughtException (*ms)
+  Error: foo
+      *
+      *
+      *
+
+ unfinished test with unhandledRejection (*ms)
+  Error: bar
+      *
+      *
+      *
+
+ assertion errors display actual and expected properly (*ms)
+  AssertionError: Expected values to be loosely deep-equal:
+  
+  {
+    bar: 1,
+    foo: 1
+  }
+  
+  should loosely deep-equal
+  
+  <ref *1> {
+    bar: 2,
+    c: [Circular *1]
+  }
+      * {
+    generatedMessage: false,
+    code: 'ERR_ASSERTION',
+    actual: [Object],
+    expected: [Object],
+    operator: 'deepEqual'
+  }
+
+ invalid subtest fail (*ms)
+  Error: test could not be started because its parent finished
+      *

test/parallel/test-runner-output.mjs

@@ -18,6 +18,8 @@ function replaceSpecDuration(str) {
 }
 const defaultTransform = snapshot
   .transform(snapshot.replaceWindowsLineEndings, snapshot.replaceStackTrace, replaceTestDuration);
+const specTransform = snapshot
+  .transform(snapshot.replaceWindowsLineEndings, snapshot.replaceStackTrace, replaceSpecDuration);
 
 
 const tests = [
@@ -30,14 +32,9 @@ const tests = [
   { name: 'test-runner/output/no_tests.js' },
   { name: 'test-runner/output/only_tests.js' },
   { name: 'test-runner/output/dot_reporter.js' },
-  {
-    name: 'test-runner/output/spec_reporter_successful.js',
-    transform: snapshot.transform(snapshot.replaceWindowsLineEndings, snapshot.replaceStackTrace, replaceSpecDuration)
-  },
-  {
-    name: 'test-runner/output/spec_reporter.js',
-    transform: snapshot.transform(snapshot.replaceWindowsLineEndings, snapshot.replaceStackTrace, replaceSpecDuration)
-  },
+  { name: 'test-runner/output/spec_reporter_successful.js', transform: specTransform },
+  { name: 'test-runner/output/spec_reporter.js', transform: specTransform },
+  { name: 'test-runner/output/spec_reporter_cli.js', transform: specTransform },
   { name: 'test-runner/output/output.js' },
   { name: 'test-runner/output/output_cli.js' },
   { name: 'test-runner/output/name_pattern.js' },