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 unhandled error when a bad revision is provided to changedSince #7115

Merged
merged 6 commits into from
Oct 7, 2018
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 @@ -29,6 +29,7 @@
- `[jest-jasmine2]` Fail synchronous test timeouts ([#7074](https://github.com/facebook/jest/pull/7074))
- `[jest-jasmine2]` Better error message when a describe block is empty ([#6372](https://github.com/facebook/jest/pull/6372))
- `[jest-circus]` Better error message when a describe block is empty ([#6372](https://github.com/facebook/jest/pull/6372))
- `[jest-cli]` Fix unhandled error when a bad revision is provided to `changedSince` ([#7115](https://github.com/facebook/jest/pull/7115))

### Chore & Maintenance

Expand Down
21 changes: 21 additions & 0 deletions e2e/__tests__/__snapshots__/jest_changed_files.test.js.snap
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
// Jest Snapshot v1, https://goo.gl/fbAQLP

exports[`handles a bad revision for "changedSince", for git 1`] = `
"

● Test suite failed to run

fatal: bad revision '^blablabla'

"
`;

exports[`handles a bad revision for "changedSince", for hg 1`] = `
"

● Test suite failed to run

abort: unknown revision 'blablabla'!

"
`;
37 changes: 37 additions & 0 deletions e2e/__tests__/jest_changed_files.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import {
} from '../../packages/jest-changed-files/src';
import {skipSuiteOnWindows} from '../../scripts/ConditionalTest';
import {cleanup, run, writeFiles} from '../Utils';
import runJest from '../runJest';

skipSuiteOnWindows();

Expand Down Expand Up @@ -243,6 +244,24 @@ test('monitors only root paths for git', async () => {
).toEqual(['file2.txt', 'file3.txt']);
});

test('handles a bad revision for "changedSince", for git', async () => {
writeFiles(DIR, {
'.watchmanconfig': '',
'__tests__/file1.test.js': `require('../file1'); test('file1', () => {});`,
'file1.js': 'module.exports = {}',
'package.json': '{}',
});

run(`${GIT} init`, DIR);
run(`${GIT} add .`, DIR);
run(`${GIT} commit --no-gpg-sign -m "first"`, DIR);

const {status, stderr} = runJest(DIR, ['--changedSince=blablabla']);

expect(status).toBe(1);
expect(stderr).toMatchSnapshot();
});

test('gets changed files for hg', async () => {
if (process.env.CI) {
// Circle and Travis have very old version of hg (v2, and current
Expand Down Expand Up @@ -371,3 +390,21 @@ test('monitors only root paths for hg', async () => {
.sort(),
).toEqual(['file2.txt', 'file3.txt']);
});

test('handles a bad revision for "changedSince", for hg', async () => {
writeFiles(DIR, {
'.watchmanconfig': '',
'__tests__/file1.test.js': `require('../file1'); test('file1', () => {});`,
'file1.js': 'module.exports = {}',
'package.json': '{}',
});

run(`${HG} init`, DIR);
run(`${HG} add .`, DIR);
run(`${HG} commit -m "first"`, DIR);

const {status, stderr} = runJest(DIR, ['--changedSince=blablabla']);

expect(status).toBe(1);
expect(stderr).toMatchSnapshot();
});
14 changes: 14 additions & 0 deletions packages/jest-cli/src/getChangedFilesPromise.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@
import type {GlobalConfig, ProjectConfig} from 'types/Config';
import type {ChangedFilesPromise} from 'types/ChangedFiles';
import {getChangedFilesForRoots} from 'jest-changed-files';
import {formatExecError} from 'jest-message-util';
import chalk from 'chalk';

export default (
globalConfig: GlobalConfig,
Expand All @@ -24,6 +26,18 @@ export default (
changedSince: globalConfig.changedSince,
lastCommit: globalConfig.lastCommit,
withAncestor: globalConfig.changedFilesWithAncestor,
}).catch(e => {
const message = formatExecError(e, configs[0], {noStackTrace: true})
.split('\n')
.filter(line => !line || !line.includes('Command failed:'))
Copy link
Contributor Author

@khuyn003 khuyn003 Oct 7, 2018

Choose a reason for hiding this comment

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

@SimenB Just noticed the first part actually includes empty lines. Fixed in PR #7117

Copy link
Member

Choose a reason for hiding this comment

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

That's on purpose - we want to keep the newlines in the output. This is how it looks:
image

The condition is there in case typeof line !== 'string', but thinking about it, that can't happen

.join('\n');

console.error(chalk.red(`\n\n${message}`));

process.exit(1);

// We do process.exit, so this is dead code
return Promise.reject(e);
});
}

Expand Down