Check if the maxWorkers has a value and raises an exception otherwise #4591
Conversation
In this commit we are adding a new validation in the check method of the jest-cli package, so we raise an exception if the user tries to run Jest with undefined maxWorkers, e.g. jest --maxWorkers jest --maxWorkers 2 Some users were confusing -w as an alias to --watch, but in fact it is an alias to --maxWorkers. This change will also make this distinction clearer. Fixes issue #4577
|
|
|
It seems to be Jest's default behaviour to display the whole menu when an exception is raised during the validation of the arguments.
Should we change this default behaviour, so whenever the arguments are invalid Jest displays only the error message? |
Codecov Report
@@ Coverage Diff @@
## master #4591 +/- ##
=======================================
Coverage 55.68% 55.68%
=======================================
Files 186 186
Lines 6348 6348
Branches 3 3
=======================================
Hits 3535 3535
Misses 2812 2812
Partials 1 1Continue to review full report at Codecov.
|
|
|
||
| it('raises an exception if config is not a valid JSON string', () => { | ||
| const argv: Argv = {config: 'x:1'}; | ||
| expect(() => check(argv)).toThrow(); |
There was a problem hiding this comment.
Tests, nice! I think we should at least check the core message of the error though.
Something like this would be more descriptive:
expect(() => check(argv)).toThrowError('not a valid JSON string');
packages/jest-cli/src/cli/args.js
Outdated
| throw new Error( | ||
| 'The --maxWorkers option requires a number to be specified.\n' + | ||
| 'Example usage: jest --maxWorkers 2\n' + | ||
| 'Or did you mean --watch ?', |
There was a problem hiding this comment.
nit: remove the space before question mark.
Also, how about adjusting this to most common mistake:
- The --maxWorkers option
+ The --maxWorkers (-w) option
|
@cpojer Let's wait for CI to pass and it's basically ready to merge. |
|
Agree with @thymikee, this is a great PR. Thank you <3 |
|
This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs. |
Summary
In this commit, we are adding a new validation in the
checkmethod of thejest-clipackage, so we raise an exception if the user tries to run Jest with undefinedmaxWorkersSome users were confusing
-was an alias to--watch, but in fact,-wis an alias to--maxWorkers. This change will also make this distinction clearer.Also, a new test file was added to the
jest-clipackage, in order to unit test thecheckmethod.Fixes issue #4577
Test plan