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

test: fix test-tls-client-mindhsize for OpenSSL32 #54739

Closed

Conversation

mhdawson
Copy link
Member

@mhdawson mhdawson commented Sep 3, 2024

Refs: #53382

  • OpenSSL32 has a minimum dh key size by 2048 by default.
  • Create larter 3072 dh key needed for testing and adjust tests to use it for builds with OpenSSL32

@nodejs-github-bot nodejs-github-bot added needs-ci PRs that need a full CI run. test Issues and PRs related to the tests. labels Sep 3, 2024
@mhdawson
Copy link
Member Author

mhdawson commented Sep 3, 2024

Copy link

codecov bot commented Sep 3, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 88.06%. Comparing base (1d2603b) to head (f64e597).
Report is 9 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main   #54739      +/-   ##
==========================================
- Coverage   88.06%   88.06%   -0.01%     
==========================================
  Files         651      651              
  Lines      183386   183386              
  Branches    35800    35796       -4     
==========================================
- Hits       161504   161491      -13     
- Misses      15159    15162       +3     
- Partials     6723     6733      +10     

see 31 files with indirect coverage changes

@mhdawson mhdawson added the request-ci Add this label to start a Jenkins CI on a PR. label Sep 3, 2024
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Sep 3, 2024
@nodejs-github-bot

This comment was marked as outdated.

@nodejs-github-bot

This comment was marked as outdated.

@nodejs-github-bot

This comment was marked as outdated.

@nodejs-github-bot

This comment was marked as outdated.

@nodejs-github-bot

This comment was marked as outdated.

@nodejs-github-bot

This comment was marked as outdated.

@nodejs-github-bot

This comment was marked as outdated.

@nodejs-github-bot

This comment was marked as outdated.

@nodejs-github-bot

This comment was marked as outdated.

@nodejs-github-bot

This comment was marked as outdated.

@nodejs-github-bot

This comment was marked as outdated.

@nodejs-github-bot

This comment was marked as outdated.

@nodejs-github-bot

This comment was marked as outdated.

@jasnell
Copy link
Member

jasnell commented Sep 9, 2024

This PR is currently blocked from landing due to unreliable CI.

@mhdawson
Copy link
Member Author

mhdawson commented Sep 9, 2024

@jasnell was going to rebase and try again as I'd seen some PRs mark more tests as flaky.

@mhdawson mhdawson force-pushed the increase-dh-sizes-for-openssl21 branch from 104de8a to 1a1f9ab Compare September 9, 2024 14:38
@mhdawson
Copy link
Member Author

mhdawson commented Sep 9, 2024

Rebased hoping to get more PRs where tests were marked as flaky

@mhdawson mhdawson added the request-ci Add this label to start a Jenkins CI on a PR. label Sep 9, 2024
@nodejs-github-bot

This comment was marked as outdated.

@nodejs-github-bot

This comment was marked as outdated.

@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Sep 9, 2024
@nodejs-github-bot

This comment was marked as outdated.

@nodejs-github-bot
Copy link
Collaborator

@nodejs-github-bot
Copy link
Collaborator

@nodejs-github-bot
Copy link
Collaborator

@nodejs-github-bot
Copy link
Collaborator

Refs: nodejs#53382

- OpenSSL32 has a minimum dh key size by 2048 by
  default.
- Create larter 3072 dh key needed for testing and
  adjust tests to use it for builds with OpenSSL32

Signed-off-by: Michael Dawson <midawson@redhat.com>
@mhdawson mhdawson force-pushed the increase-dh-sizes-for-openssl21 branch from 1a1f9ab to f64e597 Compare September 11, 2024 15:57
@mhdawson
Copy link
Member Author

rebased again as tests that were failing were already marked as flaky.

@nodejs-github-bot
Copy link
Collaborator

@mhdawson mhdawson added the request-ci Add this label to start a Jenkins CI on a PR. label Sep 11, 2024
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Sep 11, 2024
@nodejs-github-bot
Copy link
Collaborator

@mhdawson mhdawson added the commit-queue Add this label to land a pull request using GitHub Actions. label Sep 12, 2024
@nodejs-github-bot nodejs-github-bot added commit-queue-failed An error occurred while landing this pull request using GitHub Actions. and removed commit-queue Add this label to land a pull request using GitHub Actions. labels Sep 12, 2024
@nodejs-github-bot
Copy link
Collaborator

