Refactor: unify handling of topSuite and children suites + increase r…

…eadability (#5885) * refactor(jest-jasmine2): Simplify `Env.execute` to setup and clean resources for the top suite the same way as for all of the children suites snapshot * refactor(jest-jasmine2): Clear the meaning of treeProcessor functions and match new Env.execute behavior refactor(jest-jasmine2): Clear the meaning of treeProcessor functions and match new Env.execute behavior fix flow a * doc(CHANGELOG): add entries for Env.execute and TreeProcessor changes * Update CHANGELOG.md
jestjs · Mar 28, 2018 · 815c8bd · 815c8bd
1 parent 5bd6ee1
commit 815c8bd
Show file tree

Hide file tree

Showing 4 changed files with 99 additions and 75 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -70,9 +70,12 @@
 
 ### Chore & Maintenance
 
-* `[#5858]` Run Prettier on compiled output
+* `[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))
-* `[#5708]` Add fileChange hook for plugins
+* `[jest-cli]` Add fileChange hook for plugins
   ([#5708](https://github.com/facebook/jest/pull/5708))
 * `[docs]` Add docs on using `jest.mock(...)`
   ([#5648](https://github.com/facebook/jest/pull/5648))

diff --git a/integration-tests/__tests__/__snapshots__/globals.test.js.snap b/integration-tests/__tests__/__snapshots__/globals.test.js.snap
@@ -32,7 +32,7 @@ exports[`cannot test with no implementation 1`] = `
       4 |     test('test, no implementation');
       5 |   
       
-      at packages/jest-jasmine2/build/jasmine/Env.js:405:15
+      at packages/jest-jasmine2/build/jasmine/Env.js:429:15
       at __tests__/only-constructs.test.js:3:5
 
 "
@@ -59,7 +59,7 @@ exports[`cannot test with no implementation with expand arg 1`] = `
       4 |     test('test, no implementation');
       5 |   
       
-      at packages/jest-jasmine2/build/jasmine/Env.js:405:15
+      at packages/jest-jasmine2/build/jasmine/Env.js:429:15
       at __tests__/only-constructs.test.js:3:5
 
 "

diff --git a/packages/jest-jasmine2/src/jasmine/Env.js b/packages/jest-jasmine2/src/jasmine/Env.js
@@ -176,84 +176,103 @@ 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) {
           if (!suite.disabled) {
             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
@@ -43,40 +43,42 @@ 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();
     };
   }
 
   function wrapChildren(node: TreeNode, enabled: boolean) {
     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();
 }