Skip to content

test: add hasCrypto check to test-cli-node-options - #12692

Closed
danbev wants to merge 1 commit into
nodejs:masterfrom
danbev:add-has-crypto-check-cli-node-options
Closed

test: add hasCrypto check to test-cli-node-options#12692
danbev wants to merge 1 commit into
nodejs:masterfrom
danbev:add-has-crypto-check-cli-node-options

Conversation

@danbev

Copy link
Copy Markdown
Contributor

Currently when configure --without-ssl the test will throw the following
error:

bad option: --use-openssl-ca

This commit checks if crypto was enabled and skips the crypto related
tests if that is the case.

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • commit message follows commit guidelines
Affected core subsystem(s)

test

Currently when configure --without-ssl the test will throw the following
error:
bad option: --use-openssl-ca
This commit checks if crypto was enabled and skips the crypto related
tests if that is the case.
@nodejs-github-botnodejs-github-bot added the test Issues and PRs related to the tests. label Apr 27, 2017
@danbev

Copy link
Copy Markdown
ContributorAuthor

@mscdexmscdex added the cli Issues and PRs related to the Node.js command line interface. label Apr 27, 2017
@sam-github

Copy link
Copy Markdown
Contributor

@nodejs/testing @nodejs/build We need a CI job, probably linux-only would be fine, that does a configure and test without SSL. This fix is only one of a string of fixes that have been made for this. Its really hard to remember such an obscure build case, sorry @danbev

@gibfahn

gibfahn commented Apr 27, 2017

Copy link
Copy Markdown
Member

@sam-github it's on my list... nodejs/build#643

I'm thinking that another job that runs a variety of targets with different configurations would probably be useful, (we need --without-intl as well)

danbev added a commit to danbev/node that referenced this pull request Apr 29, 2017
Currently when configure --without-ssl the test will throw the following
error:
bad option: --use-openssl-ca
This commit checks if crypto was enabled and skips the crypto related
tests if that is the case.
PR-URL: nodejs#12692
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Sam Roberts <vieuxtech@gmail.com>/
@danbev

Copy link
Copy Markdown
ContributorAuthor

Landed in 427cd29

@danbevdanbev closed this Apr 29, 2017
@danbev
danbev deleted the add-has-crypto-check-cli-node-options branch April 30, 2017 05:39
@jasnelljasnell mentioned this pull request May 11, 2017
@gibfahn

Copy link
Copy Markdown
Member

Should land with #12677

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cliIssues and PRs related to the Node.js command line interface.testIssues and PRs related to the tests.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

9 participants

@danbev@sam-github@gibfahn@jasnell@lpinca@cjihrig@mscdex@MylesBorins@nodejs-github-bot