Skip to content

Conversation

@pulkit-30
Copy link
Contributor

@pulkit-30 pulkit-30 commented Jan 23, 2023

fix: #45836

code

const test = require('node:test'); test('escaped description \\ # \\#\\ \n \t \f \v \b \r'); 
  • tap escaping without --test
TAP version 13 # Subtest: escaped description \\ \# \\\#\\ \\n \\t \\f \\v \\b \\r ok 1 - escaped description \\ \# \\\#\\ \\n \\t \\f \\v \\b \\r --- duration_ms: 3.569875 ... 1..1 # tests 1 # pass 1 # fail 0 # cancelled 0 # skipped 0 # todo 0 # duration_ms 6.989541 
  • tap escaping with --test
TAP version 13 # Subtest: /Users/pulkitgupta/Desktop/node/test.js # Subtest: escaped description \\ \# \\\#\\ \\n \\t \\f \\v \\b \\r ok 1 - escaped description \\ \# \\\#\\ \\n \\t \\f \\v \\b \\r --- duration_ms: 2.582292 ... 1..1 ok 1 - /Users/pulkitgupta/Desktop/node/test.js --- duration_ms: 49.498875 ... 1..1 # tests 1 # pass 1 # fail 0 # cancelled 0 # skipped 0 # todo 0 # duration_ms 50.930334 

Also modify test output's [test/message/test_runner_output.out , test/message/test_runner_output_cli.out]

@nodejs-github-bot
Copy link
Collaborator

Review requested:

  • @nodejs/test_runner
@nodejs-github-bot nodejs-github-bot added dont-land-on-v14.x needs-ci PRs that need a full CI run. test_runner Issues and PRs related to the test runner subsystem. labels Jan 23, 2023
Copy link
Member

@benjamingr benjamingr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe add a test that running with and without --test produces the same output?

@pulkit-30
Copy link
Contributor Author

Hey @benjamingr,
Thanks for the review,
Already there are tests for this (test_runner_output.js, test_runner_output_cli.js), that runs with and without --test flag,
I have modified output for these tests to ensure consistent/same output with and without --test flag.

Please let me know if more tests are required for the same;

@MoLow MoLow requested a review from benjamingr January 26, 2023 07:42
@MoLow MoLow added the request-ci Add this label to start a Jenkins CI on a PR. label Jan 26, 2023
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Jan 26, 2023
@benjamingr benjamingr added the request-ci Add this label to start a Jenkins CI on a PR. label Jan 28, 2023
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Jan 28, 2023
@MoLow MoLow added the commit-queue Add this label to land a pull request using GitHub Actions. label Jan 28, 2023
@nodejs-github-bot nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Jan 28, 2023
@nodejs-github-bot nodejs-github-bot merged commit 2f38c74 into nodejs:main Jan 28, 2023
@nodejs-github-bot
Copy link
Collaborator

Landed in 2f38c74

ruyadorno pushed a commit that referenced this pull request Feb 1, 2023
PR-URL: #46311 Fixes: #45836 Reviewed-By: Moshe Atlow <moshe@atlow.co.il> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
@ruyadorno ruyadorno mentioned this pull request Feb 1, 2023
MoLow pushed a commit to MoLow/node-core-test that referenced this pull request Feb 7, 2023
PR-URL: nodejs/node#46311 Fixes: nodejs/node#45836 Reviewed-By: Moshe Atlow <moshe@atlow.co.il> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> (cherry picked from commit 2f38c74e263ed2e7f3b087efb9adee2442dd25c4)
MoLow pushed a commit to nodejs/node-core-test that referenced this pull request Feb 8, 2023
PR-URL: nodejs/node#46311 Fixes: nodejs/node#45836 Reviewed-By: Moshe Atlow <moshe@atlow.co.il> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> (cherry picked from commit 2f38c74e263ed2e7f3b087efb9adee2442dd25c4)
MoLow pushed a commit to MoLow/node that referenced this pull request Feb 25, 2023
PR-URL: nodejs#46311 Fixes: nodejs#45836 Reviewed-By: Moshe Atlow <moshe@atlow.co.il> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
MoLow pushed a commit to MoLow/node that referenced this pull request Feb 25, 2023
PR-URL: nodejs#46311 Fixes: nodejs#45836 Reviewed-By: Moshe Atlow <moshe@atlow.co.il> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
MoLow pushed a commit to MoLow/node that referenced this pull request Feb 25, 2023
PR-URL: nodejs#46311 Fixes: nodejs#45836 Reviewed-By: Moshe Atlow <moshe@atlow.co.il> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
juanarbol pushed a commit that referenced this pull request Mar 3, 2023
PR-URL: #46311 Backport-PR-URL: #46839 Fixes: #45836 Reviewed-By: Moshe Atlow <moshe@atlow.co.il> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
@juanarbol juanarbol mentioned this pull request Mar 3, 2023
juanarbol pushed a commit that referenced this pull request Mar 5, 2023
PR-URL: #46311 Backport-PR-URL: #46839 Fixes: #45836 Reviewed-By: Moshe Atlow <moshe@atlow.co.il> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

needs-ci PRs that need a full CI run. test_runner Issues and PRs related to the test runner subsystem.

4 participants