test: Added more validations to setEncoding#9997
test: Added more validations to setEncoding#9997pauljlucas wants to merge 2 commits intonodejs:masterfrom
Conversation
imyller
added
code-and-learn
|
Can you fix the lint errors? |
- Added more strict validations to properly test setEncoding - Changed all var usage to const or let - Changed assert.equal to assert.strictEqual
pauljlucas
force-pushed
the
paul/updating-setencoding-test
branch
from
309a854 to
5066e27
Compare
|
Hey @gibfahn and @bnoordhuis, Figured out the issue and fixed. Looks like my comments went on too long :) Let me know if you have anything else that you would like me to change and thanks again for the workshop today. |
|
Hey @gibfahn Just looked at the failed linux portion and am seeing that the test i modified passes: from CI console output: https://ci.nodejs.org/job/node-test-commit-linux/6374/nodes=ubuntu1604_docker_alpine34-64/console Not sure if this is something on the code side or jenkins side, but if you have any ideas would appreciate you pointing me in the right direction to get this working. Thanks |
|
@pauljlucas Looks like a flaky test. As the test you modified passed on all platforms, I'd say CI was successful. We could rerun, but I don't think it's necessary. |
|
Thanks for the clarification @gibfahn. I'm fine with whatever the standard is. Let me know if any clarifications are needed or if there is anything i need to do for getting the code reviewed. |
|
|
||
| // Confirming the buffer string is encoded in ASCII | ||
| // and thus does NOT match the UTF8 string | ||
| assert.notEqual(buffer, messageUtf8); |
There was a problem hiding this comment.
Can you use assert.notStrictEqual() here.
This is changing a notEqual test to notStrictEqual as reference in pr: nodejs#9997
* Added more strict validations to properly test setEncoding * Changed all var usage to const or let * Changed assert.equal to assert.strictEqual PR-URL: #9997 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
|
Landed in 0c45959. Thank you for the PR and for participating in code-and-learn! |
* Added more strict validations to properly test setEncoding * Changed all var usage to const or let * Changed assert.equal to assert.strictEqual PR-URL: #9997 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
* Added more strict validations to properly test setEncoding * Changed all var usage to const or let * Changed assert.equal to assert.strictEqual PR-URL: nodejs#9997 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
* Added more strict validations to properly test setEncoding * Changed all var usage to const or let * Changed assert.equal to assert.strictEqual PR-URL: nodejs#9997 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
* Added more strict validations to properly test setEncoding * Changed all var usage to const or let * Changed assert.equal to assert.strictEqual PR-URL: #9997 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
9 participants
Checklist
make -j8 test(UNIX), orvcbuild test nosign(Windows) passesAffected core subsystem(s)
test
Description of change
Added more strict validations to properly test setEncoding
Changed all var usage to const or let
Changed assert.equal to assert.strictEqual