FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

test: move require('https') to after crypto check by danbev · Pull Request #25388 · nodejs/node · GitHub

/ node Public

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

Closed
danbev wants to merge 2 commits into
nodejs:masterfrom
danbev:https-client-override-crypto-check
ConversationCommits (2)ChecksFiles changed
Closed

test: move require('https') to after crypto check#25388
danbev wants to merge 2 commits into
nodejs:masterfrom
danbev:https-client-override-crypto-check

Conversation

danbev commented Jan 8, 2019

Copy link
Copy Markdown
Contributor

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.

nodejs-github-bot commented Jan 8, 2019

Copy link
Copy Markdown
Collaborator

@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 Issues and PRs related to the tests. label Jan 8, 2019

danbev commented Jan 8, 2019

Copy link
Copy Markdown
Contributor Author

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

Trott approved these changes Jan 8, 2019
View reviewed changes

Trott left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

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.

danbev commented Jan 8, 2019

Copy link
Copy Markdown
Contributor Author

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

Sounds good, I'll update shortly. Thanks

danbev commented Jan 8, 2019 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

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

cjihrig mentioned this pull request Jan 8, 2019
Closed
3 tasks

danbev commented Jan 11, 2019

Copy link
Copy Markdown
Contributor Author

Landed in 9abb646.

danbev closed this Jan 11, 2019
danbev deleted the https-client-override-crypto-check branch January 11, 2019 04:38
danbev added a commit that referenced this pull request Jan 11, 2019
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 <rtrott@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
danbev pushed a commit that referenced this pull request Jan 11, 2019
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 <riclau@uk.ibm.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com>
addaleax pushed a commit that referenced this pull request Jan 14, 2019
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 <rtrott@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
addaleax pushed a commit that referenced this pull request Jan 14, 2019
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 <riclau@uk.ibm.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com>
BridgeAR mentioned this pull request Jan 16, 2019
Merged
BridgeAR pushed a commit to BridgeAR/node that referenced this pull request Jan 16, 2019
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 <rtrott@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
BridgeAR pushed a commit to BridgeAR/node that referenced this pull request Jan 16, 2019
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 <riclau@uk.ibm.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com>
MylesBorins mentioned this pull request Jan 24, 2019
Merged
BethGriggs pushed a commit that referenced this pull request Apr 28, 2019
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 <rtrott@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
BethGriggs pushed a commit that referenced this pull request Apr 28, 2019
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 <riclau@uk.ibm.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com>
BethGriggs mentioned this pull request May 1, 2019
Merged
BethGriggs pushed a commit that referenced this pull request May 10, 2019
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 <rtrott@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
BethGriggs pushed a commit that referenced this pull request May 10, 2019
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 <riclau@uk.ibm.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com>
MylesBorins pushed a commit that referenced this pull request May 16, 2019
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 <rtrott@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
MylesBorins pushed a commit that referenced this pull request May 16, 2019
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 <riclau@uk.ibm.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com>
This was referenced May 29, 2019
Closed
Closed
Closed
Closed
Closed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

jasnell jasnell approved these changes
Trott Trott approved these changes
cjihrig cjihrig approved these changes
richardlau richardlau approved these changes

Assignees

No one assigned

Labels

test Issues and PRs related to the tests.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

Back | FazBrowse Home | New Git URL