Skip to content

Commit 3c5764a

Browse files
cjihrigcjihrig
authored
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(). 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>
1 parent 63d04d4 commit 3c5764a

File tree

10 files changed

+112
-5
lines changed

10 files changed

+112
-5
lines changed

‎lib/internal/test_runner/reporter/spec.js‎

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -148,11 +148,16 @@ class SpecReporter extends Transform {
148148
const results = [`\n${this.#colors['test:fail']}${symbols['test:fail']}failing tests:${colors.white}\n`];
149149
for (let i = 0; i < this.#failedTests.length; i++) {
150150
const test = this.#failedTests[i];
151-
const relPath = relative(this.#cwd, test.file);
152151
const formattedErr = this.#formatTestReport('test:fail', test);
153-
const location = `test at ${relPath}:${test.line}:${test.column}`;
154152

155-
ArrayPrototypePush(results, location, formattedErr);
153+
if (test.file) {
154+
const relPath = relative(this.#cwd, test.file);
155+
const location = `test at ${relPath}:${test.line}:${test.column}`;
156+
157+
ArrayPrototypePush(results, location);
158+
}
159+
160+
ArrayPrototypePush(results, formattedErr);
156161
}
157162
callback(null, ArrayPrototypeJoin(results, '\n'));
158163
}

‎lib/internal/test_runner/reporter/tap.js‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@ async function * tapReporter(source) {
3434
switch (type) {
3535
case 'test:fail': {
3636
yield reportTest(data.nesting, data.testNumber, 'not ok', data.name, data.skip, data.todo);
37-
const location = `${data.file}:${data.line}:${data.column}`;
37+
const location = data.file ? `${data.file}:${data.line}:${data.column}` : null;
3838
yield reportDetails(data.nesting, data.details, location);
3939
break;
4040
} case 'test:pass':

‎src/node_util.cc‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -148,9 +148,15 @@ static void GetCallerLocation(const FunctionCallbackInfo<Value>& args) {
148148
}
149149

150150
Local<StackFrame> frame = trace->GetFrame(isolate, 1);
151+
Local<Value> file = frame->GetScriptNameOrSourceURL();
152+
153+
if (file.IsEmpty()) {
154+
return;
155+
}
156+
151157
Local<Value> ret[] = {Integer::New(isolate, frame->GetLineNumber()),
152158
Integer::New(isolate, frame->GetColumn()),
153-
frame->GetScriptNameOrSourceURL()};
159+
file};
154160

155161
args.GetReturnValue().Set(Array::New(args.GetIsolate(), ret, arraysize(ret)));
156162
}

‎test/fixtures/test-runner/output/eval_dot.js‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
// Flags: --test-reporter=dot
2+
'use strict';
3+
eval(`
4+
const { test } = require('node:test');
5+
6+
test('passes');
7+
test('fails', () => {
8+
throw new Error('fail');
9+
});
10+
`);

‎test/fixtures/test-runner/output/eval_dot.snapshot‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
.X

‎test/fixtures/test-runner/output/eval_spec.js‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
// Flags: --test-reporter=spec
2+
'use strict';
3+
eval(`
4+
const { test } = require('node:test');
5+
6+
test('passes');
7+
test('fails', () => {
8+
throw new Error('fail');
9+
});
10+
`);

‎test/fixtures/test-runner/output/eval_spec.snapshot‎

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
1+
✔ passes (*ms)
2+
✖ fails (*ms)
3+
Error: fail
4+
*
5+
*
6+
*
7+
*
8+
*
9+
*
10+
*
11+
12+
ℹ tests 2
13+
ℹ suites 0
14+
ℹ pass 1
15+
ℹ fail 1
16+
ℹ cancelled 0
17+
ℹ skipped 0
18+
ℹ todo 0
19+
ℹ duration_ms *
20+
21+
✖ failing tests:
22+
23+
✖ fails (*ms)
24+
Error: fail
25+
*
26+
*
27+
*
28+
*
29+
*
30+
*
31+
*

‎test/fixtures/test-runner/output/eval_tap.js‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
// Flags: --test-reporter=tap
2+
'use strict';
3+
eval(`
4+
const { test } = require('node:test');
5+
6+
test('passes');
7+
test('fails', () => {
8+
throw new Error('fail');
9+
});
10+
`);

‎test/fixtures/test-runner/output/eval_tap.snapshot‎

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
1+
TAP version 13
2+
# Subtest: passes
3+
ok 1 - passes
4+
---
5+
duration_ms: *
6+
...
7+
# Subtest: fails
8+
not ok 2 - fails
9+
---
10+
duration_ms: *
11+
failureType: 'testCodeFailure'
12+
error: 'fail'
13+
code: 'ERR_TEST_FAILURE'
14+
stack: |-
15+
*
16+
*
17+
*
18+
*
19+
*
20+
*
21+
*
22+
...
23+
1..2
24+
# tests 2
25+
# suites 0
26+
# pass 1
27+
# fail 1
28+
# cancelled 0
29+
# skipped 0
30+
# todo 0
31+
# duration_ms *

‎test/parallel/test-runner-output.mjs‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -91,6 +91,9 @@ const tests = [
9191
{ name: 'test-runner/output/abort_hooks.js' },
9292
{ name: 'test-runner/output/describe_it.js' },
9393
{ name: 'test-runner/output/describe_nested.js' },
94+
{ name: 'test-runner/output/eval_dot.js' },
95+
{ name: 'test-runner/output/eval_spec.js', transform: specTransform },
96+
{ name: 'test-runner/output/eval_tap.js' },
9497
{ name: 'test-runner/output/hooks.js' },
9598
{ name: 'test-runner/output/hooks_spec_reporter.js', transform: specTransform },
9699
{ name: 'test-runner/output/timeout_in_before_each_should_not_affect_further_tests.js' },

0 commit comments

Comments
 (0)