Strict and explicit config resolution logic #4083
Conversation
| @@ -61,7 +61,7 @@ export type DefaultOptions = {| | |||
| watchman: boolean, | |||
| |}; | |||
|
|
|||
| export type InitialOptions = {| | |||
There was a problem hiding this comment.
this object is not supposed to be exact. It comes from the config file that can have anything in it. We guard against unrecognized options in jest-validate package.
| snapshotSerializers?: Array<Path>, | ||
| testEnvironment?: string, | ||
| testFailureExitCode?: string | number, | ||
| testMatch?: Array<Glob>, | ||
| testNamePattern?: string, | ||
| testPathIgnorePatterns?: Array<string>, | ||
| testPathDirs?: Array<Path>, |
There was a problem hiding this comment.
i'm not sure if it's deprecated or missing.
After properly typing this object, flow found that we use this property in the code but it was not present in this type
Codecov Report
@@ Coverage Diff @@
## master #4083 +/- ##
=========================================
Coverage ? 60.65%
=========================================
Files ? 197
Lines ? 6781
Branches ? 6
=========================================
Hits ? 4113
Misses ? 2665
Partials ? 3
Continue to review full report at Codecov.
|
|
Please bring back the glob functionality, that should definitely be part of the feature and I don't want to break it. I'm ok with the added validation otherwise. |
| @@ -153,6 +153,29 @@ const _printVersionAndExit = outputStream => { | |||
| process.exit(0); | |||
| }; | |||
|
|
|||
| const _ensureProjectsDontUseTheSameConfig = (parsedConfigs, projects) => { | |||
There was a problem hiding this comment.
"Dont" just really doesn't fit into code. How about:
- ensureDifferentConfigFiles
- ensureDiverseConfigFiles
or something similar?
There was a problem hiding this comment.
ensureNoDuplicateConfig?
|
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. |
This was one of the most confusing thing about MPR.
given an invalid path in a
projectsconfig property:current version of Jest would just either ignore this path completely and remove it from the project list, or, even worse, resolve back to the exact same
package.jsonand try to run a project with all-test.jsfiles from all the projects using the default configuration.This PR:
the only part that is lost is the ability to specify projects by a glob
but i think this isn't a good use of globs and we don't even use it for our examples either. Anything that needs dynamic project list can use a
.jsbased config and run glob explicitly.If we still want to keep glob functionality, we should at least enforce that every glob resolves into at least one match.