Skip to content

Commit

Permalink
fix: throw on nested test declarations (#9828)
Browse files Browse the repository at this point in the history
  • Loading branch information
SimenB authored Apr 17, 2020
1 parent aa64672 commit 3681cca
Show file tree
Hide file tree
Showing 10 changed files with 186 additions and 7 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
### Fixes

- `[expect]` Restore support for passing functions to `toHaveLength` matcher ([#9796](https://github.com/facebook/jest/pull/9796))
- `[jest-circus]` Throw on nested test definitions ([#9828](https://github.com/facebook/jest/pull/9828))
- `[jest-changed-files]` `--only-changed` should include staged files ([#9799](https://github.com/facebook/jest/pull/9799))
- `[jest-each]` `each` will throw an error when called with too many arguments ([#9818](https://github.com/facebook/jest/pull/9818))

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

exports[`print correct error message with nested test definitions inside describe 1`] = `
FAIL __tests__/nestedTestWithinDescribe.js
in describe
✕ outer test
● in describe › outer test
Tests cannot be nested. Test "inner test" cannot run because it is nested within "outer test".
14 | expect(getTruthy()).toBeTruthy();
15 |
> 16 | test('inner test', () => {
| ^
17 | expect(getTruthy()).toBeTruthy();
18 | });
19 | });
at Object.test (__tests__/nestedTestWithinDescribe.js:16:5)
`;

exports[`print correct error message with nested test definitions outside describe 1`] = `
FAIL __tests__/nestedTestOutsideDescribe.js
✕ outer test
● outer test
Tests cannot be nested. Test "inner test" cannot run because it is nested within "outer test".
13 | expect(getTruthy()).toBeTruthy();
14 |
> 15 | test('inner test', () => {
| ^
16 | expect(getTruthy()).toEqual('This test should not have run');
17 | });
18 | });
at Object.test (__tests__/nestedTestOutsideDescribe.js:15:3)
`;
57 changes: 57 additions & 0 deletions e2e/__tests__/nestedTestDefinitions.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,57 @@
/**
* Copyright (c) Facebook, Inc. and its affiliates. All Rights Reserved.
*
* This source code is licensed under the MIT license found in the
* LICENSE file in the root directory of this source tree.
*/

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

const cleanupRunnerStack = (stderr: string) =>
wrap(
stderr
.split('\n')
.filter(
line =>
!line.includes('packages/jest-jasmine2/build') &&
!line.includes('packages/jest-circus/build'),
)
.join('\n'),
);

test('print correct error message with nested test definitions outside describe', () => {
const result = runJest('nested-test-definitions', ['outside']);

expect(result.exitCode).toBe(1);

const summary = extractSummary(result.stderr);

expect(cleanupRunnerStack(summary.rest)).toMatchSnapshot();
});

test('print correct error message with nested test definitions inside describe', () => {
const result = runJest('nested-test-definitions', ['within']);

expect(result.exitCode).toBe(1);

const summary = extractSummary(result.stderr);

expect(cleanupRunnerStack(summary.rest)).toMatchSnapshot();
});

test('print correct message when nesting describe inside it', () => {
if (!isJestCircusRun()) {
return;
}

const result = runJest('nested-test-definitions', ['nestedDescribeInTest']);

expect(result.exitCode).toBe(1);

expect(result.stderr).toContain(
'Cannot nest a describe inside a test. Describe block "inner describe" cannot run because it is nested within "test".',
);
});
18 changes: 18 additions & 0 deletions e2e/nested-test-definitions/__tests__/nestedDescribeInTest.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
/**
* Copyright (c) Facebook, Inc. and its affiliates. All Rights Reserved.
*
* This source code is licensed under the MIT license found in the
* LICENSE file in the root directory of this source tree.
*/

'use strict';

const {getTruthy} = require('../index');

test('test', () => {
expect(getTruthy()).toBeTruthy();

describe('inner describe', () => {
// nothing to see here
});
});
18 changes: 18 additions & 0 deletions e2e/nested-test-definitions/__tests__/nestedTestOutsideDescribe.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
/**
* Copyright (c) Facebook, Inc. and its affiliates. All Rights Reserved.
*
* This source code is licensed under the MIT license found in the
* LICENSE file in the root directory of this source tree.
*/

'use strict';

const {getTruthy} = require('../index');

test('outer test', () => {
expect(getTruthy()).toBeTruthy();

test('inner test', () => {
expect(getTruthy()).toEqual('This test should not have run');
});
});
20 changes: 20 additions & 0 deletions e2e/nested-test-definitions/__tests__/nestedTestWithinDescribe.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
/**
* Copyright (c) Facebook, Inc. and its affiliates. All Rights Reserved.
*
* This source code is licensed under the MIT license found in the
* LICENSE file in the root directory of this source tree.
*/

'use strict';

const {getTruthy} = require('../index');

describe('in describe', () => {
test('outer test', () => {
expect(getTruthy()).toBeTruthy();

test('inner test', () => {
expect(getTruthy()).toBeTruthy();
});
});
});
10 changes: 10 additions & 0 deletions e2e/nested-test-definitions/index.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
/**
* Copyright (c) Facebook, Inc. and its affiliates. All Rights Reserved.
*
* This source code is licensed under the MIT license found in the
* LICENSE file in the root directory of this source tree.
*/

'use strict';

module.exports.getTruthy = () => true;
5 changes: 5 additions & 0 deletions e2e/nested-test-definitions/package.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
{
"jest": {
"testEnvironment": "jsdom"
}
}
18 changes: 16 additions & 2 deletions packages/jest-circus/src/eventHandler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,14 @@ const eventHandler: Circus.EventHandler = (
}
case 'start_describe_definition': {
const {blockName, mode} = event;
const {currentDescribeBlock} = state;
const {currentDescribeBlock, currentlyRunningTest} = state;

if (currentlyRunningTest) {
throw new Error(
`Cannot nest a describe inside a test. Describe block "${blockName}" cannot run because it is nested within "${currentlyRunningTest.name}".`,
);
}

const describeBlock = makeDescribe(blockName, currentDescribeBlock, mode);
currentDescribeBlock.children.push(describeBlock);
state.currentDescribeBlock = describeBlock;
Expand Down Expand Up @@ -88,8 +95,15 @@ const eventHandler: Circus.EventHandler = (
break;
}
case 'add_test': {
const {currentDescribeBlock} = state;
const {currentDescribeBlock, currentlyRunningTest} = state;
const {asyncError, fn, mode, testName: name, timeout} = event;

if (currentlyRunningTest) {
throw new Error(
`Tests cannot be nested. Test "${name}" cannot run because it is nested within "${currentlyRunningTest.name}".`,
);
}

const test = makeTest(
fn,
mode,
Expand Down
6 changes: 1 addition & 5 deletions packages/jest-jasmine2/src/jasmine/Env.ts
Original file line number Diff line number Diff line change
Expand Up @@ -566,11 +566,7 @@ export default function (j$: Jasmine) {
// This check throws an error to warn the user about the edge-case.
if (currentSpec !== null) {
throw new Error(
'Tests cannot be nested. Test `' +
spec.description +
'` cannot run because it is nested within `' +
currentSpec.description +
'`.',
`Tests cannot be nested. Test "${spec.description}" cannot run because it is nested within "${currentSpec.description}".`,
);
}
currentDeclarationSuite.addChild(spec);
Expand Down

0 comments on commit 3681cca

Please sign in to comment.