Revert changes on the TreeProcessor (#6006 and #5885) (#6105)

jestjs · May 1, 2018 · 6640903 · 6640903
1 parent 4c75933
commit 6640903
Show file tree

Hide file tree

Showing 4 changed files with 72 additions and 101 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -144,9 +144,6 @@
 
 * `[babel-jest]` [**BREAKING**] Always return object from transformer
   ([#5991](https://github.com/facebook/jest/pull/5991))
-* `[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))
 * `[*]` Run Prettier on compiled output
   ([#5858](https://github.com/facebook/jest/pull/3497))
 * `[jest-cli]` Add fileChange hook for plugins

diff --git 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:542:34
+      at packages/jest-jasmine2/build/jasmine/Env.js:518: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
@@ -177,103 +177,84 @@ export default function(j$) {
         return j$.testPath;
       },
     });
-
+    defaultResourcesForRunnable(topSuite.id);
     currentDeclarationSuite = topSuite;
 
     this.topSuite = function() {
       return topSuite;
     };
 
-    const uncaught = err => {
-      if (currentSpec) {
-        currentSpec.onException(err);
-        currentSpec.cancel();
-      } else {
-        console.error('Unhandled error');
-        console.error(err.stack);
+    this.execute = async function(runnablesToRun) {
+      if (!runnablesToRun) {
+        if (focusedRunnables.length) {
+          runnablesToRun = focusedRunnables;
+        } else {
+          runnablesToRun = [topSuite.id];
+        }
       }
-    };
 
-    let oldListenersException;
-    let oldListenersRejection;
-    const executionSetup = function() {
+      const uncaught = err => {
+        if (currentSpec) {
+          currentSpec.onException(err);
+          currentSpec.cancel();
+        } else {
+          console.error('Unhandled error');
+          console.error(err.stack);
+        }
+      };
+
       // Need to ensure we are the only ones handling these exceptions.
-      oldListenersException = process.listeners('uncaughtException').slice();
-      oldListenersRejection = process.listeners('unhandledRejection').slice();
+      const oldListenersException = process
+        .listeners('uncaughtException')
+        .slice();
+      const 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];
-        }
-      }
 
-      if (currentlyExecutingSuites.length === 0) {
-        executionSetup();
-      }
+      reporter.jasmineStarted({totalSpecsDefined});
 
-      const lastDeclarationSuite = currentDeclarationSuite;
+      currentlyExecutingSuites.push(topSuite);
 
       await treeProcessor({
         nodeComplete(suite) {
           if (!suite.disabled) {
             clearResourcesForRunnable(suite.id);
           }
           currentlyExecutingSuites.pop();
-          if (suite === topSuite) {
-            reporter.jasmineDone({
-              failedExpectations: topSuite.result.failedExpectations,
-            });
-          } else {
-            reporter.suiteDone(suite.getResult());
-          }
+          reporter.suiteDone(suite.getResult());
         },
         nodeStart(suite) {
-          currentDeclarationSuite = suite;
           currentlyExecutingSuites.push(suite);
-          defaultResourcesForRunnable(
-            suite.id,
-            suite.parentSuite && suite.parentSuite.id,
-          );
-          if (suite === topSuite) {
-            reporter.jasmineStarted({totalSpecsDefined});
-          } else {
-            reporter.suiteStarted(suite.result);
-          }
+          defaultResourcesForRunnable(suite.id, suite.parentSuite.id);
+          reporter.suiteStarted(suite.result);
         },
         queueRunnerFactory,
         runnableIds: runnablesToRun,
-        tree: suiteTree,
+        tree: topSuite,
+      });
+      clearResourcesForRunnable(topSuite.id);
+      currentlyExecutingSuites.pop();
+      reporter.jasmineDone({
+        failedExpectations: topSuite.result.failedExpectations,
       });
 
-      currentDeclarationSuite = lastDeclarationSuite;
+      j$.process.removeListener('uncaughtException', uncaught);
+      j$.process.removeListener('unhandledRejection', uncaught);
 
-      if (currentlyExecutingSuites.length === 0) {
-        executionTeardown();
-      }
+      // restore previous exception handlers
+      oldListenersException.forEach(listener => {
+        j$.process.on('uncaughtException', listener);
+      });
+
+      oldListenersRejection.forEach(listener => {
+        j$.process.on('unhandledRejection', listener);
+      });
     };
 
     this.addReporter = function(reporterToAdd) {

diff --git a/packages/jest-jasmine2/src/tree_processor.js b/packages/jest-jasmine2/src/tree_processor.js
@@ -43,47 +43,40 @@ export default function treeProcessor(options: Options) {
     return parentEnabled || runnableIds.indexOf(node.id) !== -1;
   }
 
-  function getNodeHandler(node: TreeNode, parentEnabled: boolean) {
-    const enabled = isEnabled(node, parentEnabled);
-    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);
-    };
-  }
+  return queueRunnerFactory({
+    onException: error => tree.onException(error),
+    queueableFns: wrapChildren(tree, isEnabled(tree, false)),
+    userContext: tree.sharedUserContext(),
+  });
 
-  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,
-        userContext: node.sharedUserContext(),
-      });
-      nodeComplete(node);
-      done();
+  function executeNode(node, parentEnabled) {
+    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();
+      },
     };
   }
 
   function wrapChildren(node: TreeNode, enabled: boolean) {
     if (!node.children) {
       throw new Error('`node.children` is not defined.');
     }
-    const children = node.children.map(child => ({
-      fn: getNodeHandler(child, enabled),
-    }));
+    const children = node.children.map(child => executeNode(child, enabled));
     return node.beforeAllFns.concat(children).concat(node.afterAllFns);
   }
-
-  const treeHandler = getNodeHandler(tree, false);
-  return treeHandler();
 }