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: verify rootDir and all roots are directories #9569

Merged
merged 7 commits into from
Feb 12, 2020
Merged
Show file tree
Hide file tree
Changes from all 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
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@

- `[jest-config]` Treat `setupFilesAfterEnv` like `setupFiles` when normalizing configs against presets ([#9495](https://github.com/facebook/jest/pull/9495))
- `[jest-config]` Support `.mjs` config files on Windows as well ([#9558](https://github.com/facebook/jest/pull/9558))
- `[jest-config]` Verify `rootDir` and all `roots` are directories ([#9569](https://github.com/facebook/jest/pull/9569))
- `[jest-cli]` Set `coverageProvider` correctly when provided in config ([#9562](https://github.com/facebook/jest/pull/9562))
- `[jest-matcher-utils]` Fix diff highlight of symbol-keyed object. ([#9499](https://github.com/facebook/jest/pull/9499))
- `[@jest/reporters]` Notifications should be fire&forget rather than having a timeout ([#9567](https://github.com/facebook/jest/pull/9567))
Expand Down
76 changes: 76 additions & 0 deletions e2e/__tests__/existentRoots.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,76 @@
/**
* Copyright (c) Facebook, Inc. and its affiliates. All Rights Reserved.
*
* This source code is licensed under the MIT license found in the
* LICENSE file in the root directory of this source tree.
*/

import * as path from 'path';
import {tmpdir} from 'os';
import {skipSuiteOnWindows} from '@jest/test-utils';
import {cleanup, writeFiles} from '../Utils';
import runJest from '../runJest';

const DIR = path.resolve(tmpdir(), 'existent-roots');

beforeEach(() => cleanup(DIR));
afterAll(() => cleanup(DIR));

skipSuiteOnWindows();

function writeConfig(rootDir: string, roots?: Array<string>) {
writeFiles(DIR, {
'jest.config.js': `
module.exports = ${JSON.stringify({rootDir, roots}, null, 2)};
`,
'package.json': '{}',
});
}

test('error when rootDir does not exist', () => {
const fakeRootDir = path.join(DIR, 'foobar');
writeConfig(fakeRootDir);

const {exitCode, stderr} = runJest(DIR);

expect(exitCode).toBe(1);
expect(stderr).toContain(
`Directory ${fakeRootDir} in the rootDir option was not found.`,
);
});

test('error when rootDir is a file', () => {
const fakeRootDir = path.join(DIR, 'jest.config.js');
writeConfig(fakeRootDir);

const {exitCode, stderr} = runJest(DIR);

expect(exitCode).toBe(1);
expect(stderr).toContain(
`${fakeRootDir} in the rootDir option is not a directory.`,
);
});

test('error when roots directory does not exist', () => {
const fakeRootDir = path.join(DIR, 'foobar');
writeConfig(DIR, ['<rootDir>', fakeRootDir]);

const {exitCode, stderr} = runJest(DIR);

expect(exitCode).toBe(1);
expect(stderr).toContain(
`Directory ${fakeRootDir} in the roots[1] option was not found.`,
);
});

test('error when roots is a file', () => {
const fakeRootDir = path.join(DIR, 'jest.config.js');
writeConfig(DIR, ['<rootDir>', fakeRootDir]);

const {exitCode, stderr} = runJest(DIR);

expect(exitCode).toBe(1);
expect(stderr).toContain(
`${fakeRootDir} in the roots[1] option is not a directory.`,
);
});
12 changes: 11 additions & 1 deletion packages/jest-config/src/__tests__/normalize.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,17 @@ import {DEFAULT_JS_PATTERN} from '../constants';

const DEFAULT_CSS_PATTERN = '^.+\\.(css)$';

jest.mock('jest-resolve').mock('path', () => jest.requireActual('path').posix);
jest
.mock('jest-resolve')
.mock('path', () => jest.requireActual('path').posix)
.mock('fs', () => {
const realFs = jest.requireActual('fs');

return {
...realFs,
statSync: () => ({isDirectory: () => true}),
};
});

let root;
let expectedPathFooBar;
Expand Down
40 changes: 40 additions & 0 deletions packages/jest-config/src/normalize.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
*/

import {createHash} from 'crypto';
import {statSync} from 'fs';
import * as path from 'path';
import {sync as glob} from 'glob';
import {Config} from '@jest/types';
Expand Down Expand Up @@ -46,6 +47,39 @@ type AllOptions = Config.ProjectConfig & Config.GlobalConfig;
const createConfigError = (message: string) =>
new ValidationError(ERROR, message, DOCUMENTATION_NOTE);

function verifyDirectoryExists(path: Config.Path, key: string) {
try {
const rootStat = statSync(path);

if (!rootStat.isDirectory()) {
throw createConfigError(
` ${chalk.bold(path)} in the ${chalk.bold(
key,
)} option is not a directory.`,
);
}
} catch (err) {
if (err instanceof ValidationError) {
throw err;
Copy link
Member Author

@SimenB SimenB Feb 12, 2020

Choose a reason for hiding this comment

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

printing a warning instead of these 3 throws is trivial if we don't want a breaking change. However, I think non-existent directories provided to these options are so blatantly a configuration error that I think it's fine to throw

}

if (err.code === 'ENOENT') {
throw createConfigError(
` Directory ${chalk.bold(path)} in the ${chalk.bold(
key,
)} option was not found.`,
);
}

// Not sure in which cases `statSync` can throw, so let's just show the underlying error to the user
throw createConfigError(
` Got an error trying to find ${chalk.bold(path)} in the ${chalk.bold(
key,
)} option.\n\n Error was: ${err.message}`,
);
}
}

// TS 3.5 forces us to split these into 2
const mergeModuleNameMapperWithPreset = (
options: Config.InitialOptionsWithRootDir,
Expand Down Expand Up @@ -366,6 +400,8 @@ const normalizeRootDir = (
// ignored
}

verifyDirectoryExists(options.rootDir, 'rootDir');

return {
...options,
rootDir: options.rootDir,
Expand Down Expand Up @@ -916,6 +952,10 @@ export default function normalize(
return newOptions;
}, newOptions);

newOptions.roots.forEach((root, i) => {
verifyDirectoryExists(root, `roots[${i}]`);
});

try {
// try to resolve windows short paths, ignoring errors (permission errors, mostly)
newOptions.cwd = realpath(process.cwd());
Expand Down
15 changes: 15 additions & 0 deletions packages/jest-core/src/__tests__/SearchSource.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,21 @@ import SearchSource, {SearchResult} from '../SearchSource';

jest.setTimeout(15000);

jest.mock('fs', () => {
const realFs = jest.requireActual('fs');

return {
...realFs,
statSync: path => {
if (path === '/foo/bar/prefix') {
return {isDirectory: () => true};
}

return realFs.statSync(path);
},
};
});

const rootDir = path.resolve(__dirname, 'test_root');
const testRegex = path.sep + '__testtests__' + path.sep;
const testMatch = ['**/__testtests__/**/*'];
Expand Down