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

Allow configuration object in projects array #5176

Merged
merged 10 commits into from
Dec 28, 2017
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
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
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@
([#5154](https://github.com/facebook/jest/pull/5154))
* `[jest-jasmine2]` Support generator functions as specs.
([#5166](https://github.com/facebook/jest/pull/5166))
* `[jest-config]` Allow configuration objects inside `projects` array
([#5176](https://github.com/facebook/jest/pull/5176))

### Chore & Maintenance

Expand Down
25 changes: 25 additions & 0 deletions integration_tests/__tests__/multi_project_runner.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -154,6 +154,31 @@ test('"No tests found" message for projects', () => {
);
});

test('objects in project configuration', () => {
writeFiles(DIR, {
'__tests__/file1.test.js': `
test('foo', () => {});
`,
'__tests__/file2.test.js': `
test('foo', () => {});
`,
'jest.config.js': `module.exports = {
projects: [
{ testMatch: ['<rootDir>/__tests__/file1.test.js$'] },
{ testMatch: ['<rootDir>/__tests__/file2.test.js$'] },
]
};`,
'package.json': '{}',
});

const {stdout, stderr} = runJest(DIR);
expect(stderr).toEqual('');
expect(stdout).toContain(
' 2 files checked across 2 projects. ' +
'Run with `--verbose` for more details.',
);
});

test('resolves projects and their <rootDir> properly', () => {
writeFiles(DIR, {
'.watchmanconfig': '',
Expand Down
10 changes: 8 additions & 2 deletions packages/jest-cli/src/cli/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -199,7 +199,9 @@ const ensureNoDuplicateConfigs = (parsedConfigs, projects) => {
});
throw new Error(message);
}
configPathSet.add(configPath);
if (configPath !== null) {
configPathSet.add(configPath);
}
}
};

Expand All @@ -225,9 +227,11 @@ const getConfigs = (
let hasDeprecationWarnings;
let configs: Array<ProjectConfig> = [];
let projects = projectsFromCLIArgs;
let configPath: ?Path;

if (projectsFromCLIArgs.length === 1) {
const parsedConfig = readConfig(argv, projects[0]);
configPath = parsedConfig.configPath;

if (parsedConfig.globalConfig.projects) {
// If this was a single project, and its config has `projects`
Expand All @@ -246,7 +250,9 @@ const getConfigs = (
}

if (projects.length > 1) {
const parsedConfigs = projects.map(root => readConfig(argv, root, true));
const parsedConfigs = projects.map(root =>
readConfig(argv, root, true, configPath)
);
ensureNoDuplicateConfigs(parsedConfigs, projects);
configs = parsedConfigs.map(({projectConfig}) => projectConfig);
if (!hasDeprecationWarnings) {
Expand Down
24 changes: 17 additions & 7 deletions packages/jest-config/src/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,24 +17,34 @@ import readConfigFileAndSetRootDir from './read_config_file_and_set_root_dir';

function readConfig(
argv: Argv,
packageRoot: string,
packageRootOrConfig: Path | ProjectConfig,
// Whether it needs to look into `--config` arg passed to CLI.
// 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.
skipArgvConfigOption?: boolean,
parentConfigPath?: ?Path,
Copy link
Member

Choose a reason for hiding this comment

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

what does ? on both sides mean? From my understanding name: ?type allows null, undefined or a value, while name?: type allows undefined or a value.

https://flow.org/en/docs/types/primitives/#toc-maybe-types

Copy link
Contributor Author

@azz azz Dec 27, 2017

Choose a reason for hiding this comment

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

I'm not very proficient with Flow. I just added the ? on the LHS to satisfy the checker. Maybe I can remove the one left, but the property is optional so I think I'll get an error when it's not explicitly passed.

Copy link
Member

Choose a reason for hiding this comment

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

Yeah, I think so 🙂

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removing it worked.

): {
configPath: ?Path,
globalConfig: GlobalConfig,
hasDeprecationWarnings: boolean,
projectConfig: ProjectConfig,
} {
let rawOptions;
let configPath;
let configPath = null;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Is there anywhere that requires a configPath to be set other than readConfigFileAndSetRootDir?


// A JSON string was passed to `--config` argument and we can parse it
// and use as is.
if (isJSONString(argv.config)) {
if (typeof packageRootOrConfig !== 'string') {
if (parentConfigPath) {
rawOptions = packageRootOrConfig;
rawOptions.rootDir = parentConfigPath;
} else {
throw new Error(
Copy link
Member

Choose a reason for hiding this comment

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

can we have a test for this (mostly for coverage)?

'jest: Cannot use configuration as an object without a file path',
Copy link
Member

Choose a reason for hiding this comment

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

Can you end this sentence with a full stop (.) ?

Also, please capitalize Jest.

);
}
} else if (isJSONString(argv.config)) {
// A JSON string was passed to `--config` argument and we can parse it
// and use as is.
let config;
try {
config = JSON.parse(argv.config);
Expand All @@ -45,7 +55,7 @@ function readConfig(
}

// NOTE: we might need to resolve this dir to an absolute path in the future
config.rootDir = config.rootDir || packageRoot;
config.rootDir = config.rootDir || packageRootOrConfig;
rawOptions = config;
// 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`
Expand All @@ -54,7 +64,7 @@ function readConfig(
rawOptions = readConfigFileAndSetRootDir(configPath);
} else {
// Otherwise just try to find config in the current rootDir.
configPath = resolveConfigPath(packageRoot, process.cwd());
configPath = resolveConfigPath(packageRootOrConfig, process.cwd());
rawOptions = readConfigFileAndSetRootDir(configPath);
}

Expand Down
3 changes: 2 additions & 1 deletion packages/jest-config/src/normalize.js
Original file line number Diff line number Diff line change
Expand Up @@ -437,7 +437,8 @@ export default function normalize(options: InitialOptions, argv: Argv) {
// Project can be specified as globs. If a glob matches any files,
// We expand it to these paths. If not, we keep the original path
// for the future resolution.
const globMatches = glob.sync(project);
const globMatches =
typeof project === 'string' ? glob.sync(project) : [];
return projects.concat(globMatches.length ? globMatches : project);
}, []);
break;
Expand Down