-
-
Notifications
You must be signed in to change notification settings - Fork 949
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
test: migrate datatype to test snapshots #875
Conversation
Codecov Report
@@ Coverage Diff @@
## main #875 +/- ##
==========================================
- Coverage 99.64% 99.64% -0.01%
==========================================
Files 2146 2146
Lines 230380 230380
Branches 980 979 -1
==========================================
- Hits 229571 229569 -2
- Misses 788 790 +2
Partials 21 21
|
I'm sorry but in what world is this easier to maintain? By changing any label in the entire test chain, you have an invalid expectation key. Also since every snapshot value is a string, prettier can't apply the style guide correctly. Or am I missing something? Can you please elaborate on how this helps maintainability wise? |
Snapshots can be updated by just using But yeah, I was not aware that it just uses strings and therefore we loose the types 🤔 |
Ok, I didn't know there was such an option. Thought the migration was done by hand. |
That's the whole point of using snapshots 😉 |
18f0328
to
e2c67be
Compare
8873762
to
1b54199
Compare
I would prefer to have all tests migrated in one PR so it is only one commit in main branch history. You could create commits test by test to make review easier. |
1b54199
to
922be88
Compare
922be88
to
1e30d69
Compare
All converted. Ready for review. |
We should migrate from our old inline seeded tests to using test snapshots, as these are way easier to update.
I plan to update the other modules as well, but this one is the first in case something needs to be changed.