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

feat(jest-circus,jest-jasmine2): throw when describe returns a value #10947

Merged
merged 3 commits into from Dec 14, 2020
Merged
Show file tree
Hide file tree
Changes from 2 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 @@ -2,6 +2,7 @@

### Features

- `[jest-circus, jest-jasmine2]` [**BREAKING**] Fail the test instead of just warning when describe returns a value ([#10947](https://github.com/facebook/jest/pull/10947))
- `[jest-config]` [**BREAKING**] Default to Node testing environment instead of browser (JSDOM) ([#9874](https://github.com/facebook/jest/pull/9874))
- `[jest-config]` [**BREAKING**] Use `jest-circus` as default test runner ([#10686](https://github.com/facebook/jest/pull/10686))
- `[jest-config, jest-runtime]` Support ESM for files other than `.js` and `.mjs` ([#10823](https://github.com/facebook/jest/pull/10823))
Expand Down
54 changes: 22 additions & 32 deletions e2e/__tests__/__snapshots__/declarationErrors.test.ts.snap
@@ -1,41 +1,31 @@
// Jest Snapshot v1, https://goo.gl/fbAQLP

exports[`warns if describe returns a Promise 1`] = `
console.log
● Test suite failed to run

Returning a Promise from "describe" is not supported. Tests must be defined synchronously.
Returning a value from "describe" will fail the test in a future version of Jest.

9 | 'use strict';
10 |
> 11 | describe('Promise describe warns', () => {
| ^
12 | it('t', () => {});
13 | return Promise.resolve();
14 | });

at Object.describe (__tests__/describeReturnPromise.test.js:11:1)
exports[`errors if describe returns a Promise 1`] = `
Returning a Promise from "describe" is not supported. Tests must be defined synchronously.
Returning a value from "describe" will fail the test in a future version of Jest.
jeysal marked this conversation as resolved.
Show resolved Hide resolved

9 | 'use strict';
10 |
> 11 | describe('Promise describe errors', () => {
| ^
12 | it('t', () => {});
13 | return Promise.resolve();
14 | });

at Object.describe (__tests__/describeReturnPromise.test.js:11:1)
`;

exports[`warns if describe returns something 1`] = `
console.log
● Test suite failed to run

A "describe" callback must not return a value.
Returning a value from "describe" will fail the test in a future version of Jest.

9 | 'use strict';
10 |
> 11 | describe('describe return warns', () => {
| ^
12 | it('t', () => {});
13 | return 42;
14 | });

at Object.describe (__tests__/describeReturnSomething.test.js:11:1)
exports[`errors if describe returns something 1`] = `
A "describe" callback must not return a value.
Returning a value from "describe" will fail the test in a future version of Jest.

9 | 'use strict';
10 |
> 11 | describe('describe return errors', () => {
| ^
12 | it('t', () => {});
13 | return 42;
14 | });

at Object.describe (__tests__/describeReturnSomething.test.js:11:1)
`;
30 changes: 18 additions & 12 deletions e2e/__tests__/declarationErrors.test.ts
Expand Up @@ -7,37 +7,43 @@

import wrap from 'jest-snapshot-serializer-raw';
import runJest from '../runJest';
import {extractSummary} from '../Utils';

const normalizeCircusJasmine = (str: string) =>
const extractMessage = (str: string) =>
wrap(
str
.replace(/console\.log .+:\d+/, 'console.log')
.replace(/.+addSpecsToSuite (.+:\d+:\d+).+\n/g, '')
.replace(/.+_dispatchDescribe (.+:\d+:\d+).+\n/g, ''),
extractSummary(str)
.rest.replace(
// circus-jasmine normalization
/.+addSpecsToSuite (.+:\d+:\d+).+\n/g,
'',
)
.match(
// all lines from the first to the last mentioned "describe" after the "●" line
/●(.|\n)*?\n(?<lines>.*describe((.|\n)*describe)*.*)(\n|$)/im,
jeysal marked this conversation as resolved.
Show resolved Hide resolved
)?.groups?.lines ?? '',
);

it('warns if describe returns a Promise', () => {
it('errors if describe returns a Promise', () => {
const result = runJest('declaration-errors', [
'describeReturnPromise.test.js',
]);

expect(result.exitCode).toBe(0);
expect(normalizeCircusJasmine(result.stdout)).toMatchSnapshot();
expect(result.exitCode).toBe(1);
expect(extractMessage(result.stderr)).toMatchSnapshot();
});

it('warns if describe returns something', () => {
it('errors if describe returns something', () => {
const result = runJest('declaration-errors', [
'describeReturnSomething.test.js',
]);

expect(result.exitCode).toBe(0);
expect(normalizeCircusJasmine(result.stdout)).toMatchSnapshot();
expect(result.exitCode).toBe(1);
expect(extractMessage(result.stderr)).toMatchSnapshot();
});

it('errors if describe throws', () => {
const result = runJest('declaration-errors', ['describeThrow.test.js']);

expect(result.exitCode).toBe(1);
expect(result.stdout).toBe('');
expect(result.stderr).toContain('whoops');
});
Expand Up @@ -8,7 +8,7 @@

'use strict';

describe('Promise describe warns', () => {
describe('Promise describe errors', () => {
it('t', () => {});
return Promise.resolve();
});
Expand Up @@ -8,7 +8,7 @@

'use strict';

describe('describe return warns', () => {
describe('describe return errors', () => {
it('t', () => {});
return 42;
});
37 changes: 8 additions & 29 deletions packages/jest-circus/src/index.ts
Expand Up @@ -5,10 +5,8 @@
* LICENSE file in the root directory of this source tree.
*/

import chalk = require('chalk');
import type {Circus, Global} from '@jest/types';
import {bind as bindEach} from 'jest-each';
import {formatExecError} from 'jest-message-util';
import {ErrorWithStack, isPromise} from 'jest-util';
import {dispatchSync} from './state';

Expand Down Expand Up @@ -60,36 +58,17 @@ const _dispatchDescribe = (
});
const describeReturn = blockFn();

// TODO throw in Jest 25
if (isPromise(describeReturn)) {
// eslint-disable-next-line no-console
console.log(
formatExecError(
new ErrorWithStack(
chalk.yellow(
'Returning a Promise from "describe" is not supported. Tests must be defined synchronously.\n' +
'Returning a value from "describe" will fail the test in a future version of Jest.',
),
describeFn,
),
{rootDir: '', testMatch: []},
{noStackTrace: false},
),
throw new ErrorWithStack(
jeysal marked this conversation as resolved.
Show resolved Hide resolved
'Returning a Promise from "describe" is not supported. Tests must be defined synchronously.\n' +
'Returning a value from "describe" will fail the test in a future version of Jest.',
describeFn,
);
} else if (describeReturn !== undefined) {
// eslint-disable-next-line no-console
console.log(
formatExecError(
new ErrorWithStack(
chalk.yellow(
'A "describe" callback must not return a value.\n' +
'Returning a value from "describe" will fail the test in a future version of Jest.',
),
describeFn,
),
{rootDir: '', testMatch: []},
{noStackTrace: false},
),
throw new ErrorWithStack(
'A "describe" callback must not return a value.\n' +
'Returning a value from "describe" will fail the test in a future version of Jest.',
describeFn,
);
}

Expand Down
33 changes: 6 additions & 27 deletions packages/jest-jasmine2/src/jasmine/Env.ts
Expand Up @@ -31,8 +31,6 @@ WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
/* eslint-disable sort-keys, local/prefer-spread-eventually, local/prefer-rest-params-eventually */

import {AssertionError} from 'assert';
import chalk = require('chalk');
import {formatExecError} from 'jest-message-util';
import {ErrorWithStack, isPromise} from 'jest-util';
import assertionErrorMessage from '../assertionErrorMessage';
import isError from '../isError';
Expand Down Expand Up @@ -444,34 +442,15 @@ export default function (j$: Jasmine) {
declarationError = e;
}

// TODO throw in Jest 25: declarationError = new Error
if (isPromise(describeReturnValue)) {
// eslint-disable-next-line no-console
console.log(
formatExecError(
new Error(
chalk.yellow(
'Returning a Promise from "describe" is not supported. Tests must be defined synchronously.\n' +
'Returning a value from "describe" will fail the test in a future version of Jest.',
),
),
{rootDir: '', testMatch: []},
{noStackTrace: false},
),
declarationError = new Error(
'Returning a Promise from "describe" is not supported. Tests must be defined synchronously.\n' +
'Returning a value from "describe" will fail the test in a future version of Jest.',
);
} else if (describeReturnValue !== undefined) {
// eslint-disable-next-line no-console
console.log(
formatExecError(
new Error(
chalk.yellow(
'A "describe" callback must not return a value.\n' +
'Returning a value from "describe" will fail the test in a future version of Jest.',
),
),
{rootDir: '', testMatch: []},
{noStackTrace: false},
),
declarationError = new Error(
'A "describe" callback must not return a value.\n' +
'Returning a value from "describe" will fail the test in a future version of Jest.',
);
}

Expand Down