test(prompt.js): enhance prompt.js test coverage#438
test(prompt.js): enhance prompt.js test coverage#438lwasser merged 2 commits intoall-contributors:mainfrom
Conversation
src/contributors/prompt.js
Outdated
| return inquirer.prompt(questions).then(_.assign(defaults)) | ||
| } | ||
|
|
||
| module.exports.getQuestions = getQuestions |
There was a problem hiding this comment.
/** @testonly */
i tried this but knip continues to flag these imports
There was a problem hiding this comment.
Interestingly, it's only flagging 1 or the two unused imports. I'm not sure why. @JoshuaKGoldberg, do you have any ideas why this might be getting flagged by Knip or what I'm doing wrong? many thanks!!
src/cli.js
Outdated
| } | ||
|
|
||
| run() | ||
| if (require.main === module) { |
There was a problem hiding this comment.
I want to be able to test the individual functions in these modules. if there is a better way i'm very interested in that. but this just allows us to explore addContribution but also use cli.js as a cli too - i'm not sure what the best way to do this is in general as knip does not like these exports that are test only .
|
Ok friends - this led me down a rabbit hole! I am stopping here, but we need more test coverage. I found a bunch of bugs in the CLI and fixed them here, but we need tests for the CLI itself, which I opened #439 for that effort. It will require some more mocking. Please test this pr locally to ensure the CLI runs as expected! My biggest issue right now is figuring out how to export functions for use in tests without making Knip angry. I tried Test coverage has increased a lot with this pr but codecov is failing it because there is still a lot of uncovered code. (specifically the cli.js module needs more tests. |
lwasser
force-pushed
the
prompt-tests
branch
4 times, most recently
from
f4fadc2 to
ffec72e
Compare
lwasser
force-pushed
the
prompt-tests
branch
from
ffec72e to
31811da
Compare
lwasser
force-pushed
the
prompt-tests
branch
from
8acdb3a to
a375166
Compare
|
Ok i'd love a review of this. This is my first time writing tests in node and it seems there are some foundational differences between how node vs python works - both in docstrings / documentation and also exports and imports. Everything is running now but I'd love a careful review to ensure this looks OK as is (or did I break some node conventions that I don't know about!!). Generally, do node developers not add docstrings? I ask because i'm used to docstrings that you can then automatically turn into api docs and also is exporting functions so we can test them ok? i've read different things about this too but unit tests seems important here for us. thank you for any and all feedback!! |
|
Thanks for the review @Berkmann18 !! i'll merge. |
3 participants
This PR enhances test coverage for prompt.js. I started here because there were issues in the CLI with prompts not working, and the module was largely uncovered by tests. This PR adds tests and fixes several bugs I identified along the way.
Checklist: