From d9ffeb28f61ea24911e371392c8f4ee17a0bdc0f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bazyli=20Brz=C3=B3ska?= Date: Wed, 28 Mar 2018 10:21:40 +0200 Subject: [PATCH 1/3] Refactor: unify handling of topSuite and children suites + increase readability (#5885) --- CHANGELOG.md | 3 + packages/jest-jasmine2/src/jasmine/Env.js | 109 +++++++++++-------- packages/jest-jasmine2/src/tree_processor.js | 54 ++++----- 3 files changed, 95 insertions(+), 71 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 9c7acc04eb1e..c83cd12903ca 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -153,6 +153,9 @@ ### Chore & Maintenance +* `[jest-jasmine2]` Simplify `Env.execute` and TreeProcessor to setup and clean + resources for the top suite the same way as for all of the children suites + ([#5885](https://github.com/facebook/jest/pull/5885)) * `[babel-jest]` [**BREAKING**] Always return object from transformer ([#5991](https://github.com/facebook/jest/pull/5991)) * `[*]` Run Prettier on compiled output diff --git a/packages/jest-jasmine2/src/jasmine/Env.js b/packages/jest-jasmine2/src/jasmine/Env.js index fae558bc6d39..f73c1745065b 100644 --- a/packages/jest-jasmine2/src/jasmine/Env.js +++ b/packages/jest-jasmine2/src/jasmine/Env.js @@ -177,49 +177,65 @@ export default function(j$) { return j$.testPath; }, }); - defaultResourcesForRunnable(topSuite.id); + currentDeclarationSuite = topSuite; this.topSuite = function() { return topSuite; }; - this.execute = async function(runnablesToRun) { - if (!runnablesToRun) { - if (focusedRunnables.length) { - runnablesToRun = focusedRunnables; - } else { - runnablesToRun = [topSuite.id]; - } + const uncaught = err => { + if (currentSpec) { + currentSpec.onException(err); + currentSpec.cancel(); + } else { + console.error('Unhandled error'); + console.error(err.stack); } + }; - const uncaught = err => { - if (currentSpec) { - currentSpec.onException(err); - currentSpec.cancel(); - } else { - console.error('Unhandled error'); - console.error(err.stack); - } - }; - + let oldListenersException; + let oldListenersRejection; + const executionSetup = function() { // Need to ensure we are the only ones handling these exceptions. - const oldListenersException = process - .listeners('uncaughtException') - .slice(); - const oldListenersRejection = process - .listeners('unhandledRejection') - .slice(); + oldListenersException = process.listeners('uncaughtException').slice(); + oldListenersRejection = process.listeners('unhandledRejection').slice(); j$.process.removeAllListeners('uncaughtException'); j$.process.removeAllListeners('unhandledRejection'); j$.process.on('uncaughtException', uncaught); j$.process.on('unhandledRejection', uncaught); + }; + + const executionTeardown = function() { + j$.process.removeListener('uncaughtException', uncaught); + j$.process.removeListener('unhandledRejection', uncaught); + + // restore previous exception handlers + oldListenersException.forEach(listener => { + j$.process.on('uncaughtException', listener); + }); + + oldListenersRejection.forEach(listener => { + j$.process.on('unhandledRejection', listener); + }); + }; + + this.execute = async function(runnablesToRun, suiteTree = topSuite) { + if (!runnablesToRun) { + if (focusedRunnables.length) { + runnablesToRun = focusedRunnables; + } else { + runnablesToRun = [suiteTree.id]; + } + } - reporter.jasmineStarted({totalSpecsDefined}); + if (currentlyExecutingSuites.length === 0) { + executionSetup(); + } - currentlyExecutingSuites.push(topSuite); + const lastDeclarationSuite = currentDeclarationSuite; await treeProcessor({ nodeComplete(suite) { @@ -227,34 +243,37 @@ export default function(j$) { clearResourcesForRunnable(suite.id); } currentlyExecutingSuites.pop(); - reporter.suiteDone(suite.getResult()); + if (suite === topSuite) { + reporter.jasmineDone({ + failedExpectations: topSuite.result.failedExpectations, + }); + } else { + reporter.suiteDone(suite.getResult()); + } }, nodeStart(suite) { + currentDeclarationSuite = suite; currentlyExecutingSuites.push(suite); - defaultResourcesForRunnable(suite.id, suite.parentSuite.id); - reporter.suiteStarted(suite.result); + defaultResourcesForRunnable( + suite.id, + suite.parentSuite && suite.parentSuite.id, + ); + if (suite === topSuite) { + reporter.jasmineStarted({totalSpecsDefined}); + } else { + reporter.suiteStarted(suite.result); + } }, queueRunnerFactory, runnableIds: runnablesToRun, - tree: topSuite, + tree: suiteTree, }); - clearResourcesForRunnable(topSuite.id); - currentlyExecutingSuites.pop(); - reporter.jasmineDone({ - failedExpectations: topSuite.result.failedExpectations, - }); - - j$.process.removeListener('uncaughtException', uncaught); - j$.process.removeListener('unhandledRejection', uncaught); - // restore previous exception handlers - oldListenersException.forEach(listener => { - j$.process.on('uncaughtException', listener); - }); + currentDeclarationSuite = lastDeclarationSuite; - oldListenersRejection.forEach(listener => { - j$.process.on('unhandledRejection', listener); - }); + if (currentlyExecutingSuites.length === 0) { + executionTeardown(); + } }; this.addReporter = function(reporterToAdd) { diff --git a/packages/jest-jasmine2/src/tree_processor.js b/packages/jest-jasmine2/src/tree_processor.js index 8f5d88835ca9..0ebcc01a59da 100644 --- a/packages/jest-jasmine2/src/tree_processor.js +++ b/packages/jest-jasmine2/src/tree_processor.js @@ -43,32 +43,29 @@ export default function treeProcessor(options: Options) { return parentEnabled || runnableIds.indexOf(node.id) !== -1; } - return queueRunnerFactory({ - onException: error => tree.onException(error), - queueableFns: wrapChildren(tree, isEnabled(tree, false)), - userContext: tree.sharedUserContext(), - }); - - function executeNode(node, parentEnabled) { + function getNodeHandler(node: TreeNode, parentEnabled: boolean) { const enabled = isEnabled(node, parentEnabled); - if (!node.children) { - return { - fn(done) { - node.execute(done, enabled); - }, - }; - } - return { - async fn(done) { - nodeStart(node); - await queueRunnerFactory({ - onException: error => node.onException(error), - queueableFns: wrapChildren(node, enabled), - userContext: node.sharedUserContext(), - }); - nodeComplete(node); - done(); - }, + return node.children + ? getNodeWithChildrenHandler(node, enabled) + : getNodeWithoutChildrenHandler(node, enabled); + } + + function getNodeWithoutChildrenHandler(node: TreeNode, enabled: boolean) { + return function fn(done: (error?: any) => void = () => {}) { + node.execute(done, enabled); + }; + } + + function getNodeWithChildrenHandler(node: TreeNode, enabled: boolean) { + return async function fn(done: (error?: any) => void = () => {}) { + nodeStart(node); + await queueRunnerFactory({ + onException: error => node.onException(error), + queueableFns: wrapChildren(node, enabled), + userContext: node.sharedUserContext(), + }); + nodeComplete(node); + done(); }; } @@ -76,7 +73,12 @@ export default function treeProcessor(options: Options) { if (!node.children) { throw new Error('`node.children` is not defined.'); } - const children = node.children.map(child => executeNode(child, enabled)); + const children = node.children.map(child => ({ + fn: getNodeHandler(child, enabled), + })); return node.beforeAllFns.concat(children).concat(node.afterAllFns); } + + const treeHandler = getNodeHandler(tree, false); + return treeHandler(); } From 182b4a6e16a0dec47446b899e5de706cbf2210eb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bazyli=20Brz=C3=B3ska?= Date: Tue, 17 Apr 2018 11:26:09 +0200 Subject: [PATCH 2/3] fix(treeProcessor): revert a small change from the #5585 refactoring (#6006) fixes #5964 moves the `wrapChildren` into higher scope to restore undocumented execution order --- packages/jest-jasmine2/src/tree_processor.js | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/packages/jest-jasmine2/src/tree_processor.js b/packages/jest-jasmine2/src/tree_processor.js index 0ebcc01a59da..9bea3c4db383 100644 --- a/packages/jest-jasmine2/src/tree_processor.js +++ b/packages/jest-jasmine2/src/tree_processor.js @@ -57,11 +57,16 @@ export default function treeProcessor(options: Options) { } function getNodeWithChildrenHandler(node: TreeNode, enabled: boolean) { + // NOTE: We create the array of queueableFns preemptively, + // in order to keep a legacy, undocumented ordering of beforeEach execution. + // Specifically, this applies to beforeEach that were added inside of tests. + // Facebook depends on this behavior internally (see #5964 for discussion) + const queueableFns = wrapChildren(node, enabled); return async function fn(done: (error?: any) => void = () => {}) { nodeStart(node); await queueRunnerFactory({ onException: error => node.onException(error), - queueableFns: wrapChildren(node, enabled), + queueableFns, userContext: node.sharedUserContext(), }); nodeComplete(node); From b10311cae136073236714c1b84f917c0e144a286 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miguel=20Jim=C3=A9nez=20Es=C3=BAn?= Date: Sun, 13 May 2018 11:12:49 +0100 Subject: [PATCH 3/3] Fix for keeping beforeEach inside it --- .../__tests__/__snapshots__/failures.test.js.snap | 2 +- packages/jest-jasmine2/src/jasmine/Env.js | 1 - packages/jest-jasmine2/src/tree_processor.js | 7 +------ 3 files changed, 2 insertions(+), 8 deletions(-) diff --git a/integration-tests/__tests__/__snapshots__/failures.test.js.snap b/integration-tests/__tests__/__snapshots__/failures.test.js.snap index 9b5b4fd184e7..1e5057341a2d 100644 --- a/integration-tests/__tests__/__snapshots__/failures.test.js.snap +++ b/integration-tests/__tests__/__snapshots__/failures.test.js.snap @@ -247,7 +247,7 @@ exports[`not throwing Error objects 5`] = ` 37 | }); 38 | - at packages/jest-jasmine2/build/jasmine/Env.js:518:34 + at packages/jest-jasmine2/build/jasmine/Env.js:541:34 at __tests__/during_tests.test.js:36:3 " diff --git a/packages/jest-jasmine2/src/jasmine/Env.js b/packages/jest-jasmine2/src/jasmine/Env.js index f73c1745065b..96a819650423 100644 --- a/packages/jest-jasmine2/src/jasmine/Env.js +++ b/packages/jest-jasmine2/src/jasmine/Env.js @@ -252,7 +252,6 @@ export default function(j$) { } }, nodeStart(suite) { - currentDeclarationSuite = suite; currentlyExecutingSuites.push(suite); defaultResourcesForRunnable( suite.id, diff --git a/packages/jest-jasmine2/src/tree_processor.js b/packages/jest-jasmine2/src/tree_processor.js index 9bea3c4db383..0ebcc01a59da 100644 --- a/packages/jest-jasmine2/src/tree_processor.js +++ b/packages/jest-jasmine2/src/tree_processor.js @@ -57,16 +57,11 @@ export default function treeProcessor(options: Options) { } function getNodeWithChildrenHandler(node: TreeNode, enabled: boolean) { - // NOTE: We create the array of queueableFns preemptively, - // in order to keep a legacy, undocumented ordering of beforeEach execution. - // Specifically, this applies to beforeEach that were added inside of tests. - // Facebook depends on this behavior internally (see #5964 for discussion) - const queueableFns = wrapChildren(node, enabled); return async function fn(done: (error?: any) => void = () => {}) { nodeStart(node); await queueRunnerFactory({ onException: error => node.onException(error), - queueableFns, + queueableFns: wrapChildren(node, enabled), userContext: node.sharedUserContext(), }); nodeComplete(node);