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

sets project name if empty fixes #5597 #5862

Merged
merged 27 commits into from
Jan 20, 2019
Merged
Show file tree
Hide file tree
Changes from 17 commits
Commits
Show all changes
27 commits
Select commit Hold shift + click to select a range
5756129
sets project name if empty fixes #5597
Mar 23, 2018
50d9933
added to changelog
Mar 23, 2018
6036dfa
cr comments
Mar 23, 2018
be1cc18
removed unused import
Mar 23, 2018
6054f0b
fixed tests
Mar 23, 2018
feb9751
use inital options instead of project config
Mar 23, 2018
0bf6d8e
fix unintentional type change
paularmstrong Mar 23, 2018
4f72619
fixed path string and deleted unneeded test files
Mar 26, 2018
b7c061f
Merge branch 'master' into set_project_name_if_empty
Mar 26, 2018
0713d0f
merged in master
Mar 26, 2018
65c64d9
updated snapshots and tests
Mar 26, 2018
a856fb1
removed updated snapshots put change under fixes
Mar 26, 2018
04b0bb6
Merge branch 'master' into set_project_name_if_empty
Mar 26, 2018
80d21e6
merged master
Apr 9, 2018
2f14830
Merge branch 'master' into set_project_name_if_empty
cbelsole Apr 9, 2018
30fecea
Merge branch 'master' into set_project_name_if_empty
SimenB Aug 9, 2018
54302cf
Merge remote-tracking branch 'origin' into set_project_name_if_empty
cbelsole Aug 10, 2018
be40277
Merge remote-tracking branch 'root/master' into set_project_name_if_e…
cbelsole Dec 29, 2018
8c8761d
unique names across config and projects
cbelsole Dec 29, 2018
78801aa
fixed changelog
cbelsole Dec 29, 2018
9bbbd61
Update CHANGELOG.md
SimenB Dec 29, 2018
61523b0
Merge branch 'master' into set_project_name_if_empty
SimenB Jan 20, 2019
44aa91d
use Set
SimenB Jan 20, 2019
a370c21
use random uuid for assigning project name
thymikee Jan 20, 2019
1ab0bac
remove unnecessary test
thymikee Jan 20, 2019
691339f
fix lint
thymikee Jan 20, 2019
6fcab16
add e2e test
thymikee Jan 20, 2019
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
- `[jest-snapshot]` Add error messages for invalid property matchers. ([#6528](https://github.com/facebook/jest/pull/6528))
- `[jest-cli]` Show open handles from inside test files as well ([#6263](https://github.com/facebook/jest/pull/6263))
- `[jest-haste-map]` Fix a problem where creating folders ending with `.js` could cause a crash ([#6818](https://github.com/facebook/jest/pull/6818))
- `[jest-config]` Add name to project if one does not exist to pick correct resolver ([#5862](https://github.com/facebook/jest/pull/5862))
SimenB marked this conversation as resolved.
Show resolved Hide resolved

### Chore & Maintenance

Expand Down
1 change: 1 addition & 0 deletions packages/jest-cli/src/cli/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -295,6 +295,7 @@ const getConfigs = (
.filter(root => {
// Ignore globbed files that cannot be `require`d.
if (
typeof root === 'string' &&
fs.existsSync(root) &&
!fs.lstatSync(root).isDirectory() &&
!root.endsWith('.js') &&
Expand Down
60 changes: 60 additions & 0 deletions packages/jest-config/src/__tests__/normalize.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,66 @@ it('picks a name based on the rootDir', () => {
).toBe(expected);
});

it('picks a name for projects based on the main rootDir', () => {
const rootDir = '/root/path/foo';
const firstExpected = crypto
.createHash('md5')
.update('/root/path/foo:0')
.digest('hex');
const secondExpected = crypto
.createHash('md5')
.update('/root/path/foo:1')
.digest('hex');

const options = normalize(
{
projects: [{}, {}],
rootDir,
},
{},
);

expect(options.options.projects[0].name).toBe(firstExpected);
expect(options.options.projects[1].name).toBe(secondExpected);
});

it('picks a name for projects based on the projects rootDir', () => {
const firstRootDir = '/root/path/foo';
const secondRootDir = '/root/path/bar';
const firstExpected = crypto
.createHash('md5')
.update('/root/path/foo')
.digest('hex');
const secondExpected = crypto
.createHash('md5')
.update('/root/path/bar')
.digest('hex');

const options = normalize(
{
projects: [{rootDir: firstRootDir}, {rootDir: secondRootDir}],
rootDir: '/root/path/baz',
},
{},
);

expect(options.options.projects[0].name).toBe(firstExpected);
expect(options.options.projects[1].name).toBe(secondExpected);
});

it('keeps custom project name based on the projects rootDir', () => {
const name = 'test';
const options = normalize(
{
projects: [{name, rootDir: '/path/to/foo'}],
rootDir: '/root/path/baz',
},
{},
);

expect(options.options.projects[0].name).toBe(name);
});

it('keeps custom names based on the rootDir', () => {
expect(
normalize(
Expand Down
13 changes: 13 additions & 0 deletions packages/jest-config/src/normalize.js
Original file line number Diff line number Diff line change
Expand Up @@ -236,6 +236,19 @@ const normalizeMissingOptions = (options: InitialOptions): InitialOptions => {
.digest('hex');
}

if (Array.isArray(options.projects)) {
options.projects = options.projects.map((project, index) => {
Copy link
Collaborator

Choose a reason for hiding this comment

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

normalize is run on every project config, once per run, data is not persisted. I don't think it's necessary to double loop like this. Seems like simply adding a timestamp to the equation fixes the problem (or maybe I'm missing a bigger picture @SimenB):

diff --git a/packages/jest-config/src/normalize.js b/packages/jest-config/src/normalize.js
index 713c8b0da..11d61e080 100644
--- a/packages/jest-config/src/normalize.js
+++ b/packages/jest-config/src/normalize.js
@@ -270,6 +270,7 @@ const normalizeMissingOptions = (options: InitialOptions): InitialOptions => {
     options.name = crypto
       .createHash('md5')
       .update(options.rootDir)
+      .update(Date.now().toString())
       .digest('hex');
   }

Copy link
Member

@SimenB SimenB Jan 3, 2019

Choose a reason for hiding this comment

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

Math.random() seems safer in case the loop completes in less than 1ms, but I agree that should probably solve it as well

Copy link
Collaborator

Choose a reason for hiding this comment

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

Agreed. I'd go with uuid/v4 for generating random number though, as Math.random is not so random after all and I'd hate to debug issue like this 😅.
Tested it locally and it fixes the issue (on Node 8, this particular example is not reproducible on Node 10)

Copy link
Member

Choose a reason for hiding this comment

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

@thymikee which double loop do you mean? It's just a single loop through projects, isn't it?

Copy link
Collaborator

Choose a reason for hiding this comment

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

@thymikee which double loop do you mean? It's just a single loop through projects, isn't it?

normalize is run for every project config, so essentially we loop through projects once again here, hence double loop. And the solution I proposed doesn't have this issue.

Copy link
Member

Choose a reason for hiding this comment

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

Push it? 😀 I fixed my Set comment, would be nice to get this fix into Jest 24

Copy link
Collaborator

Choose a reason for hiding this comment

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

Done. I checked the repro and it still works as expected. I should probably add an e2e test to be sure.

if (typeof project !== 'string' && !project.name) {
project.name = crypto
Copy link
Member

Choose a reason for hiding this comment

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

.createHash('md5')
.update(project.rootDir || `${options.rootDir}:${index}`)
SimenB marked this conversation as resolved.
Show resolved Hide resolved
.digest('hex');
}

return project;
});
}

if (!options.setupFiles) {
options.setupFiles = [];
}
Expand Down