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: improve crypto coverage #11279

Closed

Conversation

toiroakr
Copy link

This PR improves test coverage on crypto dh functions.
Just add 2 exceptions check.
Please review.

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

N/A

@nodejs-github-bot nodejs-github-bot added the test Issues and PRs related to the tests. label Feb 10, 2017
@hiroppy hiroppy added the crypto Issues and PRs related to the crypto subsystem. label Feb 10, 2017
@Trott
Copy link
Member

Trott commented Feb 10, 2017

If possible, it would be great if the regular expressions could cover the entire error message. For example, instead of /Bad format: 10/, use /^TypeError: Bad format: 10$/.

@Trott
Copy link
Member

Trott commented Feb 10, 2017

@nodejs/testing

@toiroakr toiroakr force-pushed the test-improve-crypto-coverage branch from c2701df to ccc74df Compare February 10, 2017 05:12
@toiroakr
Copy link
Author

@Trott Thanks! fixed.

// invalid test: curve argument is undefined
assert.throws(() => {
crypto.createECDH();
}, /"curve" argument should be a string/);
Copy link
Member

@hiroppy hiroppy Feb 10, 2017

Choose a reason for hiding this comment

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

You can change this line to /^TypeError: "curve" argument should be a string$/.

- check exception when ECDH curve is undefined
- check exception when getPublicKey format is invalid
@toiroakr toiroakr force-pushed the test-improve-crypto-coverage branch from ccc74df to b95d606 Compare February 10, 2017 05:39
@toiroakr
Copy link
Author

toiroakr commented Feb 10, 2017

@abouthiroppy Thanks! and sorry for letting you point out the same thing. I fixed it.

Copy link
Member

@hiroppy hiroppy left a comment

Choose a reason for hiding this comment

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

Thank you for your contribution :)

@hiroppy
Copy link
Member

hiroppy commented Feb 10, 2017

jasnell pushed a commit that referenced this pull request Feb 17, 2017
* check exception when ECDH curve is undefined
* check exception when getPublicKey format is invalid

PR-URL: #11279
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Yuta Hiroto <hello@about-hiroppy.com>
Reviewed-By: Santiago Gimeno <santiago.gimeno@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
@jasnell
Copy link
Member

jasnell commented Feb 17, 2017

Landed on 513d9bb

@jasnell jasnell closed this Feb 17, 2017
italoacasas pushed a commit to italoacasas/node that referenced this pull request Feb 20, 2017
* check exception when ECDH curve is undefined
* check exception when getPublicKey format is invalid

PR-URL: nodejs#11279
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Yuta Hiroto <hello@about-hiroppy.com>
Reviewed-By: Santiago Gimeno <santiago.gimeno@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
italoacasas pushed a commit that referenced this pull request Feb 22, 2017
* check exception when ECDH curve is undefined
* check exception when getPublicKey format is invalid

PR-URL: #11279
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Yuta Hiroto <hello@about-hiroppy.com>
Reviewed-By: Santiago Gimeno <santiago.gimeno@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
@italoacasas italoacasas mentioned this pull request Feb 25, 2017
jasnell pushed a commit that referenced this pull request Mar 7, 2017
* check exception when ECDH curve is undefined
* check exception when getPublicKey format is invalid

PR-URL: #11279
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Yuta Hiroto <hello@about-hiroppy.com>
Reviewed-By: Santiago Gimeno <santiago.gimeno@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
@jasnell
Copy link
Member

jasnell commented Mar 7, 2017

will need a backport PR to land on v4

MylesBorins pushed a commit that referenced this pull request Mar 9, 2017
* check exception when ECDH curve is undefined
* check exception when getPublicKey format is invalid

PR-URL: #11279
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Yuta Hiroto <hello@about-hiroppy.com>
Reviewed-By: Santiago Gimeno <santiago.gimeno@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
@MylesBorins MylesBorins mentioned this pull request Mar 9, 2017
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.

7 participants