-
Notifications
You must be signed in to change notification settings - Fork 98
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
composition test & close buttons for new delegation implementation #1323
Conversation
Codecov Report
@@ Coverage Diff @@
## develop #1323 +/- ##
========================================
Coverage 89.26% 89.26%
========================================
Files 84 84
Lines 1733 1733
Branches 96 96
========================================
Hits 1547 1547
Misses 179 179
Partials 7 7
|
7fb1fc8
to
1d775f8
Compare
1d775f8
to
e3e1271
Compare
|
||
await submitDelegation({ amount: 10 }) | ||
|
||
expect($store.dispatch.mock.calls).toEqual([ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
maybe use snapshot?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think snapshots are a bad idea because:
- They're stored in a separate file so you can't see what the test is expecting easily.
- They require an external tool to manipulate (instead of just your code editor).
simpler = better
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
They're stored in a separate file so you can't see what the test is expecting easily
true.
They require an external tool to manipulate (instead of just your code editor)
false.
I agree with simpler = better
. But it is a lot more comfortable to update snapshots then go through code on a format change and change all the expected values. Let's discuss it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree that snapshots are useful for tracking a format change (e.g., in HTML) as part of Golden Master testing.
The above code is a unit test (not generating HTML), which is a different kind of test. What would be the benefit of using snapshots here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have the feeling that having huge json responses in code are too much noise that distract.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, good point.
screenshots would be helpful @NodeGuy 📸 |
I added a screenshot. |
Make progress against #1219
Description:
Adds a composition test and close buttons to the new Stake modal.