Skip to content
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.

Sign up for GitHub

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

Jump to bottom

[base-controller] Add RestrictedControllerMessenger tests for runtime error handling #2059

Closed
MajorLift opened this issue Nov 17, 2023 · 0 comments · Fixed by #2058
Closed

[base-controller] Add RestrictedControllerMessenger tests for runtime error handling #2059

MajorLift opened this issue Nov 17, 2023 · 0 comments · Fixed by #2058
Assignees
@MajorLift
Labels
team-wallet-framework

Comments

@MajorLift
Copy link
Contributor

Motivation

  • Currently, error handling branches for RestrictedControllerMessenger methods are not being tested on the basis of compile-time type checks.
  • These branches are being ignored with the istanbul directive: /* istanbul ignore next */ // Branches unreachable with valid types
  • However, given that these methods are being called by our JavaScript clients, we also need assurances on runtime error-handling behavior.

Tasks

  • Add jest tests for the previously ignored error handling branches.
  • Use @ts-expect-error in the tests to suppress the compile-time type errors and simulate a JavaScript environment.
@MajorLift MajorLift self-assigned this Nov 17, 2023
@MajorLift MajorLift mentioned this issue Nov 17, 2023
Merged
3 tasks
MajorLift added a commit that referenced this issue Nov 17, 2023
@MajorLift @mcmire
[base-controller] Add RestrictedControllerMessenger tests for run…
bb7f605 
…time error handling (#2058)

## Explanation

- Previously, error handling branches for
`RestrictedControllerMessenger` methods were not being tested on the
basis of compile-time checks: "Branches unreachable with valid types".
- However, given that these methods are being called by our JavaScript
clients, we also need assurances on runtime error-handling behavior.
- This PR adds jest tests for the previously ignored error handling
branches: coverage is brought up to 100%.
- `@ts-expect-error` is used to suppress the compile-time type errors to
simulate a JavaScript environment.

## References

- Closes #2059
- See #2051 (comment)

## Changelog

### `@metamask/base-controller`

## Checklist

- [x] I've updated the test suite for new or updated code as appropriate
- [x] I've updated documentation (JSDoc, Markdown, etc.) for new or
updated code as appropriate
- [x] I've highlighted breaking changes using the "BREAKING" category
above as appropriate

---------

Co-authored-by: Elliot Winkler <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@MajorLift MajorLift

Labels
team-wallet-framework
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

[base-controller] Add RestrictedControllerMessenger tests for runtime error handling MetaMask/core
1 participant
@MajorLift