New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

test: move require('https') to after crypto check #25388

Closed

danbev wants to merge 2 commits into nodejs:master from danbev:https-client-override-crypto-check

Contributor

danbev commented Jan 8, 2019

Currently, test-https-client-override-global-agent.js fails with the
following error when configured --without-ssl:

Error [ERR_NO_CRYPTO]:
Node.js is not compiled with OpenSSL crypto support
  at Object.assertCrypto (internal/util.js:101:11)
  ...
  at Object.<anonymous>
  (/node/test/parallel/test-https-client-override-global-agent.js:5:15)

This commit moves the require statement to after the crypto check.

Checklist

make -j4 test (UNIX), or vcbuild test (Windows) passes
commit message follows commit guidelines


          test: move require('https') to after crypto check

09cda82

Currently, test-https-client-override-global-agent.js fails with the
following error when configured --without-ssl:

Error [ERR_NO_CRYPTO]:
Node.js is not compiled with OpenSSL crypto support
  at Object.assertCrypto (internal/util.js:101:11)
  ...
  at Object.<anonymous>
  (/node/test/parallel/test-https-client-override-global-agent.js:5:15)

This commit moves the require statement to after the crypto check.

Collaborator

nodejs-github-bot commented Jan 8, 2019

@danbev build started: https://ci.nodejs.org/blue/organizations/jenkins/node-test-pull-request-lite-pipeline/detail/node-test-pull-request-lite-pipeline/2175/pipeline

nodejs-github-bot added the test label

Contributor Author

danbev commented Jan 8, 2019

CI: https://ci.nodejs.org/job/node-test-pull-request/19987/

Trott approved these changes

View reviewed changes

Member

Trott left a comment

LGTM.

I suggest moving the check to immediately after const common = .... If we standardize on doing crypto (and other) checks before loading any other modules, it may be possible to update our current "include cyprto check" lint rule to also make sure the crypto check happens before any other modules are loaded, catching this sort of thing before it happens.

Contributor Author

danbev commented Jan 8, 2019

I suggest moving the check to immediately after const common = ....

Sounds good, I'll update shortly. Thanks


          squash! move hasCrypto straight after common req

3ad31a9

Contributor Author

danbev commented Jan 8, 2019 •

edited

Loading

Re-run of failing node-test-commit-freebsd ✔️
Re-run of failing node-test-commit-smartos ✔️

cjihrig approved these changes

View reviewed changes

cjihrig mentioned this pull request

tools: update crypo check rule #25399

Closed

3 tasks

richardlau approved these changes

View reviewed changes

jasnell approved these changes

View reviewed changes

Contributor Author

danbev commented Jan 11, 2019

Landed in 9abb646.

danbev closed this

danbev deleted the https-client-override-crypto-check branch

January 11, 2019 04:38

danbev added a commit that referenced this pull request


          test: move require('https') to after crypto check

9abb646

Currently, test-https-client-override-global-agent.js fails with the
following error when configured --without-ssl:

Error [ERR_NO_CRYPTO]:
Node.js is not compiled with OpenSSL crypto support
  at Object.assertCrypto (internal/util.js:101:11)
  ...
  at Object.<anonymous>
  (/node/test/parallel/test-https-client-override-global-agent.js:5:15)

This commit moves the require statement to after the crypto check.

PR-URL: #25388
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: James M Snell <[email protected]>

danbev pushed a commit that referenced this pull request


          tools: update crypo check rule

4c9ea8f

This commit updates the custom crypto-check ESLint rule to
detect require() calls that come before any hasCrypto
checks.

PR-URL: #25399
Refs: #25388
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Daniel Bevenius <[email protected]>

addaleax pushed a commit that referenced this pull request


          test: move require('https') to after crypto check

fe9b6ee

Currently, test-https-client-override-global-agent.js fails with the
following error when configured --without-ssl:

Error [ERR_NO_CRYPTO]:
Node.js is not compiled with OpenSSL crypto support
  at Object.assertCrypto (internal/util.js:101:11)
  ...
  at Object.<anonymous>
  (/node/test/parallel/test-https-client-override-global-agent.js:5:15)

This commit moves the require statement to after the crypto check.

PR-URL: #25388
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: James M Snell <[email protected]>

addaleax pushed a commit that referenced this pull request


          tools: update crypo check rule

2a85cc7

This commit updates the custom crypto-check ESLint rule to
detect require() calls that come before any hasCrypto
checks.

PR-URL: #25399
Refs: #25388
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Daniel Bevenius <[email protected]>

BridgeAR mentioned this pull request

