test: use snapshots to properly test version-select component#1185
test: use snapshots to properly test version-select component#1185aryanshridhar wants to merge 1 commit intoelectron:mainfrom
Conversation
|
cc @georgexu99 @dsanders11 :) |
|
The test looks good, but I'm thinking that we should put this test with the rest of the PR? @dsanders11 Also, perhaps this is a bit of an unfounded worry, but given the size of the other PR and its age, it may honestly be easier reapply the changes on top of main (with a careful cherry-pick or by hand), unless merging the new main changes don't pose a huge problem |
erickzhao
left a comment
There was a problem hiding this comment.
I would lean against replacing the existing assertion with a snapshot test.
IMO this change makes it harder to read what the exact assertion is and makes the test more brittle to any change in the component (easy to get false positives).
|
@erickzhao makes a good point about this making the test more brittle. Let's find a different way to fix the test failure in #1160. Sorry about the churn, @aryanshridhar. |
|
No worries :) |
#1160 (comment)