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: refactor test-crypto-pbkdf2 #35693

Merged
merged 1 commit into from Oct 20, 2020

Conversation

tniessen
Copy link
Member

This simplifies the test case by introducing a helper function that simply verifies that the sync and async variants produce the same result.

Also, testPBKDF2 did not use mustCall()/mustSucceed(), so it would not fail if the callback was never called.

This effectively also changes the loose mustCall() check at the end of the file to the stricter mustSucceed() function.

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • tests and/or benchmarks are included
  • commit message follows commit guidelines

@tniessen tniessen added crypto Issues and PRs related to the crypto subsystem. test Issues and PRs related to the tests. labels Oct 17, 2020
@codecov-io

This comment has been minimized.

@tniessen tniessen added the request-ci Add this label to start a Jenkins CI on a PR. label Oct 17, 2020
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Oct 17, 2020
@nodejs-github-bot

This comment has been minimized.

@nodejs-github-bot

This comment has been minimized.

@nodejs-github-bot
Copy link
Collaborator

@Trott Trott added request-ci Add this label to start a Jenkins CI on a PR. and removed request-ci Add this label to start a Jenkins CI on a PR. labels Oct 18, 2020
PR-URL: nodejs#35693
Reviewed-By: Daijiro Wachi <daijiro.wachi@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
@Trott Trott force-pushed the test-refactor-test-crypto-pbkdf2 branch from 0a3b260 to 9e227d5 Compare October 20, 2020 11:58
@Trott
Copy link
Member

Trott commented Oct 20, 2020

Landed in 9e227d5

@Trott Trott merged commit 9e227d5 into nodejs:master Oct 20, 2020
BethGriggs pushed a commit that referenced this pull request Oct 21, 2020
PR-URL: #35693
Reviewed-By: Daijiro Wachi <daijiro.wachi@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
@BethGriggs BethGriggs mentioned this pull request Oct 21, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
crypto Issues and PRs related to the crypto subsystem. test Issues and PRs related to the tests.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

8 participants