fix(testing): update jest batch mode #12764
Conversation
barbados-clemens
requested review from
Cammisuli,
AgentEnder,
FrozenPandaz and
Coly010
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
barbados-clemens
added
the
scope: testing tools
jest go burrrr now
barbados-clemens
force-pushed
the
fix/jest-batch-go-brrrr
branch
from
0b58994
to
f3badc0
Compare
| /* The display name in the jest.config.js is the correct project name jest | ||
| * uses to determine projects. It is usually the same as the Nx projectName | ||
| * but it can be changed. The safest method is to extract the displayName | ||
| * from the config file, but skip the project if it does not exist. */ | ||
| const displayNameValueRegex = new RegExp( | ||
| /(['"]+.*['"])(?<=displayName+.*)/, | ||
| 'g' | ||
| ); | ||
| const fileContents = readFileSync(configPath, { encoding: 'utf-8' }); | ||
| if (!displayNameValueRegex.test(fileContents)) { | ||
| console.warn(stripIndents`Could not find "displayName" in ${configPath}. | ||
| Please add a "displayName" to the Jest Config File. | ||
| Skipping this project (${task.split(':')[0]})...`); | ||
| continue; | ||
| } | ||
|
|
||
| const displayName = fileContents | ||
| .match(displayNameValueRegex) | ||
| .map((value) => value.substring(1, value.length - 1))[0]; | ||
| selectedProjects.push(displayName); |
There was a problem hiding this comment.
@barbados-clemens Here is the code to find the actual jest project 😄
There was a problem hiding this comment.
this should be fine for now, but there is a snowballs chance in hell that someone is using a var to reference the displayName. i.e.
const myName = 'something-cool';
export default {
displayName: myName
}also more likely,
export default {
displayName: {
name: 'something-cool',
color: 'blue'
}
}in which case we'd error/warn about it missing when it was just configured differently.
I think for an alpha it's fine, but we should make sure we handle this at some point.
so maybe we should be more explicit in the error we are expecting the displayName to be a string for now?
Coly010
force-pushed
the
fix/jest-batch-go-brrrr
branch
from
4d222e2
to
bd11c52
Compare
| /(['"]+.*['"])(?<=displayName+.*)/, | ||
| 'g' | ||
| ); | ||
| const fileContents = readFileSync(configPath, { encoding: 'utf-8' }); |
There was a problem hiding this comment.
I'd look at not using sync here. We should try and optimize reading the files, because that was the original issue with jest 😅
Or do some measurements and see if we can get away with sync
There was a problem hiding this comment.
agree. we should perf this and maybe read in parallel with the a promise. though idk how that will scale either with say 1000 projects trying to read in parallel.
There was a problem hiding this comment.
I had the concern about potentially having too many processes in parallel, but we should definitely perf it
There was a problem hiding this comment.
You could limit how many parallel calls are allowed
|
This pull request has already been merged/closed. If you experience issues related to these changes, please open a new issue referencing this pull request. |
jest go burrrr now
Current Behavior
jest tests are slower in Nx than the jest CLI
(shout out @Coly010 for finding the config changes)
Note:
NX_BATCH_MODE=trueenv var to be set.--output-style=streamlooks best/is easier to see what is going on (this should be the default for batch mode)Expected Behavior
test are the same speed as jest cli
Related Issue(s)
TODO
batchJestFixes #