Repository navigation
node:test outputs invalid TAP if there is a newline in the test name #45396
Description
Activity
- addedgood first issueIssues that are suitable for first-time contributors.Issues that are suitable for first-time contributors.test_runnerIssues and PRs related to the test runner subsystem.Issues and PRs related to the test runner subsystem.
on Nov 10, 2022 Hey. I would like to work on it. Is it available?
Is this still available to work on? It's my first issue but I think I should be able to finish it up pretty quickly.
Yep :) go ahed!
@MoLow can you assign this issue to me? :)
@daltonna If you are looking for some good first issue
Issues that are suitable for first-time contributors. , you can go over comments in #45326 that were left for follow-up and implement themHey @MoLow, how can i test changes?
you can run any test using
./node test/path/to/the/test.jsare usingtools/test.py test/path/to/the/test.js
see the contribution guide: https://github.com/nodejs/node/blob/main/doc/contributing/pull-requests.md#the-process-of-making-changesReacted by SevkiHey @MoLow,
I have just created a PR about this issue in here.
It works but i think it should be more cleaner resolve.
In addition to i should find a solution how it include some escape characters such as\r,\t,\b,\f,\v,\',\", too.I had tried that create an array like that
const escapedCharacters = ['\n', '\t', '\b' ...]and loop in here. I wanted to call StringPrototypeReplaceAll function with each escape characters. Unfortunately I couldn't like that. It may be cleaner solving.What do you think about it? Do you have any suggestion for me?
Thanks.- added a commit that references this issue
on Dec 11, 2022 - added 2 commits that reference this issue
on Dec 12, 2022 - added a commit that references this issue
on Feb 2, 2023
Version
19.0.1
Platform
Darwin Kernel Version 21.3.0: Wed Jan 5 21:37:58 PST 2022; root:xnu-8019.80.24~20/RELEASE_ARM64_T8101 arm64
Subsystem
node:test
What steps will reproduce the bug?
How often does it reproduce? Is there a required condition?
Requires a test name with a newline character. Suspect other characters should be escaped as well to prevent problems I stumbled across this when writing something like:
What is the expected behavior?
The
node:testTAP producer should be robust to test names and produce valid TAP. I would recommend escaping\n(and probably other) characters when rendering test names to TAP.The output should be:
What do you see instead?
The newline is not escaped. In this case I'm using
not okafter the newline to drive home the problems as its particularly nasty. The output of the supplied example is:Depending on the contents after any
\ns in test names, this could result in totally invalid TAP, false diagnostics or misleading TAP output like above.Additional information
it seems that the escaping should also occur in diagnostic serialization code. In the example output above, the newline in the diagnostic was not escaped in the diagnostic either.
Interestingly,
#seems to be escaped already so:produces good output of:
@nodejs/test_runner