From 77f975db5fa7c16e3bd04e37a1ad1908dd6b936c Mon Sep 17 00:00:00 2001 From: jeiea Date: Thu, 30 Apr 2020 15:03:10 +0900 Subject: [PATCH 1/2] fix: don't run beforeAll/afterAll in skipped block --- CHANGELOG.md | 2 + e2e/__tests__/skipBeforeAfterAll.test.ts | 58 +++++++++++++++++++++ packages/jest-jasmine2/src/treeProcessor.ts | 10 ++-- 3 files changed, 65 insertions(+), 5 deletions(-) create mode 100644 e2e/__tests__/skipBeforeAfterAll.test.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index 8052d1334837..b6ce7e310f0f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,8 @@ ### Fixes +- `[jest-jasmine2]` Don't run `beforeAll` / `afterAll` in skipped describe block ([#9931](https://github.com/facebook/jest/pull/9931)) + ### Chore & Maintenance ### Performance diff --git a/e2e/__tests__/skipBeforeAfterAll.test.ts b/e2e/__tests__/skipBeforeAfterAll.test.ts new file mode 100644 index 000000000000..4e1d0f74efa2 --- /dev/null +++ b/e2e/__tests__/skipBeforeAfterAll.test.ts @@ -0,0 +1,58 @@ +/** + * 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. + */ + +let hasBeforeAllRun = false; +let hasAfterAllRun = false; + +describe.skip('in describe.skip', () => { + describe('in describe', () => { + beforeAll(() => { + hasBeforeAllRun = true; + }); + + afterAll(() => { + hasAfterAllRun = true; + }); + + test('it should be skipped', () => { + throw new Error('This should never happen'); + }); + }); +}); + +test('describe.skip should not run beforeAll', () => { + expect(hasBeforeAllRun).toBe(false); +}); + +test('describe.skip should not run afterAll', () => { + expect(hasAfterAllRun).toBe(false); +}); + +let hasBeforeAllRun2 = false; +let hasAfterAllRun2 = false; + +describe('in describe', () => { + beforeAll(() => { + hasBeforeAllRun2 = true; + }); + + afterAll(() => { + hasAfterAllRun2 = true; + }); + + test.skip('it should be skipped', () => { + throw new Error('This should never happen'); + }); +}); + +test('describe having only skipped test should not run beforeAll', () => { + expect(hasBeforeAllRun2).toBe(false); +}); + +test('describe having only skipped test should not run afterAll', () => { + expect(hasAfterAllRun2).toBe(false); +}); diff --git a/packages/jest-jasmine2/src/treeProcessor.ts b/packages/jest-jasmine2/src/treeProcessor.ts index 5bd4eeebe255..e46b9abfaca8 100644 --- a/packages/jest-jasmine2/src/treeProcessor.ts +++ b/packages/jest-jasmine2/src/treeProcessor.ts @@ -23,7 +23,7 @@ export type TreeNode = { onException: (error: Error) => void; sharedUserContext: () => any; children?: Array; -} & Pick; +} & Pick; export default function treeProcessor(options: Options): void { const { @@ -64,11 +64,11 @@ export default function treeProcessor(options: Options): void { }; } - function hasEnabledTest(node: TreeNode): boolean { + function hasNoEnabledTest(node: TreeNode): boolean { if (node.children) { - return node.children.some(hasEnabledTest); + return node.children.every(hasNoEnabledTest); } - return !node.disabled; + return node.disabled || node.markedPending; } function wrapChildren(node: TreeNode, enabled: boolean) { @@ -78,7 +78,7 @@ export default function treeProcessor(options: Options): void { const children = node.children.map(child => ({ fn: getNodeHandler(child, enabled), })); - if (!hasEnabledTest(node)) { + if (hasNoEnabledTest(node)) { return children; } return node.beforeAllFns.concat(children).concat(node.afterAllFns); From 7694ce90cb00a943ea663437b1ed551c3a70fb51 Mon Sep 17 00:00:00 2001 From: Simen Bekkhus Date: Fri, 1 May 2020 21:02:51 +0200 Subject: [PATCH 2/2] run integration test --- e2e/__tests__/skipBeforeAfterAll.test.ts | 55 +++--------------- .../__tests__/beforeAllFiltered.test.js | 58 +++++++++++++++++++ e2e/before-all-skipped/package.json | 5 ++ 3 files changed, 71 insertions(+), 47 deletions(-) create mode 100644 e2e/before-all-skipped/__tests__/beforeAllFiltered.test.js create mode 100644 e2e/before-all-skipped/package.json diff --git a/e2e/__tests__/skipBeforeAfterAll.test.ts b/e2e/__tests__/skipBeforeAfterAll.test.ts index 4e1d0f74efa2..52e996552e11 100644 --- a/e2e/__tests__/skipBeforeAfterAll.test.ts +++ b/e2e/__tests__/skipBeforeAfterAll.test.ts @@ -5,54 +5,15 @@ * LICENSE file in the root directory of this source tree. */ -let hasBeforeAllRun = false; -let hasAfterAllRun = false; +import * as path from 'path'; +import {json as runWithJson} from '../runJest'; -describe.skip('in describe.skip', () => { - describe('in describe', () => { - beforeAll(() => { - hasBeforeAllRun = true; - }); +const DIR = path.resolve(__dirname, '../before-all-skipped'); - afterAll(() => { - hasAfterAllRun = true; - }); +test('correctly skip `beforeAll`s in skipped tests', () => { + const {json} = runWithJson(DIR); - test('it should be skipped', () => { - throw new Error('This should never happen'); - }); - }); -}); - -test('describe.skip should not run beforeAll', () => { - expect(hasBeforeAllRun).toBe(false); -}); - -test('describe.skip should not run afterAll', () => { - expect(hasAfterAllRun).toBe(false); -}); - -let hasBeforeAllRun2 = false; -let hasAfterAllRun2 = false; - -describe('in describe', () => { - beforeAll(() => { - hasBeforeAllRun2 = true; - }); - - afterAll(() => { - hasAfterAllRun2 = true; - }); - - test.skip('it should be skipped', () => { - throw new Error('This should never happen'); - }); -}); - -test('describe having only skipped test should not run beforeAll', () => { - expect(hasBeforeAllRun2).toBe(false); -}); - -test('describe having only skipped test should not run afterAll', () => { - expect(hasAfterAllRun2).toBe(false); + expect(json.numTotalTests).toBe(6); + expect(json.numPassedTests).toBe(4); + expect(json.numPendingTests).toBe(2); }); diff --git a/e2e/before-all-skipped/__tests__/beforeAllFiltered.test.js b/e2e/before-all-skipped/__tests__/beforeAllFiltered.test.js new file mode 100644 index 000000000000..4e1d0f74efa2 --- /dev/null +++ b/e2e/before-all-skipped/__tests__/beforeAllFiltered.test.js @@ -0,0 +1,58 @@ +/** + * 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. + */ + +let hasBeforeAllRun = false; +let hasAfterAllRun = false; + +describe.skip('in describe.skip', () => { + describe('in describe', () => { + beforeAll(() => { + hasBeforeAllRun = true; + }); + + afterAll(() => { + hasAfterAllRun = true; + }); + + test('it should be skipped', () => { + throw new Error('This should never happen'); + }); + }); +}); + +test('describe.skip should not run beforeAll', () => { + expect(hasBeforeAllRun).toBe(false); +}); + +test('describe.skip should not run afterAll', () => { + expect(hasAfterAllRun).toBe(false); +}); + +let hasBeforeAllRun2 = false; +let hasAfterAllRun2 = false; + +describe('in describe', () => { + beforeAll(() => { + hasBeforeAllRun2 = true; + }); + + afterAll(() => { + hasAfterAllRun2 = true; + }); + + test.skip('it should be skipped', () => { + throw new Error('This should never happen'); + }); +}); + +test('describe having only skipped test should not run beforeAll', () => { + expect(hasBeforeAllRun2).toBe(false); +}); + +test('describe having only skipped test should not run afterAll', () => { + expect(hasAfterAllRun2).toBe(false); +}); diff --git a/e2e/before-all-skipped/package.json b/e2e/before-all-skipped/package.json new file mode 100644 index 000000000000..148788b25446 --- /dev/null +++ b/e2e/before-all-skipped/package.json @@ -0,0 +1,5 @@ +{ + "jest": { + "testEnvironment": "node" + } +}