refactor: Simplify test-runner logic #196
Conversation
|
@trevor-bliss The entire test suite has to be rebuilt reworked after this change. Before I do this I would like to get your feeling on this change. |
| function getJestArgs(argv) { | ||
| // Extract known options from parsed arguments | ||
| const knownOptions = Object.keys(argv) | ||
| .filter((key) => argv[key] && JEST_PASSTHROUGH_OPTIONS.has(key)) |
There was a problem hiding this comment.
Looking through this repository, it doesn't look like we have any logic that requires explicit default values of false for boolean arguments so I think we can remove the defaults from the configuration in this file. That would make this code more robust since it wouldn't need to change if we ever introduce new non-boolean arguments with truthy values.
Otherwise, my second preference would be to change this to:
| .filter((key) => argv[key] && JEST_PASSTHROUGH_OPTIONS.has(key)) | |
| .filter((key) => argv[key] !== false && JEST_PASSTHROUGH_OPTIONS.has(key)) |
|
@trevor-bliss Do you mind doing another round of review now I have reworked the test suite? |
| @@ -16,7 +16,7 @@ | |||
| }, | |||
| "scripts": { | |||
| "check-license-headers": "node ./scripts/checkLicenseHeaders.js", | |||
| "lint": "eslint src/", | |||
| "lint": "eslint src/ tests/", | |||
There was a problem hiding this comment.
Test files were not linted before.
|
|
||
| process.on('unhandledRejection', reason => { | ||
| runJest(args) |
There was a problem hiding this comment.
Delegating all the process.exit handling to the bin file instead of having this all around the code base.
| }); | ||
| }); | ||
| }; | ||
| const promiseExec = promisify(exec); |
There was a problem hiding this comment.
This is a beautiful help in Node.js for doing this =)
This PR is an attempt to simplify the test runner logic:
Fixes #36