JoshKaufman / ursa

URSA - RSA public/private key OpenSSL bindings for Node.js
Other
619 stars 135 forks source link

converting over to use mocha for the test harness #155

Closed mansona closed 7 years ago

mansona commented 7 years ago

Fixes #108

mansona commented 7 years ago

Howdy 👋

I know it's been a while since I offered to swap this repo over to using Mocha in #108 but I finally got around to doing it 🎉 You can see the output now lists all of the tests in Travis: https://travis-ci.org/quartzjer/ursa/jobs/202954687

One thing that I did notice through this implementation is that there was a function in the native test file that was never being called: test_getPrivateKeyPemWithPassPhrase. When I added it as a proper test it was creating a segfault so I marked it as a skipped test it.skip() which now shows up clearly in the test output.

Just to be clear this test was never running and I have just surfaced that information 😂

Now if you want to add tests or develop with this repo you can run npm run test-watch which will watch for changes to your files and re-run the tests.

Let me know if you have any questions.

mansona commented 7 years ago

Also now that this is using Mocha it is trivial to enable code-coverage analysis via CodeClimate if that is something you were interested in. Please let me know if you would like me to help with that (you would need to sign up as the owner of the repo I think) 👍

quartzjer commented 7 years ago

@mansona I can no longer maintain this module and haven't found someone else to take it over yet, would you mind taking ownership?

JoshKaufman commented 7 years ago

@mansona I know its been a while since you worked on this 😅 but if you're still around how do you feel about this?

I'm going to compare/test it and merge so if you have anything to add 🗣

mansona commented 7 years ago

@JoshKaufman yes I still feel good about this PR, essentially it was just to make testing a little bit easier and more "modern"

Do you need me to bring it up to date or anything?

JoshKaufman commented 7 years ago

@mansona nah, I'm going to merge this and bump the npm versions separately as part of getting everything up to latest.