Repository navigation
Tap parser fails if a test logs a number #46048
Description
Activity
cc @manekinekko
Reacted by Wassim CheghamWhile testing my PR, I observed something unexpected,
when i run codetest('testing', () => { console.log(123); })test('testing', () => { console.log("123"); })it throws error, saying
error: "Cannot read properties of undefined (reading 'value')", that points herebut when I am running code
test('testing', () => { console.log(123, ".."); console.log(123, "..", 123); })then it pass, but doesn't print the expected output
and with code
test('testing', () => { console.log("anytext 123"); console.log("anytest", 123); })it passed as expected.
what I found, this happening because of logic written inside
#Plan()function.your views @cjihrig @manekinekko
@pulkit-30 writing to stdout has the potential to interfere with TAP parsing because TAP also uses stdout. I believe that's what is happening here. I don't think we can really avoid that aspect, but the parser should be able to handle these cases without crashing.
Reacted by Pulkit GuptaIt would be very nice if output during tests could be redirected as e.g. TAP comments. I'm off and on working on building a node:test extension for VS Code and ended up implementing that by hand in a wrapper script
Reacted by Remco HaszingAh, got it. That only happens when running
node --test test.js, not when running simplynode test.js.Reacted by Colin Ihrig, Moshe Atlow, Wassim Chegham and Pulkit Gupta@pulkit-30 writing to stdout has the potential to interfere with TAP parsing because TAP also uses stdout. I believe that's what is happening here. I don't think we can really avoid that aspect, but the parser should be able to handle these cases without crashing.
The parser throws on invalid syntax by design. We can however catch those exceptions (in the runner) and decide whether we silence them or how to handle invalid syntax. @cjihrig @MoLow any suggestions?
The parser throws on invalid syntax by design.
I don't think that is the correct behavior, based on this text from the spec:
Any line that is not a valid version, plan, test point, YAML diagnostic, pragma, a blank line, or a bail out is invalid TAP.
A Harness may silently ignore invalid TAP lines, pass them through to its own stderr or stdout, or report them in some other fashion. However, Harnesses should not treat invalid TAP lines as a test failure by default.Right now, it looks like we are treating it as a failure. We're also surfacing an error message (
Cannot read properties of undefined (reading 'value')) that seems much more like a bug in Node than an intentional error message.We're also losing valid data from that file. Given the following file, I would expect test 1 and test 3 at least to pass.
const test = require('node:test'); test('test 1'); test('test 2', () => { console.log('1234'); }); test('test 3');
In my opinion we should ignore invalid TAP, but continue parsing.
EDIT: By ignore invalid TAP, I really mean display it as a diagnostic and ignore it from a parsing perspective.
Reacted by Moshe Atlow, Pulkit Gupta and Toni Villena- added a commit that references this issue
on Feb 6, 2023 - added 2 commits that reference this issue
on Feb 7, 2023 - added a commit that references this issue
on Feb 18, 2023 - added 2 commits that reference this issue
on Mar 3, 2023
Version
v19.3.0
Platform
Linux Helios 5.10.102.1-microsoft-standard-WSL2 #1 SMP Wed Mar 2 00:30:59 UTC 2022 x86_64 x86_64 x86_64 GNU/Linux
Subsystem
test_runner
What steps will reproduce the bug?
Have a test file containing
How often does it reproduce? Is there a required condition?
100% reproduction
What is the expected behavior?
The test passes when running
node --testWhat do you see instead?
Additional information
This happens for any numberic value, so
console.log('1234')will produce the same result