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.

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

quictls/openssl 3.0.0-alpha-15 tests #38451

Merged
merged 2 commits into from
May 6, 2021

Conversation

danbev
Copy link
Contributor

@danbev danbev commented Apr 28, 2021

This pull request contains to commits related to updating to quictls/openssl 3.0.0-alpha-15.

Fixes: #38373

@github-actions github-actions bot added needs-ci PRs that need a full CI run. test Issues and PRs related to the tests. labels Apr 28, 2021
@danbev
Copy link
Contributor Author

danbev commented Apr 28, 2021

@richardlau This should enable us to dynamically link with quictls/openssl 3.0.0-alpha-15. I've tested this locally and I'm able to get all the tests to pass and hopefully this will enable us to update the CI job.

@danbev danbev changed the title Openssl 3.0.0 alpha15 tests quictls/openssl 3.0.0-alpha-15 tests Apr 28, 2021
@richardlau
Copy link
Member

@danbev With this PR I'm down to one failure when testing locally with openssl3.0.0-alpha15+quic, benchmark/test-benchmark-crypto:

=== release test-benchmark-crypto ===
Path: benchmark/test-benchmark-crypto
--- stderr ---
node:internal/crypto/keys:637
    handle.init(kKeyTypePrivate, data, format, type, passphrase);
           ^

Error: error:068000A8:asn1 encoding routines::wrong tag
    at Object.createPrivateKey (node:internal/crypto/keys:637:12)
    at getKeyObject (/home/iojs/node/benchmark/crypto/oneshot-sign-verify.js:22:24)
    at Array.map (<anonymous>)
    at main (/home/iojs/node/benchmark/crypto/oneshot-sign-verify.js:109:23)
    at /home/iojs/node/benchmark/common.js:42:9
    at processTicksAndRejections (node:internal/process/task_queues:78:11) {
  opensslErrorStack: [
    'error:0688010A:asn1 encoding routines::nested asn1 error',
    'error:0688010A:asn1 encoding routines::nested asn1 error'
  ],
  library: 'asn1 encoding routines',
  reason: 'wrong tag',
  code: 'ERR_OSSL_ASN1_WRONG_TAG'
}
node:assert:123
  throw new AssertionError(obj);
  ^

AssertionError [ERR_ASSERTION]: Expected values to be strictly equal:

1 !== 0

    at ChildProcess.<anonymous> (/home/iojs/node/test/common/benchmark.js:30:12)
    at ChildProcess.emit (node:events:365:28)
    at Process.ChildProcess._handle.onexit (node:internal/child_process:290:12) {
  generatedMessage: true,
  code: 'ERR_ASSERTION',
  actual: 1,
  expected: 0,
  operator: 'strictEqual'
}
Command: out/Release/node /home/iojs/node/test/benchmark/test-benchmark-crypto.js

@danbev
Copy link
Contributor Author

danbev commented Apr 28, 2021

With this PR I'm down to one failure when testing locally with openssl3.0.0-alpha15+quic, benchmark/test-benchmark-crypto:

Ah sorry about that, I only ran the test target. I'll run test-benchmark and update the test.

@richardlau
Copy link
Member

Tests are all passing now. Thanks.

@richardlau
Copy link
Member

@danbev any reason for this to remain in draft?

@danbev
Copy link
Contributor Author

danbev commented Apr 30, 2021

@danbev any reason for this to remain in draft?

Not really, I was only concerned about it breaking until we upgrade to alpha-15 but I'll change it now.

@danbev danbev marked this pull request as ready for review April 30, 2021 04:09
@nodejs-github-bot
Copy link
Collaborator

@richardlau
Copy link
Member

@danbev any reason for this to remain in draft?

Not really, I was only concerned about it breaking until we upgrade to alpha-15 but I'll change it now.

Ah didn't realise this breaks on openssl 3.0.0-alpha14 😞. This will need some coordination to not break all the other open PRs.

@Trott
Copy link
Member

Trott commented May 2, 2021

That's a lot of skipped tests. Is the idea that when it's out of alpha, they won't be skipped anymore?

@danbev
Copy link
Contributor Author

danbev commented May 3, 2021

That's a lot of skipped tests. Is the idea that when it's out of alpha, they won't be skipped anymore?

We should be able to enable these again once #15067 is in a new release which will hopefully be in the next alpha (or beta) release of OpenSSL.

@nodejs-github-bot
Copy link
Collaborator

@nodejs-github-bot
Copy link
Collaborator

@richardlau
Copy link
Member

Does anyone know how to get the actions to run? Do we need to force push to this PR to trigger them?

@danbev
Copy link
Contributor Author

danbev commented May 6, 2021

Does anyone know how to get the actions to run?

I'm not sure but I'll rebase this now and that should run them.

@danbev danbev force-pushed the openssl-3.0.0-alpha15-tests branch from 8524e0d to 3cd3b58 Compare May 6, 2021 07:02
@nodejs-github-bot
Copy link
Collaborator

@richardlau
Copy link
Member

Thanks. As previously mentioned this PR needs to be coordinated with nodejs/build#2644 as it makes the tests break on openssl 3.0.0-alpha14+quic (currently used on the CI). My plan is to get CI runs on this PR that are passing except for the openssl300 build (expected to fail) and then update the build containers and resume (which should result in the openssl300 build passing) before landing.

@richardlau richardlau self-assigned this May 6, 2021
@nodejs-github-bot
Copy link
Collaborator

@nodejs-github-bot
Copy link
Collaborator

@nodejs-github-bot
Copy link
Collaborator

danbev added 2 commits May 6, 2021 10:19
PR-URL: nodejs#38451
Fixes: nodejs#38373
Reviewed-By: Richard Lau <rlau@redhat.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
This commit skips some test when OpenSSL 3.0.0-alpha15 is used as there
is an issue that causes them to fail.

This is only a temp solution until there is new OpenSSL release.

Fixes: nodejs#38373

PR-URL: nodejs#38451
Reviewed-By: Richard Lau <rlau@redhat.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
@richardlau richardlau force-pushed the openssl-3.0.0-alpha15-tests branch from 3cd3b58 to 2ff93c8 Compare May 6, 2021 14:29
@richardlau
Copy link
Member

Landed in aed17e9...2ff93c8.

@richardlau richardlau merged commit 2ff93c8 into nodejs:master May 6, 2021
richardlau added a commit to nodejs/build that referenced this pull request May 6, 2021
richardlau pushed a commit to richardlau/node-1 that referenced this pull request May 14, 2021
PR-URL: nodejs#38451
Fixes: nodejs#38373
Reviewed-By: Richard Lau <rlau@redhat.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
richardlau pushed a commit to richardlau/node-1 that referenced this pull request May 14, 2021
This commit skips some test when OpenSSL 3.0.0-alpha15 is used as there
is an issue that causes them to fail.

This is only a temp solution until there is new OpenSSL release.

Fixes: nodejs#38373

PR-URL: nodejs#38451
Reviewed-By: Richard Lau <rlau@redhat.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs-ci PRs that need a full CI run. test Issues and PRs related to the tests.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Test failures with openssl 3.0.0 alpha 15 (quictls fork)
7 participants