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
@nodejs-github-bot
Copy link
Collaborator

@nodejs-github-bot
Copy link
Collaborator

@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 <[email protected]>
Reviewed-By: Benjamin Gruenbaum <[email protected]>
@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 <[email protected]>
Reviewed-By: Benjamin Gruenbaum <[email protected]>
(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 <[email protected]>
Reviewed-By: Benjamin Gruenbaum <[email protected]>
(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 <[email protected]>
Reviewed-By: Benjamin Gruenbaum <[email protected]>
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 <[email protected]>
Reviewed-By: Benjamin Gruenbaum <[email protected]>
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 <[email protected]>
Reviewed-By: Benjamin Gruenbaum <[email protected]>
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 <[email protected]>
Reviewed-By: Benjamin Gruenbaum <[email protected]>
@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 <[email protected]>
Reviewed-By: Benjamin Gruenbaum <[email protected]>
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.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Tap escaping not consistent with and without --test
4 participants