Skip to content
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(read-config): allow multiple projects with programmatic usage #11307

Merged
merged 14 commits into from
Apr 23, 2021

Conversation

nicojs
Copy link
Contributor

@nicojs nicojs commented Apr 16, 2021

Summary

Allow for runCli with multiple projects.

Fixes #11213
Fixes #7415

const { runCLI } = require('jest');

const config = {
  projects: [ 
    { testMatch: [ "<rootDir>/client/**/*.test.js" ] }, 
    { testMatch: [ "<rootDir>/server/**/*.test.js" ] }
  ]
};

runCLI({ config: JSON.stringify(config) }, [process.cwd()])
 .then(() => console.log('✅ Done'))
 .catch(err => {
    console.error(err);
    process.exitCode = 1;
});

Test plan

I've tested this change in the reproduction zip of #11213. Output was:

image

@nicojs
Copy link
Contributor Author

nicojs commented Apr 16, 2021

Please let me know if you want me to add an integration test for this.

@codecov-commenter
Copy link

codecov-commenter commented Apr 16, 2021

Codecov Report

Merging #11307 (04d329b) into master (1be8d73) will decrease coverage by 0.01%.
The diff coverage is 25.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master   #11307      +/-   ##
==========================================
- Coverage   64.19%   64.18%   -0.02%     
==========================================
  Files         308      308              
  Lines       13528    13529       +1     
  Branches     3297     3298       +1     
==========================================
- Hits         8684     8683       -1     
  Misses       4131     4131              
- Partials      713      715       +2     
Impacted Files Coverage Δ
packages/jest-config/src/index.ts 54.16% <25.00%> (+0.64%) ⬆️
packages/expect/src/utils.ts 94.83% <0.00%> (-1.30%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 1be8d73...04d329b. Read the comment docs.

Copy link
Member

@SimenB SimenB left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ooh, nice! An e2e test for this would be great, yeah 🙂 Along with a changelog entry

@nicojs
Copy link
Contributor Author

nicojs commented Apr 23, 2021

Thanks for the review @SimenB. I've added an e2e test, as well as the changelog entry. 👷‍♂️✅

@nicojs nicojs requested a review from SimenB April 23, 2021 06:19
Copy link
Member

@SimenB SimenB left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks! Just test nitpicks 🙂

e2e/__tests__/runProgrammaticallyMultipleProjects.test.ts Outdated Show resolved Hide resolved
CHANGELOG.md Outdated Show resolved Hide resolved
test('run programmatically with multiple projects', () => {
const {stdout, exitCode} = run(`node run-jest.js `, dir);
expect(exitCode).toEqual(0);
expect(stdout).toMatch(/Done/);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

could you pass stdout through extractSummaries or something like that and snapshot it to ensure both tests show up?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A great addition, will do that and use snapshotting 👍

e2e/run-programmatically-multiple-projects/run-jest.js Outdated Show resolved Hide resolved
@SimenB
Copy link
Member

SimenB commented Apr 23, 2021

Also, CI complains about missing copyright headers 🙂

@nicojs
Copy link
Contributor Author

nicojs commented Apr 23, 2021

Great feedback 👍

I've implemented all of it and added the copyright header (what a waste of disk space to have that in each file, but what can you do I guess 🤷‍♂️)

@SimenB
Copy link
Member

SimenB commented Apr 23, 2021

I tweaked the test, btw. But it seems GH Actions does not like it. Maybe we need to pass the output through strip-ansi first?

@nicojs
Copy link
Contributor Author

nicojs commented Apr 23, 2021

Hmm if I run the tests locally and update the snapshots I see this diff:

image

Maybe we should sort the test results? Is there a helper for that?

@SimenB
Copy link
Member

SimenB commented Apr 23, 2021

Ah, flake in addition. We don't really care about rest, just summary is enough (as it says "2"). We don't care about the order they complete

@nicojs
Copy link
Contributor Author

nicojs commented Apr 23, 2021

Ah yes, I see what you mean. I've removed rest. Let's see some green 💚🍿

@SimenB
Copy link
Member

SimenB commented Apr 23, 2021

I think we still need to pass through strip-ansi for GH actions

@nicojs
Copy link
Contributor Author

nicojs commented Apr 23, 2021

Ok, I think this should be it then 🍏🟢

@SimenB SimenB merged commit 510c8bb into jestjs:master Apr 23, 2021
@SimenB
Copy link
Member

SimenB commented Apr 23, 2021

Thanks!

@github-actions
Copy link

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.
Please note this issue tracker is not a help forum. We recommend using StackOverflow or our discord channel for questions.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators May 24, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
4 participants