-
-
Notifications
You must be signed in to change notification settings - Fork 6.5k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
multi runner tests and bugfixes #4023
Conversation
packages/jest-config/src/index.js
Outdated
// It only used to read initial config. If the initial config contains | ||
// `project` property, we don't want to read `--config` value and rather | ||
// read individual configs for every project. | ||
dontUseArgvConfig?: boolean, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i really want to rewrite this file and the part of jest-cli
that resolves/reads config. This function should not be used for every config resolution (default config resolution, --config file.js
, `--config '{"json": true}', and MPR configs) because they have different logic and using the same flow for everything will cause a lot of bugs.
this little hack should work for now though
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, for now, can you call this variable something else? Having dont
isn't great, given that you are testing for the opposite anyway.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i can't really come up with a more descriptive name. renamed it to skipArgvConfigOption
. I hope that works!
const file1 = require('file1'); | ||
test('file1', () => {}); | ||
`, | ||
'project1/file1.js': fileContentWithProvidesModule('file1'), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i used haste modules to make sure multiple roots with the same modules don't conflict with each other using MPR (happened in fb/www)
packages/jest-config/src/index.js
Outdated
root = path.resolve(process.cwd(), rawOptions); | ||
// A string passed to `--config`, which is either a direct path to the config | ||
// or a path to directory containing `package.json` or `jest.conf.js` | ||
if (!dontUseArgvConfig && typeof argv.config == 'string') { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
for MPR runs with --config
specified this would always be true and --config
value would've been taken for each project
Codecov Report
@@ Coverage Diff @@
## master #4023 +/- ##
==========================================
+ Coverage 60.28% 60.31% +0.02%
==========================================
Files 196 196
Lines 6764 6761 -3
Branches 6 6
==========================================
Hits 4078 4078
+ Misses 2683 2680 -3
Partials 3 3
Continue to review full report at Codecov.
|
Love it, please rename the variable. <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. |
right now there's a bug in multi runner.
it can run
jest --projects p1 p2
but can't run
jest --config c.json
withc.json
having{projects: ['p1', 'p2']}
I added a test for MPR and fixed the config bug