Skip to content
This repository was archived by the owner on Nov 9, 2023. It is now read-only.

Conversation

@rekmarks
Copy link
Member

@rekmarks rekmarks commented Apr 8, 2022

Migrates all tests to Jest and TypeScript per the module template. Care was taken to modify the existing tests as little as possible. Therefore, the tests unfortunately make prodigious use of casts to any. Nevertheless, to minimize such casts, a pair of assertion type guards were added for successful and failed JSON-RPC responses. They use the existing boolean type guards under the hood, and are fully tested.

Assertion type guards are tremendously helpful in situations like this, where boolean type guards don't help since e.g. expect(typeGuard(value)).toBe(true); doesn't tell TypeScript anything, and we have lint rules preventing us from calling expect conditionally.

@rekmarks rekmarks force-pushed the standardize-repository branch from 9bca929 to ac87a75 Compare April 8, 2022 17:11
Base automatically changed from standardize-repository to main April 8, 2022 18:16
@rekmarks rekmarks marked this pull request as ready for review April 8, 2022 18:17
@rekmarks rekmarks requested a review from a team as a code owner April 8, 2022 18:17
@rekmarks
Copy link
Member Author

rekmarks commented Apr 8, 2022

@mcmire @Gudahtt does coverage fail for you locally on any version of Node 14? It does not do so on my end.

Edit: Resolved by changing the coverage provider from v8 to Babel.

@rekmarks
Copy link
Member Author

rekmarks commented Apr 8, 2022

@mcmire I converted all function keyword functions to arrow functions in the test files. I'm surprised our lint config didn't yell at me about that 🤔

`Reflect` doesn't have that method. It only works because the property access walks the prototype chain back to `Object`.
Copy link

@mcmire mcmire left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for doing this. This looks good to me. There's obviously more we can do to improve the test suite as I'm sure we are aware, but this is good on its own.

@rekmarks rekmarks merged commit 07f7604 into main May 16, 2022
@rekmarks rekmarks deleted the use-jest branch May 16, 2022 01:06
rekmarks added a commit to MetaMask/utils that referenced this pull request May 16, 2022
`json-rpc-engine` recently added some generic JSON utils. This PR copies them to this package so that they can be deleted from `json-rpc-engine`. The tests and utilities are copied over exactly as they were implemented in MetaMask/json-rpc-engine#102.
mcmire pushed a commit to mcmire/core that referenced this pull request Jul 17, 2023
`json-rpc-engine` recently added some generic JSON utils. This PR copies them to this package so that they can be deleted from `json-rpc-engine`. The tests and utilities are copied over exactly as they were implemented in MetaMask/json-rpc-engine#102.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants