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’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add more tests #92

Open
lucasbento opened this issue Aug 20, 2019 · 10 comments
Open

Add more tests #92

lucasbento opened this issue Aug 20, 2019 · 10 comments
Labels
enhancement New feature or request help wanted Extra attention is needed

Comments

@lucasbento
Copy link
Member

Feature Request

Right now we only have these sad little tests and we definitely need to add more.

@lucasbento lucasbento added enhancement New feature or request help wanted Extra attention is needed labels Aug 20, 2019
@KevinHu2014
Copy link
Contributor

@lucasbento Would you mind if I give it a try?
Recently I'm learning Enzyme, it will be great if I can learn by doing.

@lucasbento
Copy link
Member Author

@KevinHu2014: go for it :)

We already have react-testing-library setup so I would recommend continuing with it and if you need any help just give me a shout 🙂

@KevinHu2014
Copy link
Contributor

Just finished reading the docs of react-testing-library and some Kent C. Dodds's video.
Will get started in the following week.

@jmporchet jmporchet mentioned this issue Sep 10, 2019
2 tasks
@jmporchet
Copy link
Contributor

@KevinHu2014 feel free to contribute!

@KevinHu2014
Copy link
Contributor

@jmporchet Thanks for the PR! ❤️
It give me a hint on how I should start.
I'll try to add more test (such as the behavior after the button is clicked) after the PR is merge.

@pvinis
Copy link
Member

pvinis commented Sep 13, 2019

I just saw all the snapshot testing. Good work, and thanks!

I do have one question, and please don't take it the wrong way, it's just me being ignorant, so any info would be helpful for me. Why do we need snapshots? 😬 I mean what do they give us besides knowing that the UI will stay the same? If we change the UI, we update the snapshots, so it's extra work. If we don't change the UI, then they stay the same, same as if we didn't have them. What is the point of them?

Thank you ahead of time, if you decide to help me understand :D

@lucasbento
Copy link
Member Author

It's better to have 10 snapshot tests than no tests at all.

Basically, if you have a function that return X and you want to refactor it, you just want it to perform in a different way but keep giving the same result.

The snapshots are only to guarantee that the output your code is providing won't change.

@pvinis
Copy link
Member

pvinis commented Sep 13, 2019

But specifically for UI, how is that useful? It means that every single time we make any UI change, we need to update the snapshots. And any change we do in the logic and the rest of the functions, will not matter for UI. So what do the snapshots give us? 😬

@jmporchet
Copy link
Contributor

jmporchet commented Sep 14, 2019

@pvinis this is a very valid concern.
At the moment, the tests only ensure that the components render, which would avoid the "catastrophic failure" path.
It would probably be a good idea to have a test to select two versions, click on the button and ensure that the output is what's expected.

I agree that snapshot testing might not be necessary or even advisable for all components actually.
Since the PR has been merged, we can now refactor the tests to give more value.

By the way I would be interested in your thoughts for good test cases!

@pvinis
Copy link
Member

pvinis commented Sep 25, 2019

Just writing it here, I would like to add some tests for the version filtering.

I would like to have lists of releases, and from that derive if they should be shown or not (for rcs for example), if we should display the popover for we recommend using the latest patch, etc.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

4 participants