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, crypto: use correct object on assert #51820

Merged
merged 1 commit into from
May 12, 2024
Merged

test, crypto: use correct object on assert #51820

merged 1 commit into from
May 12, 2024

Conversation

xicilion
Copy link
Contributor

The test case for KeyObject does not really test the creation of asymmetric cryptographic keys using jwk because of a misspelling of the variable.

@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 Feb 21, 2024
@lpinca lpinca added the request-ci Add this label to start a Jenkins CI on a PR. label Feb 21, 2024
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Feb 21, 2024
@nodejs-github-bot
Copy link
Collaborator

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

H4ad
H4ad approved these changes Feb 21, 2024
View reviewed changes
Copy link
Member

@H4ad H4ad left a comment

Choose a reason for hiding this comment

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

Can you please change the commit message to something like: crypto: use correct object on assert

It should start with crypto and the first word should not be in the past.

@xicilion xicilion changed the title test, fix tests for KeyObject. crypto: use correct object on assert Feb 22, 2024
@lpinca lpinca added the request-ci Add this label to start a Jenkins CI on a PR. label Feb 22, 2024
@lpinca
Copy link
Member

lpinca commented Feb 22, 2024

The correct subsystem is test. You can use test, crypto: if wanted. The length of each line in the commit message should not be longer than 72 characters.

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

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

@xicilion xicilion changed the title crypto: use correct object on assert test, crypto: use correct object on assert Feb 22, 2024
@lpinca
Copy link
Member

lpinca commented Feb 22, 2024

@xicilion can you please amend the commit message again? It should look like this:

test, crypto: use correct object on assert

The test case for KeyObject does not really test the creation of
asymmetric cryptographic keys using jwk because of a misspelling of
the variable.

@xicilion
test, crypto: use correct object on assert
15294ca 
The test case for KeyObject does not really test the creation of
asymmetric cryptographic keys using jwk because of a misspelling of
the variable.
@lpinca lpinca added the request-ci Add this label to start a Jenkins CI on a PR. label Feb 22, 2024
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Feb 22, 2024
@nodejs-github-bot
Copy link
Collaborator

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

@nodejs-github-bot
Copy link
Collaborator

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

@nodejs-github-bot
Copy link
Collaborator

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

@nodejs-github-bot
Copy link
Collaborator

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

@nodejs-github-bot
Copy link
Collaborator

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

@nodejs-github-bot
Copy link
Collaborator

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

@xicilion
Copy link
Contributor Author

xicilion commented Feb 26, 2024 •
edited

Why is this pr generating new CI over and over again?

@lpinca
Copy link
Member

lpinca commented Feb 26, 2024

We can't land this without a green CI and there are flaky tests.

@scooby22400 scooby22400 mentioned this pull request Mar 4, 2024
Closed
@nodejs-github-bot
Copy link
Collaborator

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

@nodejs-github-bot
Copy link
Collaborator

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

@nodejs-github-bot
Copy link
Collaborator

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

@aduh95 aduh95 added author ready PRs that have at least one approval, no pending requests for changes, and a CI started. request-ci Add this label to start a Jenkins CI on a PR. labels May 11, 2024
@github-actions github-actions bot added request-ci-failed An error occurred while starting CI via request-ci label, and manual interventon is needed. and removed request-ci Add this label to start a Jenkins CI on a PR. labels May 11, 2024
@github-actions GitHub Actions
Copy link
Contributor

Failed to start CI
   ⚠  Something was pushed to the Pull Request branch since the last approving review.
   ✘  Refusing to run CI on potentially unsafe PR
https://github.com/nodejs/node/actions/runs/9044713310

@H4ad H4ad added request-ci Add this label to start a Jenkins CI on a PR. and removed request-ci-failed An error occurred while starting CI via request-ci label, and manual interventon is needed. labels May 11, 2024
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label May 11, 2024
@nodejs-github-bot
Copy link
Collaborator

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

@nodejs-github-bot
Copy link
Collaborator

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

@nodejs-github-bot
Copy link
Collaborator

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

@aduh95 aduh95 added the commit-queue Add this label to land a pull request using GitHub Actions. label May 12, 2024
@nodejs-github-bot nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label May 12, 2024
@nodejs-github-bot nodejs-github-bot merged commit 91d30f3 into nodejs:main May 12, 2024
64 checks passed
@nodejs-github-bot
Copy link
Collaborator

Landed in 91d30f3

targos pushed a commit that referenced this pull request May 13, 2024
@xicilion @targos
test, crypto: use correct object on assert
944ae59 
The test case for KeyObject does not really test the creation of
asymmetric cryptographic keys using jwk because of a misspelling of
the variable.

PR-URL: #51820
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Vinícius Lourenço Claro Cardoso <contact@viniciusl.com.br>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
@targos targos mentioned this pull request May 13, 2024
Merged
lukins-cz pushed a commit to lukins-cz/OS-Aplet-node that referenced this pull request Jun 1, 2024
@xicilion @lukins-cz
test, crypto: use correct object on assert
f1f24af 
The test case for KeyObject does not really test the creation of
asymmetric cryptographic keys using jwk because of a misspelling of
the variable.

PR-URL: nodejs#51820
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Vinícius Lourenço Claro Cardoso <contact@viniciusl.com.br>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@lpinca lpinca lpinca approved these changes

@tniessen tniessen tniessen approved these changes

@H4ad H4ad H4ad approved these changes

Assignees
No one assigned
Labels
author ready PRs that have at least one approval, no pending requests for changes, and a CI started. needs-ci PRs that need a full CI run. test Issues and PRs related to the tests.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
6 participants
@xicilion @nodejs-github-bot @lpinca @tniessen @H4ad @aduh95