From 6bade2deff33c239153e3a3f86b850b2fc276d5f Mon Sep 17 00:00:00 2001 From: Kevin Huynh Date: Sun, 7 Oct 2018 13:01:37 -0700 Subject: [PATCH 1/6] Fix unhandled error when a bad revision is provided to `changedSince` --- packages/jest-changed-files/package.json | 1 + packages/jest-changed-files/src/index.js | 52 +++++++++++++++++------- 2 files changed, 39 insertions(+), 14 deletions(-) diff --git a/packages/jest-changed-files/package.json b/packages/jest-changed-files/package.json index cabb73c57560..4cea86b197d0 100644 --- a/packages/jest-changed-files/package.json +++ b/packages/jest-changed-files/package.json @@ -9,6 +9,7 @@ "main": "build/index.js", "dependencies": { "execa": "^1.0.0", + "jest-message-util": "^23.4.0", "throat": "^4.0.0" } } diff --git a/packages/jest-changed-files/src/index.js b/packages/jest-changed-files/src/index.js index a1aedfab904e..f4636cc7ab8a 100644 --- a/packages/jest-changed-files/src/index.js +++ b/packages/jest-changed-files/src/index.js @@ -13,6 +13,7 @@ import type {ChangedFilesPromise, Options, Repos} from 'types/ChangedFiles'; import git from './git'; import hg from './hg'; import throat from 'throat'; +import {formatExecError} from 'jest-message-util'; // This is an arbitrary number. The main goal is to prevent projects with // many roots (50+) from spawning too many processes at once. @@ -29,23 +30,46 @@ export const getChangedFilesForRoots = async ( const changedFilesOptions = Object.assign({}, {includePaths: roots}, options); - const gitPromises = Array.from(repos.git).map(repo => - git.findChangedFiles(repo, changedFilesOptions), - ); + let changedFiles = new Set([]); - const hgPromises = Array.from(repos.hg).map(repo => - hg.findChangedFiles(repo, changedFilesOptions), - ); + try { + const gitPromises = Array.from(repos.git).map(repo => + git.findChangedFiles(repo, changedFilesOptions), + ); + + const hgPromises = Array.from(repos.hg).map(repo => + hg.findChangedFiles(repo, changedFilesOptions), + ); + + changedFiles = (await Promise.all(gitPromises.concat(hgPromises))).reduce( + (allFiles, changedFilesInTheRepo) => { + for (const file of changedFilesInTheRepo) { + allFiles.add(file); + } + + return allFiles; + }, + new Set(), + ); + + return {changedFiles, repos}; + } catch (error) { + const formattedError = formatExecError( + error, + {rootDir: '', testMatch: []}, + {noStackTrace: true}, + ) + .split('\n') + .filter(line => !!line); - const changedFiles = (await Promise.all( - gitPromises.concat(hgPromises), - )).reduce((allFiles, changedFilesInTheRepo) => { - for (const file of changedFilesInTheRepo) { - allFiles.add(file); - } + console.error( + `\n\n${formattedError[0]}\n\n${ + formattedError[formattedError.length - 1] + }\n`, + ); - return allFiles; - }, new Set()); + process.exit(1); + } return {changedFiles, repos}; }; From 18aabce9c1dbb90ff345946627e0d838e77b7f5b Mon Sep 17 00:00:00 2001 From: Kevin Huynh Date: Sat, 6 Oct 2018 11:47:23 -0700 Subject: [PATCH 2/6] Add integration tests for handling bad revision values for `changedSince` --- e2e/__tests__/jest_changed_files.test.js | 39 ++++++++++++++++++++++++ 1 file changed, 39 insertions(+) diff --git a/e2e/__tests__/jest_changed_files.test.js b/e2e/__tests__/jest_changed_files.test.js index 4498dd14598b..2d5f822b39a1 100644 --- a/e2e/__tests__/jest_changed_files.test.js +++ b/e2e/__tests__/jest_changed_files.test.js @@ -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(); @@ -243,6 +244,25 @@ 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 -m "first"`, DIR); + + const stderr = runJest(DIR, ['--changedSince=blablabla']).stderr; + + expect(stderr).toMatch( + `\n\n ● Test suite failed to run\n\n fatal: bad revision '^blablabla'\n`, + ); +}); + test('gets changed files for hg', async () => { if (process.env.CI) { // Circle and Travis have very old version of hg (v2, and current @@ -371,3 +391,22 @@ 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 stderr = runJest(DIR, ['--changedSince=blablabla']).stderr; + + expect(stderr).toMatch( + `\n\n ● Test suite failed to run\n\n abort: unknown revision 'blablabla'!\n`, + ); +}); From 718c8fce8b52a66a3f3f5a381f9ef65ba9435fd0 Mon Sep 17 00:00:00 2001 From: Kevin Huynh Date: Sat, 6 Oct 2018 12:16:54 -0700 Subject: [PATCH 3/6] Update changelog --- CHANGELOG.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 4babad50435c..67879a634130 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -28,7 +28,8 @@ - `[jest-circus]` Fail synchronous hook timeouts ([#7074](https://github.com/facebook/jest/pull/7074)) - `[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-circus]` Better error message when a describe block is empty ([#6372] +- `[jest-cli]` Fix unhandled error when a bad revision is provided to `changedSince` ([#7112](https://github.com/facebook/jest/pull/7112)) ### Chore & Maintenance From fbea782af0211931b6baf818e4c77fc531016b30 Mon Sep 17 00:00:00 2001 From: Simen Bekkhus Date: Sun, 7 Oct 2018 22:57:51 +0200 Subject: [PATCH 4/6] fix changelog --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 67879a634130..f758a4212809 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -28,7 +28,7 @@ - `[jest-circus]` Fail synchronous hook timeouts ([#7074](https://github.com/facebook/jest/pull/7074)) - `[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] +- `[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` ([#7112](https://github.com/facebook/jest/pull/7112)) ### Chore & Maintenance From 6e252282b4148409936a70c98a223a103185736f Mon Sep 17 00:00:00 2001 From: Simen Bekkhus Date: Sun, 7 Oct 2018 22:57:16 +0200 Subject: [PATCH 5/6] move error-handling to jest-cli --- .../jest_changed_files.test.js.snap | 21 ++++++++ e2e/__tests__/jest_changed_files.test.js | 16 +++--- packages/jest-changed-files/package.json | 1 - packages/jest-changed-files/src/index.js | 52 +++++-------------- .../jest-cli/src/getChangedFilesPromise.js | 14 +++++ 5 files changed, 56 insertions(+), 48 deletions(-) create mode 100644 e2e/__tests__/__snapshots__/jest_changed_files.test.js.snap diff --git a/e2e/__tests__/__snapshots__/jest_changed_files.test.js.snap b/e2e/__tests__/__snapshots__/jest_changed_files.test.js.snap new file mode 100644 index 000000000000..61d5013154f9 --- /dev/null +++ b/e2e/__tests__/__snapshots__/jest_changed_files.test.js.snap @@ -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'! + +" +`; diff --git a/e2e/__tests__/jest_changed_files.test.js b/e2e/__tests__/jest_changed_files.test.js index 2d5f822b39a1..d0aa0a02d35a 100644 --- a/e2e/__tests__/jest_changed_files.test.js +++ b/e2e/__tests__/jest_changed_files.test.js @@ -254,13 +254,12 @@ test('handles a bad revision for "changedSince", for git', async () => { run(`${GIT} init`, DIR); run(`${GIT} add .`, DIR); - run(`${GIT} commit -m "first"`, DIR); + run(`${GIT} commit --no-gpg-sign -m "first"`, DIR); - const stderr = runJest(DIR, ['--changedSince=blablabla']).stderr; + const {status, stderr} = runJest(DIR, ['--changedSince=blablabla']); - expect(stderr).toMatch( - `\n\n ● Test suite failed to run\n\n fatal: bad revision '^blablabla'\n`, - ); + expect(status).toBe(1); + expect(stderr).toMatchSnapshot(); }); test('gets changed files for hg', async () => { @@ -404,9 +403,8 @@ test('handles a bad revision for "changedSince", for hg', async () => { run(`${HG} add .`, DIR); run(`${HG} commit -m "first"`, DIR); - const stderr = runJest(DIR, ['--changedSince=blablabla']).stderr; + const {status, stderr} = runJest(DIR, ['--changedSince=blablabla']); - expect(stderr).toMatch( - `\n\n ● Test suite failed to run\n\n abort: unknown revision 'blablabla'!\n`, - ); + expect(status).toBe(1); + expect(stderr).toMatchSnapshot(); }); diff --git a/packages/jest-changed-files/package.json b/packages/jest-changed-files/package.json index 4cea86b197d0..cabb73c57560 100644 --- a/packages/jest-changed-files/package.json +++ b/packages/jest-changed-files/package.json @@ -9,7 +9,6 @@ "main": "build/index.js", "dependencies": { "execa": "^1.0.0", - "jest-message-util": "^23.4.0", "throat": "^4.0.0" } } diff --git a/packages/jest-changed-files/src/index.js b/packages/jest-changed-files/src/index.js index f4636cc7ab8a..a1aedfab904e 100644 --- a/packages/jest-changed-files/src/index.js +++ b/packages/jest-changed-files/src/index.js @@ -13,7 +13,6 @@ import type {ChangedFilesPromise, Options, Repos} from 'types/ChangedFiles'; import git from './git'; import hg from './hg'; import throat from 'throat'; -import {formatExecError} from 'jest-message-util'; // This is an arbitrary number. The main goal is to prevent projects with // many roots (50+) from spawning too many processes at once. @@ -30,46 +29,23 @@ export const getChangedFilesForRoots = async ( const changedFilesOptions = Object.assign({}, {includePaths: roots}, options); - let changedFiles = new Set([]); - - try { - const gitPromises = Array.from(repos.git).map(repo => - git.findChangedFiles(repo, changedFilesOptions), - ); - - const hgPromises = Array.from(repos.hg).map(repo => - hg.findChangedFiles(repo, changedFilesOptions), - ); - - changedFiles = (await Promise.all(gitPromises.concat(hgPromises))).reduce( - (allFiles, changedFilesInTheRepo) => { - for (const file of changedFilesInTheRepo) { - allFiles.add(file); - } - - return allFiles; - }, - new Set(), - ); + const gitPromises = Array.from(repos.git).map(repo => + git.findChangedFiles(repo, changedFilesOptions), + ); - return {changedFiles, repos}; - } catch (error) { - const formattedError = formatExecError( - error, - {rootDir: '', testMatch: []}, - {noStackTrace: true}, - ) - .split('\n') - .filter(line => !!line); + const hgPromises = Array.from(repos.hg).map(repo => + hg.findChangedFiles(repo, changedFilesOptions), + ); - console.error( - `\n\n${formattedError[0]}\n\n${ - formattedError[formattedError.length - 1] - }\n`, - ); + const changedFiles = (await Promise.all( + gitPromises.concat(hgPromises), + )).reduce((allFiles, changedFilesInTheRepo) => { + for (const file of changedFilesInTheRepo) { + allFiles.add(file); + } - process.exit(1); - } + return allFiles; + }, new Set()); return {changedFiles, repos}; }; diff --git a/packages/jest-cli/src/getChangedFilesPromise.js b/packages/jest-cli/src/getChangedFilesPromise.js index d8bc8ed59e84..7dc849f1a8c1 100644 --- a/packages/jest-cli/src/getChangedFilesPromise.js +++ b/packages/jest-cli/src/getChangedFilesPromise.js @@ -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, @@ -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:')) + .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); }); } From 47c8a9a2a04dd208521a1db6c026ee7eaf606808 Mon Sep 17 00:00:00 2001 From: Simen Bekkhus Date: Sun, 7 Oct 2018 23:26:14 +0200 Subject: [PATCH 6/6] Update CHANGELOG.md --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index f758a4212809..28fc48be6596 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -29,7 +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` ([#7112](https://github.com/facebook/jest/pull/7112)) +- `[jest-cli]` Fix unhandled error when a bad revision is provided to `changedSince` ([#7115](https://github.com/facebook/jest/pull/7115)) ### Chore & Maintenance