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 bail setting to control when to bail out of a failing test run #7335

Merged
merged 1 commit into from
Dec 6, 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 @@ -34,6 +34,7 @@
- `[jest-config]` Add `haste.computeSha1` option to compute the sha-1 of the files in the haste map ([#7345](https://github.com/facebook/jest/pull/7345))
- `[expect]` `expect(Infinity).toBeCloseTo(Infinity)` Treats `Infinity` as equal in toBeCloseTo matcher ([#7405](https://github.com/facebook/jest/pull/7405))
- `[jest-worker]` Add node worker-thread support to jest-worker ([#7408](https://github.com/facebook/jest/pull/7408))
- `[jest-config]` Allow `bail` setting to be configured with a number allowing tests to abort after `n` of failures ([#7335](https://github.com/facebook/jest/pull/7335))

### Fixes

Expand Down
2 changes: 1 addition & 1 deletion TestUtils.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@
import type {GlobalConfig, ProjectConfig} from 'types/Config';

const DEFAULT_GLOBAL_CONFIG: GlobalConfig = {
bail: false,
bail: 0,
changedFilesWithAncestor: false,
changedSince: '',
collectCoverage: false,
Expand Down
2 changes: 1 addition & 1 deletion docs/CLI.md
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,7 @@ When you run `jest` with an argument, that argument is treated as a regular expr

### `--bail`

Alias: `-b`. Exit the test suite immediately upon the first failing test suite.
Alias: `-b`. Exit the test suite immediately upon `n` number of failing test suite. Defaults to `1`.

### `--cache`

Expand Down
8 changes: 4 additions & 4 deletions docs/Configuration.md
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ When using the `--config` option, the JSON file must not contain a "jest" key:

```json
{
"bail": true,
"bail": 1,
"verbose": true
}
```
Expand Down Expand Up @@ -99,11 +99,11 @@ _Note: Core modules, like `fs`, are not mocked by default. They can be mocked ex

_Note: Automocking has a performance cost most noticeable in large projects. See [here](troubleshooting.html#tests-are-slow-when-leveraging-automocking) for details and a workaround._

### `bail` [boolean]
### `bail` [number | boolean]
SimenB marked this conversation as resolved.
Show resolved Hide resolved

Default: `false`
Default: `0`

By default, Jest runs all tests and produces all errors into the console upon completion. The bail config option can be used here to have Jest stop running tests after the first failure.
By default, Jest runs all tests and produces all errors into the console upon completion. The bail config option can be used here to have Jest stop running tests after `n` failures. Setting bail to `true` is the same as setting bail to `1`.

### `browser` [boolean]

Expand Down
3 changes: 1 addition & 2 deletions docs/WatchPlugins.md
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
---
id: watch-plugins
title: Watch Plugins
original_id: watch-plugins
---

The Jest watch plugin system provides a way to hook into specific parts of Jest and to define watch mode menu prompts that execute code on key press. Combined, these features allow you to develop interactive experiences custom for your workflow.
Expand Down Expand Up @@ -155,7 +154,7 @@ class MyWatchPlugin {

For stability and safety reasons, only part of the global configuration keys can be updated with `updateConfigAndRun`. The current white list is as follows:

- [`bail`](configuration.html#bail-boolean)
- [`bail`](configuration.html#bail-number-boolean)
- [`collectCoverage`](configuration.html#collectcoverage-boolean)
- [`collectCoverageFrom`](configuration.html#collectcoveragefrom-array)
- [`collectCoverageOnlyFrom`](configuration.html#collectcoverageonlyfrom-array)
Expand Down
2 changes: 1 addition & 1 deletion e2e/__tests__/__snapshots__/show_config.test.js.snap
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ exports[`--showConfig outputs config info and exits 1`] = `
}
],
\\"globalConfig\\": {
\\"bail\\": false,
\\"bail\\": 0,
\\"changedFilesWithAncestor\\": false,
\\"collectCoverage\\": false,
\\"collectCoverageFrom\\": null,
Expand Down
5 changes: 4 additions & 1 deletion packages/jest-cli/src/TestScheduler.js
Original file line number Diff line number Diff line change
Expand Up @@ -351,7 +351,10 @@ export default class TestScheduler {
aggregatedResults: AggregatedResult,
watcher: TestWatcher,
): Promise<void> {
if (this._globalConfig.bail && aggregatedResults.numFailedTests !== 0) {
if (
this._globalConfig.bail !== 0 &&
aggregatedResults.numFailedTests >= this._globalConfig.bail
Copy link
Collaborator

Choose a reason for hiding this comment

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

this will still be true if bail is explicitly set to false. Shouldn't we normalize it just in case?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

There shouldn't be any case where bail gets to this point that isn't a number: https://github.com/facebook/jest/pull/7335/files#diff-7121e12d406ff9a3060a13ee808632de

) {
if (watcher.isWatchMode()) {
watcher.setState({interrupted: true});
} else {
Expand Down
64 changes: 64 additions & 0 deletions packages/jest-cli/src/__tests__/TestScheduler.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,10 @@ jest.mock('jest-runner-parallel', () => jest.fn(() => mockParallelRunner), {
virtual: true,
});

beforeEach(() => {
mockSerialRunner.runTests.mockClear();
});

test('config for reporters supports `default`', () => {
const undefinedReportersScheduler = new TestScheduler(
{
Expand Down Expand Up @@ -116,3 +120,63 @@ test('schedule tests run in serial if the runner flags them', async () => {
expect(mockSerialRunner.runTests).toHaveBeenCalled();
expect(mockSerialRunner.runTests.mock.calls[0][5].serial).toBeTruthy();
});

test('should bail after `n` failures', async () => {
const scheduler = new TestScheduler({bail: 2}, {});
const test = {
context: {
config: {
rootDir: './',
runner: 'jest-runner-serial',
},
hasteFS: {
matchFiles: jest.fn(() => []),
},
},
path: './test/path.js',
};

const tests = [test];
const setState = jest.fn();
await scheduler.scheduleTests(tests, {
isInterrupted: jest.fn(),
isWatchMode: () => true,
setState,
});
await mockSerialRunner.runTests.mock.calls[0][3](test, {
numFailingTests: 2,
snapshot: {},
testResults: [{}],
});
expect(setState).toBeCalledWith({interrupted: true});
});

test('should not bail if less than `n` failures', async () => {
const scheduler = new TestScheduler({bail: 2}, {});
const test = {
context: {
config: {
rootDir: './',
runner: 'jest-runner-serial',
},
hasteFS: {
matchFiles: jest.fn(() => []),
},
},
path: './test/path.js',
};

const tests = [test];
const setState = jest.fn();
await scheduler.scheduleTests(tests, {
isInterrupted: jest.fn(),
isWatchMode: () => true,
setState,
});
await mockSerialRunner.runTests.mock.calls[0][3](test, {
numFailingTests: 1,
snapshot: {},
testResults: [{}],
});
expect(setState).not.toBeCalled();
});
4 changes: 2 additions & 2 deletions packages/jest-cli/src/cli/args.js
Original file line number Diff line number Diff line change
Expand Up @@ -85,8 +85,8 @@ export const options = {
bail: {
alias: 'b',
default: undefined,
description: 'Exit the test suite immediately upon the first failing test.',
type: 'boolean',
rickhanlonii marked this conversation as resolved.
Show resolved Hide resolved
description:
'Exit the test suite immediately after `n` number of failing tests.',
},
browser: {
default: undefined,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,8 +26,8 @@ module.exports = {
// All imported modules in your tests should be mocked automatically
// automock: false,

// Stop running tests after the first failure
// bail: false,
// Stop running tests after \`n\` failures
// bail: 0,

// Respect \\"browser\\" field in package.json when resolving modules
// browser: false,
Expand Down
6 changes: 4 additions & 2 deletions packages/jest-cli/src/lib/update_global_config.js
Original file line number Diff line number Diff line change
Expand Up @@ -66,8 +66,10 @@ export default (globalConfig: GlobalConfig, options: Options): GlobalConfig => {
!newConfig.testNamePattern &&
!newConfig.testPathPattern;

if (options.bail !== undefined) {
newConfig.bail = options.bail || false;
if (typeof options.bail === 'boolean') {
newConfig.bail = options.bail ? 1 : 0;
} else if (options.bail !== undefined) {
newConfig.bail = options.bail;
}

if (options.changedSince !== undefined) {
Expand Down
2 changes: 1 addition & 1 deletion packages/jest-config/src/Defaults.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ const NODE_MODULES_REGEXP = replacePathSepForRegex(NODE_MODULES);

export default ({
automock: false,
bail: false,
bail: 0,
browser: false,
cache: true,
cacheDirectory: getCacheDirectory(),
Expand Down
2 changes: 1 addition & 1 deletion packages/jest-config/src/Descriptions.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@

export default ({
automock: 'All imported modules in your tests should be mocked automatically',
bail: 'Stop running tests after the first failure',
bail: 'Stop running tests after `n` failures',
browser: 'Respect "browser" field in package.json when resolving modules',
cacheDirectory:
'The directory where Jest should store its cached dependency information',
Expand Down
2 changes: 1 addition & 1 deletion packages/jest-config/src/ValidConfig.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ const NODE_MODULES_REGEXP = replacePathSepForRegex(NODE_MODULES);

export default ({
automock: false,
bail: false,
bail: (multipleValidOptions(false, 0): any),
browser: false,
cache: true,
cacheDirectory: '/tmp/user/jest',
Expand Down
15 changes: 14 additions & 1 deletion packages/jest-config/src/normalize.js
Original file line number Diff line number Diff line change
Expand Up @@ -617,8 +617,21 @@ export default function normalize(options: InitialOptions, argv: Argv) {

break;
}
case 'bail': {
if (typeof options[key] === 'boolean') {
value = options[key] ? 1 : 0;
} else if (typeof options[key] === 'string') {
value = 1;
// If Jest is invoked as `jest --bail someTestPattern` then need to
thymikee marked this conversation as resolved.
Show resolved Hide resolved
// move the pattern from the `bail` configuration and into `argv._`
// to be processed as an extra parameter
argv._.push(options[key]);
Copy link
Collaborator

Choose a reason for hiding this comment

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

can't we just remove this line? looks fishy

} else {
value = options[key];
}
break;
}
case 'automock':
case 'bail':
case 'browser':
case 'cache':
case 'changedSince':
Expand Down
4 changes: 2 additions & 2 deletions packages/jest-validate/src/__tests__/fixtures/jestConfig.js
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ const NODE_MODULES_REGEXP = replacePathSepForRegex(NODE_MODULES);

const defaultConfig = {
automock: false,
bail: false,
bail: 0,
browser: false,
cacheDirectory: path.join(os.tmpdir(), 'jest'),
clearMocks: false,
Expand Down Expand Up @@ -62,7 +62,7 @@ const defaultConfig = {

const validConfig = {
automock: false,
bail: false,
bail: 0,
browser: false,
cache: true,
cacheDirectory: '/tmp/user/jest',
Expand Down
2 changes: 1 addition & 1 deletion types/Argv.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ export type Argv = {|
$0: string,
all: boolean,
automock: boolean,
bail: boolean,
bail: boolean | number,
browser: boolean,
cache: boolean,
cacheDirectory: string,
Expand Down
6 changes: 3 additions & 3 deletions types/Config.js
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ export type ConfigGlobals = Object;

export type DefaultOptions = {|
automock: boolean,
bail: boolean,
bail: number,
browser: boolean,
cache: boolean,
cacheDirectory: Path,
Expand Down Expand Up @@ -92,7 +92,7 @@ export type DefaultOptions = {|

export type InitialOptions = {
automock?: boolean,
bail?: boolean,
bail?: boolean | number,
browser?: boolean,
cache?: boolean,
cacheDirectory?: Path,
Expand Down Expand Up @@ -190,7 +190,7 @@ export type InitialOptions = {
export type SnapshotUpdateState = 'all' | 'new' | 'none';

export type GlobalConfig = {|
bail: boolean,
bail: number,
thymikee marked this conversation as resolved.
Show resolved Hide resolved
changedSince: string,
changedFilesWithAncestor: boolean,
collectCoverage: boolean,
Expand Down