test: fix assert parameters order#23581
Conversation
This reverts commit 116134f.
… the value and the second one the expected value.
cjihrig
left a comment
There was a problem hiding this comment.
Using the red X just so someone acknowledges my comment. But if we collectively agree that change is correct, feel free to dismiss my review.
| const expected = '/café🐶'; | ||
|
|
||
| assert.strictEqual('/caf\u{e9}\u{1f436}', expected); | ||
| assert.strictEqual(expected, '/caf\u{e9}\u{1f436}'); |
There was a problem hiding this comment.
Are we sure that this one is correct?
There was a problem hiding this comment.
@cjihrig I think both the code and the change are correct… are you worried about anything in particular?
There was a problem hiding this comment.
The parameter order is actual, expected. It looks like that was already correct. The variable is even named expected 😄
There was a problem hiding this comment.
Oh, right. I think that is because expected is what we expect later down here in the test, and this is just double-checking that we are expecting the right thing? 😄
There was a problem hiding this comment.
I guess maybe it's an edge case or just poorly named. Either way, I won't block this.
There was a problem hiding this comment.
I guess the problem is with the overloaded usage of the variable expected, with name perfectly fitting for the client-server case while causing confusion in the assertion.
thefourtheye
left a comment
There was a problem hiding this comment.
The commits are not attributed to the OP.
|
Resume Build CI: https://ci.nodejs.org/job/node-test-pull-request/17968/ |
|
Landed in 35ed66c. Thanks for the contribution! 🎉 (If you're interested in other possible contributions to Node.js but don't have a good idea of where to start looking, some ideas are posted at https://www.nodetodo.org/next-steps/.) |
Change the order of assert parameters so the first argument is the value and the second one the expected value. PR-URL: nodejs#23581 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de> Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com> Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
Change the order of assert parameters so the first argument is the value and the second one the expected value. PR-URL: #23581 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de> Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com> Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
Change the order of assert parameters so the first argument is the value and the second one the expected value. PR-URL: #23581 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de> Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com> Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
Change the order of assert parameters so the first argument is the value and the second one the expected value. PR-URL: #23581 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de> Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com> Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
Change the order of assert parameters so the first argument is the value and the second one the expected value. PR-URL: #23581 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de> Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com> Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
Change the order of assert parameters so the first argument is the value and the second one the expected value. PR-URL: #23581 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de> Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com> Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
Changes the order of the assertion parameters in
test/known_issues/test-http-path-contains-unicode, so the first argument is the value and the second argument is the expected value as assertion documentation expects.Checklist
make -j4 test(UNIX), orvcbuild test(Windows) passes