-
Notifications
You must be signed in to change notification settings - Fork 567
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
Expand testing for expanded-view notifications #2956
base: main
Are you sure you want to change the base?
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #2956 +/- ##
==========================================
+ Coverage 94.71% 94.73% +0.01%
==========================================
Files 497 497
Lines 10802 10843 +41
Branches 1649 1662 +13
==========================================
+ Hits 10231 10272 +41
Misses 571 571 ☔ View full report in Codecov by Sentry. |
packages/snaps-jest/src/matchers.ts
Outdated
`Expected footer link: ${this.utils.printExpected( | ||
expectedFooterLink, | ||
)}\n` + | ||
`Expected content id type: ${this.utils.printExpected('String')}\n` + |
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.
What's this?
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.
Realized that the notify method is creating an interface (expanded view) but not exposing the interface id, so the id can't be used inside toSendNotification
. I was attempting to do some validation to make sure the response had a string content id. I'm now wondering if I can somehow intercept the middleware to return the original jsx content instead of the interface id so users can also validate the content inside of toSendNotification
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.
@Mrtenz In the latest commit, 2cb571f, I'm trying to get the actual jsx content now and match against it, returning the jsx content in the "received" part of the message. The example snap's test pass regardless if I have the wrong content passed into toSendNotification
so I'm not sure where I'm going wrong here, would appreciate a second look.
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.
Nevermind, figured it out :)
@metamask/notification-example-snap
to have a method to trigger an expanded view notification.test-snaps
to trigger the new methods added above.snaps-jest
to allow for unit testing of expanded view notifications.