upgrade-helper icon indicating copy to clipboard operation
upgrade-helper copied to clipboard

Add more tests

Open lucasbento opened this issue 4 years ago • 10 comments

Feature Request

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

lucasbento avatar Aug 20 '19 08:08 lucasbento

@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.

KevinHu2014 avatar Aug 25 '19 15:08 KevinHu2014

@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 🙂

lucasbento avatar Aug 26 '19 08:08 lucasbento

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

KevinHu2014 avatar Sep 05 '19 18:09 KevinHu2014

@KevinHu2014 feel free to contribute!

jmporchet avatar Sep 11 '19 04:09 jmporchet

@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.

KevinHu2014 avatar Sep 11 '19 05:09 KevinHu2014

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

pvinis avatar Sep 13 '19 07:09 pvinis

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.

lucasbento avatar Sep 13 '19 08:09 lucasbento

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? 😬

pvinis avatar Sep 13 '19 08:09 pvinis

@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!

jmporchet avatar Sep 14 '19 07:09 jmporchet

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.

pvinis avatar Sep 25 '19 11:09 pvinis