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: reduce diff noise #23048

Closed
wants to merge 3 commits into from

Conversation

BridgeAR
Copy link
Member

Assertion errors that produce a diff show a diff for identical entries
in case one side of the comparison has more object properties than the
other one. Those lines are now taken into account and will not show up
as diverging lines anymore.

Refs: #22763

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

Assertion errors that produce a diff show a diff for identical entries
in case one side of the comparison has more object properties than the
other one. Those lines are now taken into account and will not show up
as diverging lines anymore.

Refs: nodejs#22763
@nodejs-github-bot nodejs-github-bot added the assert Issues and PRs related to the assert subsystem. label Sep 24, 2018
@BridgeAR BridgeAR requested review from addaleax and targos September 24, 2018 08:26
@BridgeAR
Copy link
Member Author

lib/internal/assert.js Show resolved Hide resolved
It became hard to follow what was actually happening in the
algorithm. This adds comments to improve the situation.
@BridgeAR
Copy link
Member Author

@BridgeAR BridgeAR added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Sep 25, 2018
@BridgeAR
Copy link
Member Author

BridgeAR commented Sep 25, 2018

@BridgeAR
Copy link
Member Author

BridgeAR commented Sep 25, 2018

Resumed build: https://ci.nodejs.org/job/node-test-commit/21768/ ✔️ (besides commit-linux)

@BridgeAR
Copy link
Member Author

Rebuild commit-linux: https://ci.nodejs.org/job/node-test-commit-linux/21861/

@BridgeAR
Copy link
Member Author

BridgeAR commented Sep 26, 2018

Rebuild commit-linux again: https://ci.nodejs.org/job/node-test-commit-linux/21867/ ✔️

@BridgeAR
Copy link
Member Author

Landed in 02c44a4 and eccc659.

@BridgeAR BridgeAR closed this Sep 27, 2018
BridgeAR added a commit to BridgeAR/node that referenced this pull request Sep 27, 2018
Assertion errors that produce a diff show a diff for identical entries
in case one side of the comparison has more object properties than the
other one. Those lines are now taken into account and will not show up
as diverging lines anymore.

Refs: nodejs#22763

PR-URL: nodejs#23048
Refs: nodejs#22763
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Shingo Inoue <[email protected]>
Reviewed-By: James M Snell <[email protected]>
BridgeAR added a commit to BridgeAR/node that referenced this pull request Sep 27, 2018
It became hard to follow what was actually happening in the
algorithm. This adds comments to improve the situation.

PR-URL: nodejs#23048
Refs: nodejs#22763
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Shingo Inoue <[email protected]>
Reviewed-By: James M Snell <[email protected]>
@BridgeAR BridgeAR deleted the improve-assertion-diff branch January 20, 2020 11:41
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. author ready PRs that have at least one approval, no pending requests for changes, and a CI started.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants