From 06ba8fbe46658e04027e45a82a2e82a759650ef0 Mon Sep 17 00:00:00 2001 From: cjihrig Date: Sun, 10 Mar 2024 13:15:04 -0400 Subject: [PATCH] test_runner: handle undefined test locations 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(). --- lib/internal/test_runner/reporter/spec.js | 11 +++++-- lib/internal/test_runner/reporter/tap.js | 2 +- src/node_util.cc | 8 ++++- test/fixtures/test-runner/output/eval_dot.js | 10 ++++++ .../test-runner/output/eval_dot.snapshot | 1 + test/fixtures/test-runner/output/eval_spec.js | 10 ++++++ .../test-runner/output/eval_spec.snapshot | 31 +++++++++++++++++++ test/fixtures/test-runner/output/eval_tap.js | 10 ++++++ .../test-runner/output/eval_tap.snapshot | 31 +++++++++++++++++++ test/parallel/test-runner-output.mjs | 3 ++ 10 files changed, 112 insertions(+), 5 deletions(-) create mode 100644 test/fixtures/test-runner/output/eval_dot.js create mode 100644 test/fixtures/test-runner/output/eval_dot.snapshot create mode 100644 test/fixtures/test-runner/output/eval_spec.js create mode 100644 test/fixtures/test-runner/output/eval_spec.snapshot create mode 100644 test/fixtures/test-runner/output/eval_tap.js create mode 100644 test/fixtures/test-runner/output/eval_tap.snapshot diff --git a/lib/internal/test_runner/reporter/spec.js b/lib/internal/test_runner/reporter/spec.js index b93405872f278c..b1f071d567ed32 100644 --- 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 index ed8c3f836d4694..780d06e8b3c17f 100644 --- 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 index 8fb7426605869d..a19249c337d252 100644 --- a/src/node_util.cc +++ b/src/node_util.cc @@ -148,9 +148,15 @@ static void GetCallerLocation(const FunctionCallbackInfo& args) { } Local frame = trace->GetFrame(isolate, 1); + Local file = frame->GetScriptNameOrSourceURL(); + + if (file.IsEmpty()) { + return; + } + Local 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 new file mode 100644 index 00000000000000..b50ea9bb836e65 --- /dev/null +++ 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 new file mode 100644 index 00000000000000..901a287a41ebfa --- /dev/null +++ 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 new file mode 100644 index 00000000000000..ca4c09a6dac7cb --- /dev/null +++ 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 new file mode 100644 index 00000000000000..5c9a53009508e1 --- /dev/null +++ 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 new file mode 100644 index 00000000000000..3064d3d2ea5c97 --- /dev/null +++ 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 new file mode 100644 index 00000000000000..4f67b5d7d04863 --- /dev/null +++ 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 index c81a0f1160e7eb..1b8d3f00126269 100644 --- 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' },