test(prompt.js): enhance prompt.js test coverage#438
test(prompt.js): enhance prompt.js test coverage#438lwasser wants to merge 6 commits intoall-contributors:mainfrom
Conversation
| 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!!
| // ensure the config file exists | ||
| await util.configFile.readConfig(argv.config) | ||
| const configData = await util.configFile.readConfig(argv.config) | ||
| Object.assign(argv, configData) |
There was a problem hiding this comment.
This returns NULL if you don't assign the outputs here.
There was a problem hiding this comment.
Yea I saw this (and also introduced it haha oops) - Proper fix is in #440
There was a problem hiding this comment.
Thank you!! We can merge 440 first. i'll rebase here and update tests as needed. i'll actually move this to draft for the time being until that is done!
| } | ||
|
|
||
| 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 .
| import generate from '../index.js' | ||
| import contributors from './fixtures/contributors.json' | ||
|
|
||
| /** |
There was a problem hiding this comment.
I've added docstrings using JSDoc throughout. make it easier to read everything.
| }) | ||
|
|
||
| /** | ||
| * Tests that addContributor calls infoFetcher with the correct arguments |
There was a problem hiding this comment.
This tests for the assignment error, which was a part of why the CLI was failing previously. The config data were not being populated
|
|
||
| function fixtures() { | ||
| const options = { | ||
| repoType: 'github', |
There was a problem hiding this comment.
Essentially, in the test, I was seeing null lwasser vs github lwasser.
|
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. |
|
|
||
| /** | ||
| * Tests that addContributor properly propagates errors when infoFetcher rejects. | ||
| * The same error should be returned without being swallowed or transformed. |
There was a problem hiding this comment.
Essentially, we had stacking bugs here... so the errors didn't fully represent what was going wrong and it wasn't clear where the actual bugs were.
2 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: