[v8.x] backport some assert commits #23223
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Requireing the strict version will allow to use `assert.equal`, `assert.deepEqual` and there negated counterparts to be used with strict comparison instead of using e.g. `assert.strictEqual`. The API is identical to the regular assert export and only differs in the way that all functions use strict compairson. PR-URL: nodejs#17002 Reviewed-By: Refael Ackermann <refack@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Vse Mozhet Byt <vsemozhetbyt@gmail.com> Reviewed-By: Anna Henningsen <anna@addaleax.net>
assert.throws and assert.doesNotThrow set the operator to a internal function. That was by accident and originally the operator was undefined. This changes it to show "throws" or "doesNotThrow". PR-URL: nodejs#17575 Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Anatoli Papirovski <apapirovski@mac.com>
Right now it is difficult to know what argument stands for what property. By refactoring the arguments into a object it is clear what stands for what. PR-URL: nodejs#17582 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Jon Moss <me@jonathanmoss.me> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Anatoli Papirovski <apapirovski@mac.com>
The stack frames from .throws and .doesNotThrow got included even though that was not intended. PR-URL: nodejs#17703 Reviewed-By: Timothy Gu <timothygu99@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
Throw a TypeError in case a error message is provided in the second argument and a third argument is present as well. This is clearly a mistake and should not be done. PR-URL: nodejs#17585 Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com> Reviewed-By: Evan Lucas <evanlucas@me.com>
From now on it is possible to use a validation object in throws instead of the other possibilites. PR-URL: nodejs#17584 Refs: nodejs#17557 Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Ron Korving <ron@ronkorving.nl> Reviewed-By: Yuta Hiroto <hello@about-hiroppy.com>
Using `assert()` or `assert.ok()` resulted in a error since a refactoring. Refs: nodejs#17582 PR-URL: nodejs#17903 Refs: nodejs#17582 Reviewed-By: Anatoli Papirovski <apapirovski@mac.com> Reviewed-By: Tobias Nießen <tniessen@tnie.de> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
The current stack trace thrown in case `assert.throws(fn, object)` is used did not filter the stack trace. This fixes it. PR-URL: nodejs#18595 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Anatoli Papirovski <apapirovski@mac.com>
nodejs-github-bot
added
assert
|
landed in e3bddee...18071db |
MylesBorins
pushed a commit
that referenced
this pull request
Oct 31, 2018
Requireing the strict version will allow to use `assert.equal`, `assert.deepEqual` and there negated counterparts to be used with strict comparison instead of using e.g. `assert.strictEqual`. The API is identical to the regular assert export and only differs in the way that all functions use strict compairson. Backport-PR-URL: #23223 PR-URL: #17002 Reviewed-By: Refael Ackermann <refack@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Vse Mozhet Byt <vsemozhetbyt@gmail.com> Reviewed-By: Anna Henningsen <anna@addaleax.net>
MylesBorins
pushed a commit
that referenced
this pull request
Oct 31, 2018
assert.throws and assert.doesNotThrow set the operator to a internal function. That was by accident and originally the operator was undefined. This changes it to show "throws" or "doesNotThrow". Backport-PR-URL: #23223 PR-URL: #17575 Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Anatoli Papirovski <apapirovski@mac.com>
MylesBorins
pushed a commit
that referenced
this pull request
Oct 31, 2018
Right now it is difficult to know what argument stands for what property. By refactoring the arguments into a object it is clear what stands for what. Backport-PR-URL: #23223 PR-URL: #17582 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Jon Moss <me@jonathanmoss.me> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Anatoli Papirovski <apapirovski@mac.com>
MylesBorins
pushed a commit
that referenced
this pull request
Oct 31, 2018
Throw a TypeError in case a error message is provided in the second argument and a third argument is present as well. This is clearly a mistake and should not be done. Backport-PR-URL: #23223 PR-URL: #17585 Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com> Reviewed-By: Evan Lucas <evanlucas@me.com>
MylesBorins
pushed a commit
that referenced
this pull request
Oct 31, 2018
From now on it is possible to use a validation object in throws instead of the other possibilites. Backport-PR-URL: #23223 PR-URL: #17584 Refs: #17557 Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Ron Korving <ron@ronkorving.nl> Reviewed-By: Yuta Hiroto <hello@about-hiroppy.com>
MylesBorins
pushed a commit
that referenced
this pull request
Oct 31, 2018
Using `assert()` or `assert.ok()` resulted in a error since a refactoring. Refs: #17582 Backport-PR-URL: #23223 PR-URL: #17903 Refs: #17582 Reviewed-By: Anatoli Papirovski <apapirovski@mac.com> Reviewed-By: Tobias Nießen <tniessen@tnie.de> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
MylesBorins
added
the
semver-minor
Merged
BethGriggs
added a commit
that referenced
this pull request
Nov 20, 2018
Notable changes: * **assert**: - backport some assert commits (Ruben Bridgewater) [#23223](#23223) * **deps**: - V8: cherry-pick 64-bit hash seed commits (Yang Guo) [#23274](#23274) * **http**: - added aborted property to request (Robert Nagy) [#20094](#20094) * **http2**: - backport http2 changes from 10.x (Kelvin Jin) [#22850](#22850) PR-URL: #23974
MylesBorins
pushed a commit
that referenced
this pull request
Nov 20, 2018
Notable changes:
* **assert**:
- backport some assert commits (Ruben Bridgewater)
[#23223](#23223)
* **deps**:
- upgrade to libuv 1.23.2 (cjihrig)
[#23336](#23336)
- V8: cherry-pick 64-bit hash seed commits (Yang Guo)
[#23274](#23274)
* **http**:
- added aborted property to request (Robert Nagy)
[#20094](#20094)
* **http2**:
- graduate from experimental (James M Snell)
[#22466](#22466)
PR-URL: #23974
MylesBorins
pushed a commit
that referenced
this pull request
Nov 20, 2018
Notable changes:
* **assert**:
- backport some assert commits (Ruben Bridgewater)
[#23223](#23223)
* **deps**:
- upgrade to libuv 1.23.2 (cjihrig)
[#23336](#23336)
- V8: cherry-pick 64-bit hash seed commits (Yang Guo)
[#23274](#23274)
* **http**:
- added aborted property to request (Robert Nagy)
[#20094](#20094)
* **http2**:
- graduate from experimental (James M Snell)
[#22466](#22466)
PR-URL: #23974
|
The backport of #17585 (147aeed) included #12167 (adding It's definitely an improvement, but it would be nice if it had been part of the changelog for 8.13 so it would have been easier to find 🙂 (PR fixing Jest's test suite: jestjs/jest#7446) |
|
It was not intentional to backport that part as it's semver-major. I know I tried to keep it out while rebasing but it seemed like it still sneaked in. I am going to open a fix for that. |
|
I've fixed Jest's test suite (it was simply a matter of treating node 8 same as 9 in the test we have with node core |
BridgeAR
added a commit
to BridgeAR/node
that referenced
this pull request
Dec 2, 2018
It was not intended to change the `assert.doesNotThrow()` message with nodejs#23223. This reverts the breaking behavior to the one before.
|
I agree that it is an improvement but it was never meant to be released on 8.x. I guess there will be little harm keeping it but I am still opening a revert and keep the decision to other collaborators. |
4 tasks
refack
pushed a commit
to refack/node
that referenced
this pull request
Jan 14, 2019
Notable changes:
* **assert**:
- backport some assert commits (Ruben Bridgewater)
[nodejs#23223](nodejs#23223)
* **deps**:
- upgrade to libuv 1.23.2 (cjihrig)
[nodejs#23336](nodejs#23336)
- V8: cherry-pick 64-bit hash seed commits (Yang Guo)
[nodejs#23274](nodejs#23274)
* **http**:
- added aborted property to request (Robert Nagy)
[nodejs#20094](nodejs#20094)
* **http2**:
- graduate from experimental (James M Snell)
[nodejs#22466](nodejs#22466)
PR-URL: nodejs#23974
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Using assert became much more friendly recently. As long as we still have the chance to get some commits into 8 I thought I go ahead and open a backport for these.
Checklist
make -j4 test(UNIX), orvcbuild test(Windows) passes