-
Notifications
You must be signed in to change notification settings - Fork 84
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
fix global setup and teardown for jest multi-project setups #407
fix global setup and teardown for jest multi-project setups #407
Conversation
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.
Thanks a lot @ajwootto for your help on this 👍 Just one remark.
src/setup.ts
Outdated
const projects = config.projects.length ? config.projects : [config.rootDir]; | ||
const globalConfigPaths = projects.map(project => join(project, 'globalConfig.json')); |
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.
config.projects
can also be a list of sub-config. See https://jestjs.io/docs/configuration#projects-arraystring--projectconfig
We should probably have something like that:
const projects = config.projects.length ? config.projects : [config.rootDir]; | |
const globalConfigPaths = projects.map(project => join(project, 'globalConfig.json')); | |
const projects = config.projects.length ? config.projects : [config]; | |
const globalConfigPaths = projects.map(project.rootDir => join(project, 'globalConfig.json')); |
What do you think?
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.
@Grmiade looks like you are correct, if I set up a jest config using a combination of "string" projects and objects with project configuration, the setup
function is called with that same array. I can definitely fix it for that, but I'm struggling with the type definition to use here. The Jest source shows this function being called with an object of type "GlobalConfig"
https://github.com/facebook/jest/blob/aae0d3ac3498ece4673ec29277dc040234f9f643/packages/jest-core/src/runGlobalHook.ts#L58
but that type only specifies projects
as being an array of strings, which seems incorrect:
https://github.com/facebook/jest/blob/aae0d3ac3498ece4673ec29277dc040234f9f643/packages/jest-types/src/Config.ts#L392
it's defined correctly in "InitialOptions" but I don't think that represents the rest of what's passed in there correctly either...
https://github.com/facebook/jest/blob/aae0d3ac3498ece4673ec29277dc040234f9f643/packages/jest-types/src/Config.ts#L280
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.
disregard all that, updated the PR to just use the globalConfig
rootDir field (which is always set and works for both use-cases as well.)
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.
Thanks 🙏
thanks @harazdovskiy ! |
Fix the issue created by #389
Update the global setup and teardown to support having Jest "projects" specified. If specified, the preset will write a
globalSetup.json
file to each project directory, and remove all of them during teardown.If the "projects" option is not in use, it will continue with the behaviour introduced in the above PR by writing a single
globalSetup.json
file to therootDir
.This behaviour attempts to accommodate the use-case described in that PR without breaking the functionality for test setups using
projects
.Edit:
Updated to instead use the globalConfig's "rootDir" field rather than the projectConfig in
environment.ts
. This field is set even when theprojects
option is in use. We only need to write oneglobalConfig.json
file per Jest execution.