-
Notifications
You must be signed in to change notification settings - Fork 637
feat: Include versions in snap_getClientStatus
#3724
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
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #3724 +/- ##
=======================================
Coverage 98.28% 98.28%
=======================================
Files 418 418
Lines 12152 12154 +2
Branches 1876 1876
=======================================
+ Hits 11943 11945 +2
Misses 209 209 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
snap_getClientStatus
| * It returns an object containing useful information about the client. | ||
| */ | ||
| export type GetClientStatusResult = { locked: boolean; active: boolean }; | ||
| export type GetClientStatusResult = { |
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 a good opportunity to document the parameters now.
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.
|
|
||
| const getIsLocked = jest.fn().mockReturnValue(true); | ||
| const getIsActive = jest.fn().mockReturnValue(false); | ||
| const getVersion = jest.fn().mockReturnValue('13.6.0-flask.0'); |
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.
Bug: Test Expectation Mismatch for Hook Names
The test assertion for hookNames is missing the getVersion: true property that was added to the implementation. The test expects hookNames to only contain getIsLocked and getIsActive, but the implementation now includes getVersion as well (added at line 16 in getClientStatus.ts). This will cause the test to fail because the actual object has three properties while the test only expects two.
Include
clientVersionandplatformVersionin the response forsnap_getClientStatus. This is useful for ensuring backwards compatibility in Snaps or enabling new features in certain versions.Note
Add
clientVersionandplatformVersiontosnap_getClientStatus, introducegetVersionhook, update types, simulation, and tests.snap_getClientStatusto includeclientVersionandplatformVersioninpackages/snaps-rpc-methods/src/permitted/getClientStatus.ts.getVersiontoGetClientStatusHooksand usegetPlatformVersion()in the implementation.GetClientStatusResultto includeclientVersionandplatformVersioninpackages/snaps-sdk/src/types/methods/get-client-status.ts.getVersionpermitted hook (returns"13.6.0-flask.0") inpackages/snaps-simulation/src/simulation.tsand add corresponding test.packages/examples/packages/client-status/src/index.test.tsand add@metamask/snaps-utilsdevDependency.getClientStatustests to expect new fields.Written by Cursor Bugbot for commit ed97239. This will update automatically on new commits. Configure here.