Skip to content

test: refactor test-crypto-pbkdf2 - #35693

Merged
Trott merged 1 commit into
nodejs:masterfrom
tniessen:test-refactor-test-crypto-pbkdf2
Oct 20, 2020
Merged

test: refactor test-crypto-pbkdf2#35693
Trott merged 1 commit into
nodejs:masterfrom
tniessen:test-refactor-test-crypto-pbkdf2

Conversation

@tniessen

Copy link
Copy Markdown
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

@tniessentniessen 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.

@tniessentniessen added the request-ci Add this label to start a Jenkins CI on a PR. label Oct 17, 2020
@github-actionsgithub-actionsBot 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
Copy Markdown
Collaborator

@TrottTrott 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
Trottforce-pushed the test-refactor-test-crypto-pbkdf2 branch from 0a3b260 to 9e227d5CompareOctober 20, 2020 11:58
@Trott

Copy link
Copy Markdown
Member

Landed in 9e227d5

@Trott
Trott merged commit 9e227d5 into nodejs:masterOct 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>
@BethGriggsBethGriggs mentioned this pull request Oct 21, 2020
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cryptoIssues and PRs related to the crypto subsystem.testIssues and PRs related to the tests.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants

@tniessen@codecov-io@nodejs-github-bot@Trott@jasnell@watilde@cjihrig@targos