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

Normalize --findRelatedTests paths on win32 platforms #8961

Merged
merged 1 commit into from Jan 17, 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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Expand Up @@ -13,6 +13,7 @@
- `[jest-config]` Merge preset globals with project globals ([#9027](https://github.com/facebook/jest/pull/9027))
- `[jest-config]` Support `.cjs` config files ([#9291](https://github.com/facebook/jest/pull/9291))
- `[jest-core]` Support reporters as default exports ([#9161](https://github.com/facebook/jest/pull/9161))
- `[jest-core]` Support `--findRelatedTests` paths case insensitivity on Windows ([#8900](https://github.com/facebook/jest/issues/8900))
- `[jest-diff]` Add options for colors and symbols ([#8841](https://github.com/facebook/jest/pull/8841))
- `[jest-diff]` [**BREAKING**] Export as ECMAScript module ([#8873](https://github.com/facebook/jest/pull/8873))
- `[jest-diff]` Add `includeChangeCounts` and rename `Indicator` options ([#8881](https://github.com/facebook/jest/pull/8881))
Expand Down
26 changes: 26 additions & 0 deletions e2e/__tests__/findRelatedFiles.test.ts
Expand Up @@ -38,6 +38,32 @@ describe('--findRelatedTests flag', () => {
expect(stderr).toMatch(summaryMsg);
});

test('runs tests related to uppercased filename on case-insensitive os', () => {
if (process.platform !== 'win32') {
// This test is Windows specific, skip it on other platforms.
return;
}

writeFiles(DIR, {
'.watchmanconfig': '',
'__tests__/test.test.js': `
const a = require('../a');
test('a', () => {});
`,
'a.js': 'module.exports = {};',
'package.json': JSON.stringify({jest: {testEnvironment: 'node'}}),
});

const {stdout} = runJest(DIR, ['A.JS']);
expect(stdout).toMatch('');

const {stderr} = runJest(DIR, ['--findRelatedTests', 'A.JS']);
expect(stderr).toMatch('PASS __tests__/test.test.js');

const summaryMsg = 'Ran all test suites related to files matching /A.JS/i.';
expect(stderr).toMatch(summaryMsg);
});

test('runs tests related to filename with a custom dependency extractor', () => {
writeFiles(DIR, {
'.watchmanconfig': '',
Expand Down
19 changes: 16 additions & 3 deletions packages/jest-core/src/SearchSource.ts
Expand Up @@ -5,6 +5,7 @@
* LICENSE file in the root directory of this source tree.
*/

import * as os from 'os';
import * as path from 'path';
import micromatch = require('micromatch');
import {Context} from 'jest-runtime';
Expand Down Expand Up @@ -252,8 +253,6 @@ export default class SearchSource {
globalConfig: Config.GlobalConfig,
changedFiles?: ChangedFiles,
): SearchResult {
const paths = globalConfig.nonFlagArgs;

if (globalConfig.onlyChanged) {
if (!changedFiles) {
throw new Error('Changed files must be set when running with -o.');
Expand All @@ -263,7 +262,21 @@ export default class SearchSource {
changedFiles,
globalConfig.collectCoverage,
);
} else if (globalConfig.runTestsByPath && paths && paths.length) {
}

let paths = globalConfig.nonFlagArgs;

if (globalConfig.findRelatedTests && 'win32' === os.platform()) {
Copy link
Member

Choose a reason for hiding this comment

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

mac by default is also case insensitive, but I'm not sure if it's possible to detect it somehow beyond checking the os?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Unfortunately I don't know any api that could be used to check it

const allFiles = this._context.hasteFS.getAllFiles();
const options = {nocase: true, windows: false};
thymikee marked this conversation as resolved.
Show resolved Hide resolved

paths = paths
.map(p => path.resolve(this._context.config.cwd, p))
.map(p => micromatch(allFiles, p.replace(/\\/g, '\\\\'), options)[0])
.filter(p => p);
}

if (globalConfig.runTestsByPath && paths && paths.length) {
jeysal marked this conversation as resolved.
Show resolved Hide resolved
return this.findTestsByPaths(paths);
} else if (globalConfig.findRelatedTests && paths && paths.length) {
return this.findRelatedTestsFromPattern(
Expand Down