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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

馃И Testing: Fill in test coverage for augmentOptionsWithExcludes #699

Open
3 tasks done
JoshuaKGoldberg opened this issue Aug 25, 2023 · 3 comments
Open
3 tasks done
Labels
area: testing Improving how the repository's tests are run and/or code is tested good first issue Good for newcomers, please hop on! status: accepting prs Please, send a pull request to resolve this! type: cleanup Tech debt or other code/repository cleanups

Comments

@JoshuaKGoldberg
Copy link
Owner

JoshuaKGoldberg commented Aug 25, 2023

Bug Report Checklist

  • I have tried restarting my IDE and the issue persists.
  • I have pulled the latest main branch of the repository.
  • I have searched for related issues and found none that matched my issue.

Overview

#695 added a bunch of code changes. I didn't fully unit test them - not because I was too impatient to write tests (me?! never!!), but because I wanted to leave some good first issues as followups. Definitely that.

augmentOptionsWithExcludes is missing unit test coverage for several dozen lines one or two lines. augmentOptionsWithExcludes.test.ts only tests the case of fully automated usage. It doesn't test what happens when the user is prompted for anything. Let's add tests to augmentOptionsWithExcludes.test.ts that exercise the calls to prompt!

Additional Info

You'll likely need to use Vitest mocking - specifically, vi.mock. Search for vi.mock("@clack/prompts" in code to see how other unit tests accomplish this.

We don't need complete 100% unit test coverage of the file. If the tests are giving you grief, feel free to send an incomplete Draft PR with comments asking for help. 鉂わ笍


Note that the code might have been refactored since this issue was filed. Names might be slightly off. The general spirit of this issue should still be valid though.

@JoshuaKGoldberg JoshuaKGoldberg added good first issue Good for newcomers, please hop on! status: accepting prs Please, send a pull request to resolve this! area: testing Improving how the repository's tests are run and/or code is tested type: cleanup Tech debt or other code/repository cleanups labels Aug 25, 2023
@shobhit9957
Copy link

hey, I would love to contribute, can you please help me. I"m a beginner here...Thanks

@JoshuaKGoldberg
Copy link
Owner Author

JoshuaKGoldberg commented Aug 25, 2023

馃憢 hey @shobhit9957, great! My advice would be to:

  1. Read through all the docs in this repo -they're linked to in its README.md-
  2. Read through the 'getting started' guide of the tools needed to contribute: TypeScript, pnpm, Vitest
  3. See how other tests are written in this repo
  4. Try adding a test yourself
  5. Send a draft PR once you've made some progress - and ask any questions you need in that draft PR

Cheers!

I'm planning on writing a more helpful blog post soon. Hopefully that list is useful until then.

@shobhit9957
Copy link

hey 馃槉thanks @JoshuaKGoldberg will follow the steps you've provided and update you with the feedbacks!,

JoshuaKGoldberg added a commit that referenced this issue Feb 14, 2024
## PR Checklist

- [x] Addresses an existing open issue: fixes #1276
- [x] That issue was marked as [`status: accepting
prs`](https://github.com/JoshuaKGoldberg/create-typescript-app/issues?q=is%3Aopen+is%3Aissue+label%3A%22status%3A+accepting+prs%22)
- [x] Steps in
[CONTRIBUTING.md](https://github.com/JoshuaKGoldberg/create-typescript-app/blob/main/.github/CONTRIBUTING.md)
were taken

## Overview

Attaches `base` the options returned during augmentation, so they can
get printed out later.

I noticed that the `exclusionKeys` stuff was exported from the
`augmentOptionsWithExcludes.ts` file but never used in its tests. So I
extracted it to a new `exclusionKeys.ts`.

Adds some unit tests, but doesn't do all of #699. There's still coverage
missing.

Co-authored-by: @niklas-wortmann
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: testing Improving how the repository's tests are run and/or code is tested good first issue Good for newcomers, please hop on! status: accepting prs Please, send a pull request to resolve this! type: cleanup Tech debt or other code/repository cleanups
Projects
None yet
Development

No branches or pull requests

2 participants