Commit Queue failed
- Loading data for nodejs/node/pull/54739
✔  Done loading data for nodejs/node/pull/54739
----------------------------------- PR info ------------------------------------
Title      test: fix test-tls-client-mindhsize for OpenSSL32 (#54739)
Author     Michael Dawson <midawson@redhat.com> (@mhdawson)
Branch     mhdawson:increase-dh-sizes-for-openssl21 -> nodejs:main
Labels     test, needs-ci
Commits    1
 - test: fix test-tls-client-mindhsize for OpenSSL32
Committers 1
 - Michael Dawson <midawson@redhat.com>
PR-URL: https://github.com/nodejs/node/pull/54739
Refs: https://github.com/nodejs/node/issues/53382
Reviewed-By: Richard Lau <rlau@redhat.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
------------------------------ Generated metadata ------------------------------
PR-URL: https://github.com/nodejs/node/pull/54739
Refs: https://github.com/nodejs/node/issues/53382
Reviewed-By: Richard Lau <rlau@redhat.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
--------------------------------------------------------------------------------
   ⚠  Commits were pushed since the last approving review:
   ⚠  - test: fix test-tls-client-mindhsize for OpenSSL32
   ℹ  This PR was created on Tue, 03 Sep 2024 16:32:09 GMT
   ✔  Approvals: 3
   ✔  - Richard Lau (@richardlau) (TSC): https://github.com/nodejs/node/pull/54739#pullrequestreview-2277959625
   ✔  - Luigi Pinca (@lpinca): https://github.com/nodejs/node/pull/54739#pullrequestreview-2277994160
   ✔  - James M Snell (@jasnell) (TSC): https://github.com/nodejs/node/pull/54739#pullrequestreview-2283639489
   ✔  Last GitHub CI successful
   ℹ  Last Full PR CI on 2024-09-11T18:38:03Z: https://ci.nodejs.org/job/node-test-pull-request/62337/
- Querying data for job/node-test-pull-request/62337/
   ✔  Last Jenkins CI successful
--------------------------------------------------------------------------------
   ✔  Aborted `git node land` session in /home/runner/work/node/node/.ncu
https://github.com/nodejs/node/actions/runs/10823337922

jasnell pushed a commit that referenced this pull request Sep 12, 2024
Refs: #53382

- OpenSSL32 has a minimum dh key size by 2048 by
  default.
- Create larter 3072 dh key needed for testing and
  adjust tests to use it for builds with OpenSSL32

Signed-off-by: Michael Dawson <midawson@redhat.com>
PR-URL: #54739
Reviewed-By: Richard Lau <rlau@redhat.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
@jasnell
Copy link
Member

jasnell commented Sep 12, 2024

Landed in d9ca8b0

@jasnell jasnell closed this Sep 12, 2024
aduh95 pushed a commit that referenced this pull request Sep 12, 2024
Refs: #53382

- OpenSSL32 has a minimum dh key size by 2048 by
  default.
- Create larter 3072 dh key needed for testing and
  adjust tests to use it for builds with OpenSSL32

Signed-off-by: Michael Dawson <midawson@redhat.com>
PR-URL: #54739
Reviewed-By: Richard Lau <rlau@redhat.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
@mhdawson
Copy link
Member Author

@james thanks for landing

aduh95 pushed a commit that referenced this pull request Sep 13, 2024
Refs: #53382

- OpenSSL32 has a minimum dh key size by 2048 by
  default.
- Create larter 3072 dh key needed for testing and
  adjust tests to use it for builds with OpenSSL32

Signed-off-by: Michael Dawson <midawson@redhat.com>
PR-URL: #54739
Reviewed-By: Richard Lau <rlau@redhat.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
aduh95 pushed a commit that referenced this pull request Sep 13, 2024
Refs: #53382

- OpenSSL32 has a minimum dh key size by 2048 by
  default.
- Create larter 3072 dh key needed for testing and
  adjust tests to use it for builds with OpenSSL32

Signed-off-by: Michael Dawson <midawson@redhat.com>
PR-URL: #54739
Reviewed-By: Richard Lau <rlau@redhat.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
@RafaelGSS RafaelGSS mentioned this pull request Sep 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
commit-queue-failed An error occurred while landing this pull request using GitHub Actions. 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.

5 participants