v11.7.0 proposal #25537

Merged

BridgeAR pushed a commit to BridgeAR/node that referenced this pull request


          test: move require('https') to after crypto check

b76c968

Currently, test-https-client-override-global-agent.js fails with the
following error when configured --without-ssl:

Error [ERR_NO_CRYPTO]:
Node.js is not compiled with OpenSSL crypto support
  at Object.assertCrypto (internal/util.js:101:11)
  ...
  at Object.<anonymous>
  (/node/test/parallel/test-https-client-override-global-agent.js:5:15)

This commit moves the require statement to after the crypto check.

PR-URL: nodejs#25388
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: James M Snell <[email protected]>

BridgeAR pushed a commit to BridgeAR/node that referenced this pull request


          tools: update crypo check rule

bf66935

This commit updates the custom crypto-check ESLint rule to
detect require() calls that come before any hasCrypto
checks.

PR-URL: nodejs#25399
Refs: nodejs#25388
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Daniel Bevenius <[email protected]>

MylesBorins mentioned this pull request

v11.8.0 proposal #25687

Merged

BethGriggs pushed a commit that referenced this pull request


          test: move require('https') to after crypto check

d738fe8

Currently, test-https-client-override-global-agent.js fails with the
following error when configured --without-ssl:

Error [ERR_NO_CRYPTO]:
Node.js is not compiled with OpenSSL crypto support
  at Object.assertCrypto (internal/util.js:101:11)
  ...
  at Object.<anonymous>
  (/node/test/parallel/test-https-client-override-global-agent.js:5:15)

This commit moves the require statement to after the crypto check.

PR-URL: #25388
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: James M Snell <[email protected]>

BethGriggs pushed a commit that referenced this pull request


          tools: update crypo check rule

05b71f1

This commit updates the custom crypto-check ESLint rule to
detect require() calls that come before any hasCrypto
checks.

PR-URL: #25399
Refs: #25388
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Daniel Bevenius <[email protected]>

BethGriggs mentioned this pull request

v10.16.0 proposal #27514

Merged

BethGriggs pushed a commit that referenced this pull request


          test: move require('https') to after crypto check

38b8c19

Currently, test-https-client-override-global-agent.js fails with the
following error when configured --without-ssl:

Error [ERR_NO_CRYPTO]:
Node.js is not compiled with OpenSSL crypto support
  at Object.assertCrypto (internal/util.js:101:11)
  ...
  at Object.<anonymous>
  (/node/test/parallel/test-https-client-override-global-agent.js:5:15)

This commit moves the require statement to after the crypto check.

PR-URL: #25388
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: James M Snell <[email protected]>

BethGriggs pushed a commit that referenced this pull request


          tools: update crypo check rule

e7e0a1f

This commit updates the custom crypto-check ESLint rule to
detect require() calls that come before any hasCrypto
checks.

PR-URL: #25399
Refs: #25388
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Daniel Bevenius <[email protected]>

MylesBorins pushed a commit that referenced this pull request


          test: move require('https') to after crypto check

c8153ce

Currently, test-https-client-override-global-agent.js fails with the
following error when configured --without-ssl:

Error [ERR_NO_CRYPTO]:
Node.js is not compiled with OpenSSL crypto support
  at Object.assertCrypto (internal/util.js:101:11)
  ...
  at Object.<anonymous>
  (/node/test/parallel/test-https-client-override-global-agent.js:5:15)

This commit moves the require statement to after the crypto check.

PR-URL: #25388
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: James M Snell <[email protected]>

MylesBorins pushed a commit that referenced this pull request


          tools: update crypo check rule

21500a8

This commit updates the custom crypto-check ESLint rule to
detect require() calls that come before any hasCrypto
checks.

PR-URL: #25399
Refs: #25388
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Daniel Bevenius <[email protected]>

This was referenced May 29, 2019

2019-05-28 Version 10.16.0 'Dubnium' (LTS) @BethGriggs beevelop/docker-android-nodejs#315

Closed

2019-05-28 Version 10.16.0 'Dubnium' (LTS) @BethGriggs beevelop/docker-android-nodejs#316

Closed

2019-05-28 Version 10.16.0 'Dubnium' (LTS) @BethGriggs beevelop/docker-android-nodejs#317

Closed

2019-05-28 Version 10.16.0 'Dubnium' (LTS) @BethGriggs beevelop/docker-android-nodejs#318

Closed

2019-05-28 Version 10.16.0 'Dubnium' (LTS) @BethGriggs beevelop/docker-android-nodejs#319

Closed

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

Labels

test