test: favor deepStrictEqual over deepEqual#12883
Uh oh!
There was an error while loading. Please reload this page.
Conversation
Trott added http 
Trott commented May 7, 2017
There was a problem hiding this comment.
I want to suggest
constheadersCopy=res.getHeaders();// eslint-disable-next-line no-restricted-propertiesconstexpected={'x-test-header': 'testing','x-test-header2': 'testing','set-cookie': cookies,'x-test-array-header': arrayValues};Object.setPrototypeOf(expected,null)assert.deepStrictEqual(headersCopy,expected);And a question, why is headersCopy.__proto__ === undifined?
Also headersCopy.hasOwnPropert === undifined so the API is wrong it says the return value is an Object
https://nodejs.org/api/http.html#http_response_getheaders
There was a problem hiding this comment.
And a question, why is
headersCopy.__proto__ === undifined?
Because, as you noted, headersCopy has a null prototype, and therefore the __proto__ getter inherited from Object.prototype is missing.
Also
headersCopy.hasOwnPropert === undifinedso the API is wrong it says the return value is anObject
https://nodejs.org/api/http.html#http_response_getheaders
It’s still an object, it just doesn’t inherit from Object.prototype; if you have suggestions on how to improve the docs for this, sure.
There was a problem hiding this comment.
In my books it's not an Object if headersCopy instanceof Object === false 🤷♂️
Ref: #12885
* also correct language for the same note for querystring.parse * add assertions for said note PR-URL: nodejs#12887Fixes: nodejs#12885 Refs: nodejs#12883 Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Timothy Gu <[email protected]> Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Sakthipriyan Vairamani <[email protected]>
test-http-mutable-headers uses assert.deepEqual() in three places but appears to only needs it in two of them. Replace one with assert.deepStrictEqual() and remove linting exception.
Trott commented May 10, 2017
Went with @refack's suggestion. New CI: https://ci.nodejs.org/job/node-test-pull-request/7999/ |
test-http-mutable-headers uses assert.deepEqual() in three places but appears to only needs it in two of them. Replace one with assert.deepStrictEqual() and remove linting exception. PR-URL: nodejs#12883 Reviewed-By: Jeremiah Senkpiel <[email protected]> Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Sakthipriyan Vairamani <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Refael Ackermann <[email protected]>
Trott commented May 11, 2017
Landed in 631cb42. |
* also correct language for the same note for querystring.parse * add assertions for said note PR-URL: nodejs#12887Fixes: nodejs#12885 Refs: nodejs#12883 Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Timothy Gu <[email protected]> Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Sakthipriyan Vairamani <[email protected]>
test-http-mutable-headers uses assert.deepEqual() in three places but appears to only needs it in two of them. Replace one with assert.deepStrictEqual() and remove linting exception. PR-URL: nodejs#12883 Reviewed-By: Jeremiah Senkpiel <[email protected]> Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Sakthipriyan Vairamani <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Refael Ackermann <[email protected]>
MylesBorins commented Jun 22, 2017
This does not land cleanly in LTS. Please feel free to manually backport. Please also feel free to replace the backport request label with do-not-land if it shouldn't land |
Trott commented Jun 23, 2017
@MylesBorins If #10805 gets backported, I can backport this after that lands. |
9 participants
test-http-mutable-headers uses assert.deepEqual() in three places but
appears to only needs it in two of them. Replace one with
assert.deepStrictEqual() and remove linting exception.
Checklist
make -j4 test(UNIX), orvcbuild test(Windows) passesAffected core subsystem(s)
test http