Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

test: remove deepStrictEqual() third argument, change it into a comment #20702

Closed
wants to merge 1 commit into from

Conversation

ceccode
Copy link
Contributor

@ceccode ceccode commented May 13, 2018

The call to assert.deepStrictEqual() has a string literal for its third argument. Unfortunately, a side effect of that is that the values of the first two arguments are not displayed if there is an AssertionError. That information is useful for debugging.

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • commit message follows commit guidelines

@nodejs-github-bot nodejs-github-bot added the test Issues and PRs related to the tests. label May 13, 2018
@BridgeAR
Copy link
Member

@BridgeAR BridgeAR added fast-track PRs that do not need to wait for 48 hours to land. author ready PRs that have at least one approval, no pending requests for changes, and a CI started. labels May 13, 2018
@Trott
Copy link
Member

Trott commented May 13, 2018

Collaborators, please 👍 this comment if you are in favor of fast-tracking.

Copy link
Member

@addaleax addaleax left a comment

Choose a reason for hiding this comment

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

Thanks!

Trott pushed a commit to Trott/io.js that referenced this pull request May 13, 2018
The call to assert.deepStrictEqual() has a string literal for its third
argument. Unfortunately, a side effect of that is that the values of the
first two arguments are not displayed if there is an AssertionError.
That information is useful for debugging.

PR-URL: nodejs#20702
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
@Trott
Copy link
Member

Trott commented May 13, 2018

Landed in eafb30c.
Thanks for the contribution! 🎉

@Trott Trott closed this May 13, 2018
addaleax pushed a commit that referenced this pull request May 14, 2018
The call to assert.deepStrictEqual() has a string literal for its third
argument. Unfortunately, a side effect of that is that the values of the
first two arguments are not displayed if there is an AssertionError.
That information is useful for debugging.

PR-URL: #20702
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
@addaleax addaleax mentioned this pull request May 14, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
author ready PRs that have at least one approval, no pending requests for changes, and a CI started. fast-track PRs that do not need to wait for 48 hours to land. test Issues and PRs related to the tests.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants