kawalcovid19 / wargabantuwarga.com

Inisiatif warga untuk berbagi informasi seputar fasilitas kesehatan dan alat kesehatan untuk COVID-19.
https://www.wargabantuwarga.com
MIT License
554 stars 166 forks source link

Avoid extensive usages of `.toMatchInlineSnapshot()` in tests to make it more useful #697

Closed zainfathoni closed 3 years ago

zainfathoni commented 3 years ago

Description

The extensive usages of .toMatchInlineSnapshot() assertion causes unnecessary snapshot changes that are weakly related with the changes we made in each PRs. It renders the snapshots to be less useful.

We have to replace those assertions with more meaningful assertions.

Implementation model

We need to avoid snapshot testing using .toMatchInlineSnapshot() in favour of testing components the way the user would use it. This means using meaningful assertions as much as possible.

To quote Kent C. Dodds:

The more your tests resemble the way your software is used, the more confidence they can give you.

Further reading material: Effective Snapshot Testing by Kent C. Dodds.

Tasks

Tests using .toMatchInlineSnapshot():

resir014 commented 3 years ago

Updated first post with a list of all the test files still using .toMatchInlineSnapshot().

adibfirman commented 3 years ago

Hi @zainfathoni @resir014 I will try to pick up this issue but I have a question regarding it, according to the article above seems function .toMatchInlineSnapshot() its too "huge snapshots" that we use in our test file, so basically do we just remove it or change it with another expect function? thanks

zainfathoni commented 3 years ago

We should replace them with more meaningful assertions, such as asserting that some text are rendered, or some images with certain alt text are rendered.

adibfirman commented 3 years ago

@zainfathoni i see, noted for that thank you, I will trying to picking up this task ya mas 👌

rubiagatra commented 3 years ago

Good luck @adibfirman !