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

assert: use error diffs in throws messages #19463

Closed
wants to merge 1 commit into from

Conversation

BridgeAR
Copy link
Member

@BridgeAR BridgeAR commented Mar 20, 2018

This switches the assert.throws output to the one used in strict mode
if a error object is used for comparison. From now on it will show
the complete difference between two objects instead of only showing
the first failing property.

It also fixes detecting properties with a undefined value and fails
in case the thrown error does not contain the value at all.

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • tests and/or benchmarks are included
  • documentation is changed or added
  • commit message follows commit guidelines

@nodejs-github-bot nodejs-github-bot added the assert Issues and PRs related to the assert subsystem. label Mar 20, 2018
@BridgeAR
Copy link
Member Author

@BridgeAR BridgeAR added the wip Issues and PRs that are still a work in progress. label Mar 20, 2018
@BridgeAR BridgeAR force-pushed the improve-throws-message branch from 6270f76 to ba3edd5 Compare March 20, 2018 14:13
This switches the assert.throws output to the one used in strict mode
if a error object is used for comparison. From now on it will show
the complete difference between two objects instead of only showing
the first failing property.

It also fixes detecting properties with a undefined value and fails
in case the thrown error does not contain the value at all.
@BridgeAR BridgeAR force-pushed the improve-throws-message branch from ba3edd5 to c7cef58 Compare March 20, 2018 14:23
@BridgeAR BridgeAR removed the wip Issues and PRs that are still a work in progress. label Mar 20, 2018
@BridgeAR
Copy link
Member Author

I updated the implementation again to have a better overall output. PTAL.

@BridgeAR
Copy link
Member Author

Copy link
Member

@mcollina mcollina left a comment

Choose a reason for hiding this comment

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

LGTM

BridgeAR added a commit to BridgeAR/node that referenced this pull request Mar 25, 2018
This switches the assert.throws output to the one used in strict mode
if a error object is used for comparison. From now on it will show
the complete difference between two objects instead of only showing
the first failing property.

It also fixes detecting properties with a undefined value and fails
in case the thrown error does not contain the value at all.

PR-URL: nodejs#19463
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
@BridgeAR
Copy link
Member Author

Landed in a1c96f8

@BridgeAR BridgeAR closed this Mar 25, 2018
@BridgeAR BridgeAR reopened this Mar 25, 2018
@BridgeAR BridgeAR closed this Mar 25, 2018
@targos
Copy link
Member

targos commented Apr 2, 2018

Should this be backported to v9.x-staging? If yes please follow the guide and raise a backport PR, if not let me know or add the dont-land-on label.

@BridgeAR BridgeAR added semver-major PRs that contain breaking changes and should be released in the next major version. and removed backport-requested-v9.x labels May 1, 2018
@BridgeAR BridgeAR deleted the improve-throws-message branch April 1, 2019 23:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
assert Issues and PRs related to the assert subsystem. semver-major PRs that contain breaking changes and should be released in the next major version.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants