Move all default values into jest-config
#9924
Conversation
Codecov Report
@@ Coverage Diff @@
## master #9924 +/- ##
=======================================
Coverage 64.23% 64.23%
=======================================
Files 292 292
Lines 12466 12466
Branches 3075 3077 +2
=======================================
Hits 8007 8007
Misses 3810 3810
Partials 649 649
Continue to review full report at Codecov.
|
ghostd
force-pushed
the
fix-gh-8826
branch
4 times, most recently
from
1097f87 to
386d2c1
Compare
|
I dont know why a test fails only for the windows platform. I dont have any Windows machine to check that :-/ |
|
@ghostd don't worry about that one, it seems it's a bit flaky. It passed for all other versions 🙂 |
| clearMocks: false, | ||
| collectCoverage: false, | ||
| coveragePathIgnorePatterns: [NODE_MODULES_REGEXP], | ||
| coverageProvider: 'babel', | ||
| coverageReporters: ['json', 'text', 'lcov', 'clover'], | ||
| detectLeaks: false, | ||
| detectOpenHandles: false, |
There was a problem hiding this comment.
I'm fine with removing the defaults from args, but we left some flags like these out of user config on purpose, so they can only be set as, well, CLI flags. Don't we want to do that anymore?
There was a problem hiding this comment.
Oh, i didn't know that. I can update my PR in this way. Let me know what fits the project needs.
There was a problem hiding this comment.
yeah, I don't want that one 😀 let's just keep it open though, I'll go through more old PRs later and chime in on them
There was a problem hiding this comment.
Ok, to summarize, i need to update my PR to keep the detectOpenHandles default value in CLI options. Any other?
There was a problem hiding this comment.
wait, I don't think it matters if the default is here? As longs as it's false I mean. We just don't wanna read it from the user's config file.
@ghostd it's all the new ones added here - we want people to be able to set them from the CLI but not from config (at least not yet, we might change that)
There was a problem hiding this comment.
jest-cli already depends on jest-config, so does it make sense to use defaultOptions to init the CLI options object? Hence we'll have the default values in jest-config and all parameters having a default value in the yarg object will be not overrided by the user config.
I think the problem in relation to the open discussion may be dropping the implied 'you can import these defaults and stick any of their key value pairs into the Jest config' contract. Maybe jest-config should have an explicit 'not allowed on CLI' handling for some option, but only warn and ignore if people do specify that because they |
|
something like that should work, yeah. a blacklist should be fine, there are only a few flags we don't want people to stick in their config and forget about |
|
@ghostd would you be up for rebasing this? 🙏 |
|
@SimenB done :) |
|
Sorry for the long wait, people have been really busy. @ghostd, do you mind rebasing this PR one more time? |
ghostd
force-pushed
the
fix-gh-8826
branch
2 times, most recently
from
66743ea to
1d93971
Compare
|
Should be fine now. |
|
Looks like this breaks autodetection of CI environment, because Other cases when something reads from |
|
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. |
Fixes #8826