test: add test for invalid cert string #8179
Conversation
`tls.parseCertString()` should return an empty object if passed an invalid cert string. This behavior is not currently tested. Add minimal test.
Trott
added
tls
|
|
||
| const invalid = 'fhqwhgads'; | ||
| const invalidOut = tls.parseCertString(invalid); | ||
| assert.deepStrictEqual(invalidOut, {}); |
There was a problem hiding this comment.
Add brackets to scope the above statements?
|
LGTM with or without scoping. The rest of the test doesn't do it, it's not out of character. |
|
LGTM |
|
LGTM with CI: https://ci.nodejs.org/job/node-test-pull-request/3749/ Scoping is nice. |
|
I prefer scoped consts in the tests over unscoped, but also favor small diffs over adding-stuff-just-because-I-can. The inner tension is TOO MUCH!!!! OK, I added scopes! h/t @yorkie @Fishrock123 Re-running CI: https://ci.nodejs.org/job/node-test-pull-request/3753/ |
|
LGTM with scoped commit if CI is happy :p |
|
@jbergstroem test/arm passed an hour ago but the GH widget thing still says "pending". It had previously failed. So maybe the bug is specific to re-running CI? |
|
@jbergstroem Whoops! Nope! I'm wrong, it actually is still running. The Details link opens the wrong page, I think. |
|
Details link opens https://ci.nodejs.org/job/node-test-commit-arm/nodes=armv7-wheezy/4590/ but should probably open https://ci.nodejs.org/job/node-test-commit-arm/4590/ instead. |
|
@Trott I think the path for fanned jobs might be incorrect. Might be racy -- I'll add it to my todo. |
`tls.parseCertString()` should return an empty object if passed an invalid cert string. This behavior is not currently tested. Add minimal test. PR-URL: nodejs#8179 Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com> Reviewed-By: Yorkie Liu <yorkiefixer@gmail.com>
|
Landed in 12d7a50 |
`tls.parseCertString()` should return an empty object if passed an invalid cert string. This behavior is not currently tested. Add minimal test. PR-URL: #8179 Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com> Reviewed-By: Yorkie Liu <yorkiefixer@gmail.com>
Checklist
make -j4 test(UNIX), orvcbuild test nosign(Windows) passesAffected core subsystem(s)
test tls
Description of change
tls.parseCertString()should return an empty object if passed aninvalid cert string. This behavior is not currently tested. Add minimal
test.