Move to Jest as testing framework #1035
Conversation
shadowspawn
changed the title
|
I am happy with how this is going, and I like Jest. It is popular, comes with everything you need without extra installs or decisions, and I can find what I want in its documentation. Pushing on with converting tests to Jest. |
|
Some notes on approach:
|
|
Currently working through all the |
…cutable subcommand.
- _exit never returns (as before) - callback handled explicitly in executeSubcommand - mark _exit as private (copy and paste omission) - support executeSubCommandAsync i default override handler to prevent call to process.exit, as expected
|
Giant PR! But not expecting you will look at every file. :-) |
| program | ||
| .option('-p', 'add pepper'); | ||
| program.parse(['node', 'test', '-p']); | ||
| expect(program.P).toBe(true); |
There was a problem hiding this comment.
I'm sorry if I sound picky.
It seems good to use toBeTruthy.
https://jestjs.io/docs/ja/expect#tobetruthy
There was a problem hiding this comment.
No problem. I did consider toBeTruthy, but I decided important to test for true in at least some places. Say if we changed the implementation to return 1 or default or some other truthy value, some of the tests should fail because it would break some clients.
The README has true in an example:
$ pizza-options -d
{ debug: true, small: undefined, pizzaType: undefined }
|
Are you still reviewing @abetomo? (Just checking in case comment was only question. No rush, please take your time to check what you want.) |
|
What do you think about checking the test code with eslint? |
|
In |
|
LGTM! When eslint is over, I will review that part again. |
|
Pushed, with eslint errors fixed (and the deliberately skipped tests renamed to avoid a persistent warning). |
shadowspawn
added
the
pending release
|
(GitHub Action tests failed on macOS 8, but I can't reproduce locally in first try.) |
shadowspawn
removed
the
pending release
Following lead of #755 which converts two of the tests from should.js to AVA, this initally converts the same two tests to Jest
Related issues: #661, #649, #927
(Previous PR #1005 got closed when I deleted release/3.0.0 branch.)