Conversation
|
CLA Signature Action: All authors have signed the CLA. You may need to manually re-run the blocking PR check if it doesn't pass in a few minutes. |
Builds ready [ecc02f3]
Page Load Metrics (668 ± 87 ms)
|
Builds ready [2eee19b]
Page Load Metrics (545 ± 51 ms)
|
Builds ready [44b7909]
Page Load Metrics (578 ± 35 ms)
|
| "test:unit": "mocha --exit --require test/env.js --require test/setup.js --ignore './ui/app/pages/swaps/**/*.test.js' --recursive './{ui,app,shared}/**/*.test.js'", | ||
| "test:unit:global": "mocha --exit --require test/env.js --require test/setup.js --recursive test/unit-global/*.test.js", | ||
| "test:unit:lax": "mocha --exit --require test/env.js --require test/setup.js --ignore './app/scripts/controllers/permissions/*.test.js' --recursive './{ui,app,shared}/**/*.test.js'", | ||
| "test:unit:jest": "jest", |
| restoreMocks: true, | ||
| setupFiles: ['./test/setup.js', './test/env.js'], | ||
| testMatch: ['**/ui/app/pages/swaps/**/?(*.)+(test).js'], | ||
| }; |
There was a problem hiding this comment.
Can we please add coverageThreshold for Jest? Something like we have in the Controllers repo: https://github.com/MetaMask/controllers/blob/develop/package.json#L108
We can start with whatever numbers we get now from the coverage report and slowly but surely keep increasing them with new code.
Once we have this minimum threshold set up, I can send a PR for setting up enforcement of that threshold on every Git push. It could be done e.g. via the husky node module: https://github.com/typicode/husky
Then whoever is contributing to the Swaps feature (and later on to any feature in this repo) will have to add enough unit tests, otherwise they won't be able to push their code.
Let me know what you think.
There was a problem hiding this comment.
Setting a minimum coverage goal and ratcheting it up is a good idea - that's basically what we've done with the test:unit:strict and test:unit:lax scripts (the "strict" ones have 100% coverage).
Not sure it needs to go in at this point though? The coverage is incredibly low at the moment. Seems like it could wait for another PR.
The Husky suggestion is bound to be controversial 😅 Some people find husky very irritating. Certainly a discussion to have elsewhere. I've been considering removing the husky setup from the controllers repo and replacing it with something where the git hook is still version-controlled but is opt-in rather than automatic, so that people who prefer not using it or have their IDE running the linter/tests on each change already don't have to add --no-verify to every command.
Co-authored-by: Mark Stacey <markjstacey@gmail.com>
Builds ready [0b66411]
Page Load Metrics (578 ± 31 ms)
|
Configures jest and lint rules for the
/ui/app/pages/swaps/*.test.js.Omit the swaps folder from unit tests for now.Adds
test:unit:jestscript to run the jest tests isolated to the swap ui folder.Rewrite
ui/app/pages/swaps/swaps.util.test.jsto jest. Replacesproxyquirewithjest.mockand usenockfor api mocking.