Skip to content

Commit

Permalink
test_runner: handle undefined test locations
Browse files Browse the repository at this point in the history
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>
  • Loading branch information
cjihrig authored and marco-ippolito committed May 3, 2024
1 parent 882a64e commit e64a25a
Show file tree
Hide file tree
Showing 10 changed files with 112 additions and 5 deletions.
11 changes: 8 additions & 3 deletions lib/internal/test_runner/reporter/spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -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'));
}
Expand Down
2 changes: 1 addition & 1 deletion lib/internal/test_runner/reporter/tap.js
Original file line number Diff line number Diff line change
Expand Up @@ -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':
Expand Down
8 changes: 7 additions & 1 deletion src/node_util.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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)));
}
Expand Down
10 changes: 10 additions & 0 deletions test/fixtures/test-runner/output/eval_dot.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
// Flags: --test-reporter=dot
'use strict';
eval(`
const { test } = require('node:test');
test('passes');
test('fails', () => {
throw new Error('fail');
});
`);
1 change: 1 addition & 0 deletions test/fixtures/test-runner/output/eval_dot.snapshot
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
.X
10 changes: 10 additions & 0 deletions test/fixtures/test-runner/output/eval_spec.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
// Flags: --test-reporter=spec
'use strict';
eval(`
const { test } = require('node:test');
test('passes');
test('fails', () => {
throw new Error('fail');
});
`);
31 changes: 31 additions & 0 deletions test/fixtures/test-runner/output/eval_spec.snapshot
Original file line number Diff line number Diff line change
@@ -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
*
*
*
*
*
*
*
10 changes: 10 additions & 0 deletions test/fixtures/test-runner/output/eval_tap.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
// Flags: --test-reporter=tap
'use strict';
eval(`
const { test } = require('node:test');
test('passes');
test('fails', () => {
throw new Error('fail');
});
`);
31 changes: 31 additions & 0 deletions test/fixtures/test-runner/output/eval_tap.snapshot
Original file line number Diff line number Diff line change
@@ -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 *
3 changes: 3 additions & 0 deletions test/parallel/test-runner-output.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -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' },
Expand Down

0 comments on commit e64a25a

Please sign in to comment.