Skip to content
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: add crypto check to http2 tests #14657

Closed
wants to merge 1 commit into from
Closed

test: add crypto check to http2 tests #14657

wants to merge 1 commit into from

Conversation

danbev
Copy link
Contributor

@danbev danbev commented Aug 7, 2017

When building --without-ssl and running the tests some of the http2 test
fail with the following error message:

internal/util.js:82
    throw new errors.Error('ERR_NO_CRYPTO');
    ^

Error [ERR_NO_CRYPTO]: Node.js is not compiled with OpenSSL crypto
support
    at Object.assertCrypto (internal/util.js:82:11)
    at internal/http2/core.js:5:26
    at NativeModule.compile (bootstrap_node.js:586:7)
    at NativeModule.require (bootstrap_node.js:531:18)
    at http2.js:17:5
    at NativeModule.compile (bootstrap_node.js:586:7)
    at Function.NativeModule.require (bootstrap_node.js:531:18)
    at Function.Module._load (module.js:449:25)
    at Module.require (module.js:517:17)
    at require (internal/module.js:11:18)

This commit adds hasCrypto checks and skips the tests if there is no
crypto support.

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

test, http2

@danbev
test: add crypto check to http2 tests
9f23988 
When building --without-ssl and running the tests some of the http2 test
fail with the following error message:

internal/util.js:82
    throw new errors.Error('ERR_NO_CRYPTO');
    ^

Error [ERR_NO_CRYPTO]: Node.js is not compiled with OpenSSL crypto
support
    at Object.assertCrypto (internal/util.js:82:11)
    at internal/http2/core.js:5:26
    at NativeModule.compile (bootstrap_node.js:586:7)
    at NativeModule.require (bootstrap_node.js:531:18)
    at http2.js:17:5
    at NativeModule.compile (bootstrap_node.js:586:7)
    at Function.NativeModule.require (bootstrap_node.js:531:18)
    at Function.Module._load (module.js:449:25)
    at Module.require (module.js:517:17)
    at require (internal/module.js:11:18)

This commit adds hasCrypto checks and skips the tests if there is no
crypto support.
@nodejs-github-bot nodejs-github-bot added the test Issues and PRs related to the tests. label Aug 7, 2017
@mscdex mscdex added the http2 Issues or PRs related to the http2 subsystem. label Aug 7, 2017
Trott
Trott approved these changes Aug 7, 2017
View reviewed changes
Copy link
Member

@Trott Trott left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks for staying on top of the "do tests pass without crypto" thing. Wish we had a no-crypto and a no-icu test runner in CI.

@danbev
Copy link
Contributor Author

danbev commented Aug 7, 2017 •
edited
Loading

Wish we had a no-crypto and a no-icu test runner in CI.

@Trott I'm going to revisit #13813 this week and see if we can get it merged as it would pick up on most of these.

@danbev
Copy link
Contributor Author

danbev commented Aug 7, 2017

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

@refack
Copy link
Contributor

refack commented Aug 7, 2017

I'm going to revisit #13813 this week and see if we can get it merged as it would pick up on most of these.

can http2 function without openSSL at all?

@Trott
Copy link
Member

Trott commented Aug 7, 2017

can http2 function without openSSL at all?

Yes. Compare http2.createServer() with http2.createSecureServer().

@jasnell
Copy link
Member

jasnell commented Aug 8, 2017

red ci is unrelated.

jasnell pushed a commit that referenced this pull request Aug 8, 2017
@danbev @jasnell
test: add crypto check to http2 tests
1b719fe 
When building --without-ssl and running the tests some of the http2 test
fail with the following error message:

internal/util.js:82
    throw new errors.Error('ERR_NO_CRYPTO');
    ^

Error [ERR_NO_CRYPTO]: Node.js is not compiled with OpenSSL crypto
support
    at Object.assertCrypto (internal/util.js:82:11)
    at internal/http2/core.js:5:26
    at NativeModule.compile (bootstrap_node.js:586:7)
    at NativeModule.require (bootstrap_node.js:531:18)
    at http2.js:17:5
    at NativeModule.compile (bootstrap_node.js:586:7)
    at Function.NativeModule.require (bootstrap_node.js:531:18)
    at Function.Module._load (module.js:449:25)
    at Module.require (module.js:517:17)
    at require (internal/module.js:11:18)

This commit adds hasCrypto checks and skips the tests if there is no
crypto support.

PR-URL: #14657
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Tobias Nießen <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Refael Ackermann <[email protected]>
Reviewed-By: James M Snell <[email protected]>
@jasnell
Copy link
Member

jasnell commented Aug 8, 2017

Landed in 1b719fe

@jasnell jasnell closed this Aug 8, 2017
jasnell pushed a commit to jasnell/node that referenced this pull request Aug 13, 2017
@danbev @jasnell
test: add crypto check to http2 tests
38f55ea 
When building --without-ssl and running the tests some of the http2 test
fail with the following error message:

internal/util.js:82
    throw new errors.Error('ERR_NO_CRYPTO');
    ^

Error [ERR_NO_CRYPTO]: Node.js is not compiled with OpenSSL crypto
support
    at Object.assertCrypto (internal/util.js:82:11)
    at internal/http2/core.js:5:26
    at NativeModule.compile (bootstrap_node.js:586:7)
    at NativeModule.require (bootstrap_node.js:531:18)
    at http2.js:17:5
    at NativeModule.compile (bootstrap_node.js:586:7)
    at Function.NativeModule.require (bootstrap_node.js:531:18)
    at Function.Module._load (module.js:449:25)
    at Module.require (module.js:517:17)
    at require (internal/module.js:11:18)

This commit adds hasCrypto checks and skips the tests if there is no
crypto support.

PR-URL: nodejs#14657
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Tobias Nießen <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Refael Ackermann <[email protected]>
Reviewed-By: James M Snell <[email protected]>
addaleax pushed a commit that referenced this pull request Aug 14, 2017
@danbev @addaleax
test: add crypto check to http2 tests
dbb9c37 
When building --without-ssl and running the tests some of the http2 test
fail with the following error message:

internal/util.js:82
    throw new errors.Error('ERR_NO_CRYPTO');
    ^

Error [ERR_NO_CRYPTO]: Node.js is not compiled with OpenSSL crypto
support
    at Object.assertCrypto (internal/util.js:82:11)
    at internal/http2/core.js:5:26
    at NativeModule.compile (bootstrap_node.js:586:7)
    at NativeModule.require (bootstrap_node.js:531:18)
    at http2.js:17:5
    at NativeModule.compile (bootstrap_node.js:586:7)
    at Function.NativeModule.require (bootstrap_node.js:531:18)
    at Function.Module._load (module.js:449:25)
    at Module.require (module.js:517:17)
    at require (internal/module.js:11:18)

This commit adds hasCrypto checks and skips the tests if there is no
crypto support.

PR-URL: #14657
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Tobias Nießen <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Refael Ackermann <[email protected]>
Reviewed-By: James M Snell <[email protected]>
@addaleax addaleax mentioned this pull request Aug 14, 2017
Merged
@danbev danbev deleted the http2-crypto-checks branch November 16, 2017 08:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tniessen tniessen tniessen approved these changes

@cjihrig cjihrig cjihrig approved these changes

@lpinca lpinca lpinca approved these changes

@addaleax addaleax addaleax approved these changes

@Trott Trott Trott approved these changes

@jasnell jasnell jasnell approved these changes

@refack refack refack approved these changes

Assignees
No one assigned
Labels
http2 Issues or PRs related to the http2 subsystem. test Issues and PRs related to the tests.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.