test: add tests covering basic functionality #2
Conversation
|
@fredericoo did you have the chance to take a look? |
tests/mocks.ts
Outdated
| export const allCats = ['Salem', 'Mimo', 'Tapi']; | ||
| export const newCat = 'Millie'; |
There was a problem hiding this comment.
nit: would just move this to the test itself, theres no need for a whole file with two variables
package.json
Outdated
| @@ -4,8 +4,8 @@ | |||
| "description": "Create react contexts with zustand", | |||
| "scripts": { | |||
| "build": "tsup --dts --dts-resolve", | |||
| "test": "bun test src", | |||
| "check": "bunx @biomejs/biome check --apply ./src", | |||
| "test": "bun test src tests", | |||
There was a problem hiding this comment.
should we just move tests into src and remove those? I’d normally recommend we keep tests next to the code so we don’t feel disconnected from the tests!
tests/component.test.tsx
Outdated
| const renderApp = () => | ||
| render( | ||
| <CatsProvider initialValue={{ cats: allCats }}> | ||
| <CatsList /> | ||
| </CatsProvider>, | ||
| ); |
There was a problem hiding this comment.
would opt for repetition instead of this; it’s just indirection that saves us nothing and makes tests harder to grok
fredericoo
changed the title
tests/component.test.tsx
Outdated
|
|
||
| renderApp(); | ||
|
|
||
| await user.click(screen.getByRole('button', { name: 'Add new cat' })); |
There was a problem hiding this comment.
seeing as we’re only calling one event per test, we can just use userEvent.click instead of setting it up as user.
Are there any advantages to setting up userEvent first?
| await user.click(screen.getByRole('button', { name: 'Add new cat' })); | |
| await userEvent.click(screen.getByRole('button', { name: 'Add new cat' })); |
There was a problem hiding this comment.
Thank you for your feedback, I used it this way because it is recommended in the docs:
https://testing-library.com/docs/user-event/intro/#writing-tests-with-userevent
You can have a look at the last paragraph of this section, in which it is recommended to use the methods on the instances returned by userEvent.setup(), instead of directly invoking APIs such as userEvent.click().
There was a problem hiding this comment.
good to know it's a testing library v14 change that's just supported as they migrate away from it. I've just learnt a new thing and now wanna adjust all my unit tests at work 🤣
|
thanks for adding tests! they’re pretty good and cover getting and setting ❤ |
|
Thank you for your suggestions! I have implemented the changes you recommended, except for the |
Hey, I came across this project recently and thought that we could add some tests covering the basic functionality. This was the first time using
Bun, so please let me know if any further adjustments are needed.