test_runner: handle undefined test locations

cjihrig · marco-ippolito · commit e64a25af61d5 · 2024-05-03T08:29:12.000+02:00
This commit updates the built in reporters to check for the documented case of a test's location being undefined. As a drive by fix, the C++ code for computing the test location now returns undefined if the script location is empty. This lets tests run inside of eval(). PR-URL: #52036 Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Moshe Atlow <moshe@atlow.co.il> Reviewed-By: Chemi Atlow <chemi@atlow.co.il>
diff --git a/lib/internal/test_runner/reporter/spec.js b/lib/internal/test_runner/reporter/spec.js
@@ -148,11 +148,16 @@ class SpecReporter extends Transform {
     const results = [`\n${this.#colors['test:fail']}${symbols['test:fail']}failing tests:${colors.white}\n`];
     for (let i = 0; i < this.#failedTests.length; i++) {
       const test = this.#failedTests[i];
-      const relPath = relative(this.#cwd, test.file);
       const formattedErr = this.#formatTestReport('test:fail', test);
-      const location = `test at ${relPath}:${test.line}:${test.column}`;
 
-      ArrayPrototypePush(results, location, formattedErr);
+      if (test.file) {
+        const relPath = relative(this.#cwd, test.file);
+        const location = `test at ${relPath}:${test.line}:${test.column}`;
+
+        ArrayPrototypePush(results, location);
+      }
+
+      ArrayPrototypePush(results, formattedErr);
     }
     callback(null, ArrayPrototypeJoin(results, '\n'));
   }
diff --git a/lib/internal/test_runner/reporter/tap.js b/lib/internal/test_runner/reporter/tap.js
@@ -34,7 +34,7 @@ async function * tapReporter(source) {
     switch (type) {
       case 'test:fail': {
         yield reportTest(data.nesting, data.testNumber, 'not ok', data.name, data.skip, data.todo);
-        const location = `${data.file}:${data.line}:${data.column}`;
+        const location = data.file ? `${data.file}:${data.line}:${data.column}` : null;
         yield reportDetails(data.nesting, data.details, location);
         break;
       } case 'test:pass':
diff --git a/src/node_util.cc b/src/node_util.cc
@@ -148,9 +148,15 @@ static void GetCallerLocation(const FunctionCallbackInfo<Value>& args) {
   }
 
   Local<StackFrame> frame = trace->GetFrame(isolate, 1);
+  Local<Value> file = frame->GetScriptNameOrSourceURL();
+
+  if (file.IsEmpty()) {
+    return;
+  }
+
   Local<Value> ret[] = {Integer::New(isolate, frame->GetLineNumber()),
                         Integer::New(isolate, frame->GetColumn()),
-                        frame->GetScriptNameOrSourceURL()};
+                        file};
 
   args.GetReturnValue().Set(Array::New(args.GetIsolate(), ret, arraysize(ret)));
 }
diff --git a/test/fixtures/test-runner/output/eval_dot.js b/test/fixtures/test-runner/output/eval_dot.js
@@ -0,0 +1,10 @@
+// Flags: --test-reporter=dot
+'use strict';
+eval(`
+  const { test } = require('node:test');
+
+  test('passes');
+  test('fails', () => {
+    throw new Error('fail');
+  });
+`);
diff --git a/test/fixtures/test-runner/output/eval_dot.snapshot b/test/fixtures/test-runner/output/eval_dot.snapshot
@@ -0,0 +1 @@
+.X
diff --git a/test/fixtures/test-runner/output/eval_spec.js b/test/fixtures/test-runner/output/eval_spec.js
@@ -0,0 +1,10 @@
+// Flags: --test-reporter=spec
+'use strict';
+eval(`
+  const { test } = require('node:test');
+
+  test('passes');
+  test('fails', () => {
+    throw new Error('fail');
+  });
+`);
diff --git a/test/fixtures/test-runner/output/eval_spec.snapshot b/test/fixtures/test-runner/output/eval_spec.snapshot
@@ -0,0 +1,31 @@
+✔ passes (*ms)
+✖ fails (*ms)
+  Error: fail
+      *
+      *
+      *
+      *
+      *
+      *
+      *
+
+ℹ tests 2
+ℹ suites 0
+ℹ pass 1
+ℹ fail 1
+ℹ cancelled 0
+ℹ skipped 0
+ℹ todo 0
+ℹ duration_ms *
+
+✖ failing tests:
+
+✖ fails (*ms)
+  Error: fail
+      *
+      *
+      *
+      *
+      *
+      *
+      *
diff --git a/test/fixtures/test-runner/output/eval_tap.js b/test/fixtures/test-runner/output/eval_tap.js
@@ -0,0 +1,10 @@
+// Flags: --test-reporter=tap
+'use strict';
+eval(`
+  const { test } = require('node:test');
+
+  test('passes');
+  test('fails', () => {
+    throw new Error('fail');
+  });
+`);
diff --git a/test/fixtures/test-runner/output/eval_tap.snapshot b/test/fixtures/test-runner/output/eval_tap.snapshot
@@ -0,0 +1,31 @@
+TAP version 13
+# Subtest: passes
+ok 1 - passes
+  ---
+  duration_ms: *
+  ...
+# Subtest: fails
+not ok 2 - fails
+  ---
+  duration_ms: *
+  failureType: 'testCodeFailure'
+  error: 'fail'
+  code: 'ERR_TEST_FAILURE'
+  stack: |-
+    *
+    *
+    *
+    *
+    *
+    *
+    *
+  ...
+1..2
+# tests 2
+# suites 0
+# pass 1
+# fail 1
+# cancelled 0
+# skipped 0
+# todo 0
+# duration_ms *
diff --git a/test/parallel/test-runner-output.mjs b/test/parallel/test-runner-output.mjs
@@ -91,6 +91,9 @@ const tests = [
   { name: 'test-runner/output/abort_hooks.js' },
   { name: 'test-runner/output/describe_it.js' },
   { name: 'test-runner/output/describe_nested.js' },
+  { name: 'test-runner/output/eval_dot.js' },
+  { name: 'test-runner/output/eval_spec.js', transform: specTransform },
+  { name: 'test-runner/output/eval_tap.js' },
   { name: 'test-runner/output/hooks.js' },
   { name: 'test-runner/output/hooks_spec_reporter.js', transform: specTransform },
   { name: 'test-runner/output/timeout_in_before_each_should_not_affect_further_tests.js' },
