Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
test: add --use-bundled-ca to tls-cnnic-whitelist
If configued with --openssl-use-def-ca-store --shared-openssl the following error might be thrown: assert.js:86 throw new assert.AssertionError({ ^ AssertionError: 'UNABLE_TO_VERIFY_LEAF_SIGNATURE' === 'CERT_REVOKED' at TLSSocket.client.on.common.mustCall (/Users/danielbevenius/work/nodejs/node/test/parallel/test-tls-cnnic-whitelist.js:71:14) at TLSSocket.<anonymous> (/Users/danielbevenius/work/nodejs/node/test/common.js:461:15) at emitOne (events.js:115:13) at TLSSocket.emit (events.js:210:7) at emitErrorNT (net.js:1305:8) at _combinedTickCallback (internal/process/next_tick.js:80:11) at process._tickCallback (internal/process/next_tick.js:104:9) In this case the CA's used will be the ones shipped with OpenSSL. For tests though we should be able to specify --use-bundled-ca as a fix for the above error, but this functionality was broken by me in commit be98f26 ("src: exclude node_root_certs when use-def-ca-store"). That commit removed the abilty to use --use-bundled-ca if the build was configured --openssl-use-def-ca-store. PR-URL: #12394 Reviewed-By: thefourtheye - Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
- Loading branch information
3cf88a4
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Minor note... the
Reviewed-By
metadata does not match the typical pattern. It's not critical, by any means, but it should be like:(without the github ID)
3cf88a4
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@jasnell Thanks, I'll make sure I do that in future commits. I just took this from a git log and did think it looked a little off but though if that is how it was done before it must be correct.
3cf88a4
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@danbev use https://github.com/evanlucas/node-review, it is faster, and generates the correct metadata.
I thought PRs need two reviews, but https://github.com/nodejs/node/blob/f97156623a140c3bbeee91a038bb727f39fc5948/COLLABORATOR_GUIDE.md#accepting-modifications doesn't say that, not sure how I got that impression.
3cf88a4
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@sam-github At leas two CTC members need to sign off on semver-major change but otherwise a single sign-off from a collaborator is sufficient.