TheThingsNetwork / lorawan-stack

The Things Stack, an Open Source LoRaWAN Network Server
https://www.thethingsindustries.com/stack/
Apache License 2.0
957 stars 302 forks source link

Snapshots for visual testing in our end-to-end tests #3786

Open kschiffer opened 3 years ago

kschiffer commented 3 years ago

Summary

In this issue I want to suggest an improvement for how we conduct visual testing as part of our end-to-end test. The idea is to use snapshot testing instead of asserting on UI elements directly.

Why do we need this?

Current visual testing has issues such as:

What is already there? What do you see now?

With visual testing, I mean the parts of our tests that ensure that the UI is rendered correctly. Currently we do that by asserting on the visibility of the UI elements that are rendered on a page (see here e.g.), as well as asserting on the exact text that these elements display.

What is missing? What do you want to see?

There are libraries that facilitate visual testing using snapshots. Snapshots are either literal image snapshots, or copies of text or markup that an implementation is checked against. We can use this method to make visual testing more flexible yet more precise. When working on visual updates, we can create snapshots (via cypress) and let cypress compare the visual output against those screenshots for subsequent tests.

Obviously there are caveats for this methodology as well. E.g. one issue of snapshot testing is the danger that snapshots get carelessly updated when their test-cases fail. It's important to maintain snapshot testing consciously and that everyone working on end-to-end tests knows how snapshot tests work.

I still think that this downside does not offset the benefits that we gain from using snapshot tests (as described above).

How do you propose to implement this?

There are many cypress plugins like https://github.com/jaredpalmer/cypress-image-snapshot which we can use to implement snapshot testing. The idea would be to replace all renders UI elements in place-tests with such snapshot tests. We could then also more efficiently add visual tests for different visual states.

Can you do this yourself and submit a Pull Request?

Yes, after we made a decision.

@bafonins @asmulko please share your thoughts about this and/or greenlight if you agree.

bafonins commented 3 years ago

Locks us in into a specific arrangement of the UI (down to the exact wording of texts)

We do want to assert that elements are in the right place with correct text displayed, right?

Produces false negatives when UI changes structurally

Can you provide an example of such false negatives?

E.g. one issue of snapshot testing is the danger that snapshots get carelessly updated when their test-cases fail

We cannot enforce that. We actually had such issues when dealing with jest snapshots for unit tests.

I agree that defining displays UI elements in place tests is not the most convenient solution for ensuring that UI is rendered properly, especially taking into account that we are adding hints with a lot of text and several nested elements (like links, think of DR form hints). Asserting on these elements is tedious.

I propose to research the topic and test various approaches/libraries for visual testing when implementing https://github.com/TheThingsNetwork/lorawan-stack/issues/3478. I believe that with concrete implementations and CI scenarios we can have more constructive discussion on visual testing with its pros and cons.

For future reference: The list visual testing libraries for cypress

kschiffer commented 3 years ago

We do want to assert that elements are in the right place with correct text displayed, right?

I'd say this is debatable. There should be a proportionality between the utility of the tests and the costs in terms of development time we get by tediously having to write and maintain those. A missing or faulty text does not render the app or a feature unusable altogether but is more of an annoyance. In this regard, snapshot tests help us because there is basically no setup and refactoring cost, while we still get the assurance that things render correctly and that even with more precision.

Can you provide an example of such false negatives?

For example, if we were to decide to change or remove field descriptions, many tests will result in false negatives since the app still works as intended but the test wrongfully expects certain text to be in place when in fact such assertion is irrelevant for assessing whether a feature still works or not.

We cannot enforce that. We actually had such issues when dealing with jest snapshots for unit tests.

There will always be caveats but we have to see this issue in relation to the benefits we get from automating visual tests and I think it's acceptable. Also, jest snapshots were harder to assess since they were plain text representations of the DOM. I could imagine that it is easier to spot snapshot problems when they are actual images.

I propose to research the topic and test various approaches/libraries for visual testing when implementing #3478. I believe that with concrete implementations and CI scenarios we can have more constructive discussion on visual testing with its pros and cons.

Sounds good. I'll reassign to you then.

nejraselimovic commented 3 years ago

@kschiffer @bafonins did you make a final decision on this one? Can we remove the discuss label?

kschiffer commented 3 years ago

I think it can be considered decided that we want to try out visual testing in the future and that will be in context of #3478, so I'll remove the discussion label.