align circus with jasmine's top-to-bottom execution order (#9965)

jestjs · May 4, 2020 · e8e8146 · e8e8146
1 parent 968a301
commit e8e8146
Show file tree

Hide file tree

Showing 7 changed files with 167 additions and 90 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -12,6 +12,7 @@
 - `[jest-circus, jest-console, jest-jasmine2, jest-reporters, jest-util, pretty-format]` Fix time durating formatting and consolidate time formatting code ([#9765](https://github.com/facebook/jest/pull/9765))
 - `[jest-circus]` [**BREAKING**] Fail tests if a test takes a done callback and have return values ([#9129](https://github.com/facebook/jest/pull/9129))
 - `[jest-circus]` [**BREAKING**] Throw a proper error if a test / hook is defined asynchronously ([#8096](https://github.com/facebook/jest/pull/8096))
+- `[jest-circus]` [**BREAKING**] Align execution order of tests to match `jasmine`'s top to bottom order ([#9965](https://github.com/facebook/jest/pull/9965))
 - `[jest-config, jest-resolve]` [**BREAKING**] Remove support for `browser` field ([#9943](https://github.com/facebook/jest/pull/9943))
 - `[jest-haste-map]` Stop reporting files as changed when they are only accessed ([#7347](https://github.com/facebook/jest/pull/7347))
 - `[jest-resolve]` Show relative path from root dir for `module not found` errors ([#9963](https://github.com/facebook/jest/pull/9963))

diff --git a/e2e/__tests__/__snapshots__/globals.test.ts.snap b/e2e/__tests__/__snapshots__/globals.test.ts.snap
@@ -95,6 +95,22 @@ Time:        <<REPLACED>>
 Ran all test suites.
 `;
 
+exports[`interleaved describe and test children order 1`] = `
+PASS __tests__/interleaved.test.js
+  ✓ above
+  ✓ below
+  describe
+    ✓ inside
+`;
+
+exports[`interleaved describe and test children order 2`] = `
+Test Suites: 1 passed, 1 total
+Tests:       3 passed, 3 total
+Snapshots:   0 total
+Time:        <<REPLACED>>
+Ran all test suites.
+`;
+
 exports[`only 1`] = `
 PASS __tests__/onlyConstructs.test.js
   ✓ test.only

diff --git a/e2e/__tests__/globals.test.ts b/e2e/__tests__/globals.test.ts
@@ -45,11 +45,49 @@ test('basic test constructs', () => {
 
   writeFiles(TEST_DIR, {[filename]: content});
   const {stderr, exitCode} = runJest(DIR);
+
+  const {summary, rest} = extractSummary(stderr);
+  expect(wrap(rest)).toMatchSnapshot();
+  expect(wrap(summary)).toMatchSnapshot();
   expect(exitCode).toBe(0);
+});
+
+test('interleaved describe and test children order', () => {
+  const filename = 'interleaved.test.js';
+  const content = `
+    let lastTest;
+    test('above', () => {
+      try {
+        expect(lastTest).toBe(undefined);
+      } finally {
+        lastTest = 'above';
+      }
+    });
+    describe('describe', () => {
+      test('inside', () => {
+        try {
+          expect(lastTest).toBe('above');
+        } finally {
+          lastTest = 'inside';
+        }
+      });
+    });
+    test('below', () => {
+      try {
+        expect(lastTest).toBe('inside');
+      } finally {
+        lastTest = 'below';
+      }
+    });
+  `;
+
+  writeFiles(TEST_DIR, {[filename]: content});
+  const {stderr, exitCode} = runJest(DIR);
 
   const {summary, rest} = extractSummary(stderr);
   expect(wrap(rest)).toMatchSnapshot();
   expect(wrap(summary)).toMatchSnapshot();
+  expect(exitCode).toBe(0);
 });
 
 test('skips', () => {
@@ -106,11 +144,11 @@ test('only', () => {
 
   writeFiles(TEST_DIR, {[filename]: content});
   const {stderr, exitCode} = runJest(DIR);
-  expect(exitCode).toBe(0);
 
   const {summary, rest} = extractSummary(stderr);
   expect(wrap(rest)).toMatchSnapshot();
   expect(wrap(summary)).toMatchSnapshot();
+  expect(exitCode).toBe(0);
 });
 
 test('cannot have describe with no implementation', () => {
@@ -121,13 +159,13 @@ test('cannot have describe with no implementation', () => {
 
   writeFiles(TEST_DIR, {[filename]: content});
   const {stderr, exitCode} = runJest(DIR);
-  expect(exitCode).toBe(1);
 
   const rest = cleanStderr(stderr);
   const {summary} = extractSummary(stderr);
 
   expect(wrap(rest)).toMatchSnapshot();
   expect(wrap(summary)).toMatchSnapshot();
+  expect(exitCode).toBe(1);
 });
 
 test('cannot test with no implementation', () => {
@@ -140,11 +178,11 @@ test('cannot test with no implementation', () => {
 
   writeFiles(TEST_DIR, {[filename]: content});
   const {stderr, exitCode} = runJest(DIR);
-  expect(exitCode).toBe(1);
 
   const {summary} = extractSummary(stderr);
   expect(wrap(cleanStderr(stderr))).toMatchSnapshot();
   expect(wrap(summary)).toMatchSnapshot();
+  expect(exitCode).toBe(1);
 });
 
 test('skips with expand arg', () => {
@@ -171,11 +209,11 @@ test('skips with expand arg', () => {
 
   writeFiles(TEST_DIR, {[filename]: content});
   const {stderr, exitCode} = runJest(DIR, ['--expand']);
-  expect(exitCode).toBe(0);
 
   const {summary, rest} = extractSummary(stderr);
   expect(wrap(rest)).toMatchSnapshot();
   expect(wrap(summary)).toMatchSnapshot();
+  expect(exitCode).toBe(0);
 });
 
 test('only with expand arg', () => {
@@ -201,11 +239,11 @@ test('only with expand arg', () => {
 
   writeFiles(TEST_DIR, {[filename]: content});
   const {stderr, exitCode} = runJest(DIR, ['--expand']);
-  expect(exitCode).toBe(0);
 
   const {summary, rest} = extractSummary(stderr);
   expect(wrap(rest)).toMatchSnapshot();
   expect(wrap(summary)).toMatchSnapshot();
+  expect(exitCode).toBe(0);
 });
 
 test('cannot test with no implementation with expand arg', () => {
@@ -218,11 +256,11 @@ test('cannot test with no implementation with expand arg', () => {
 
   writeFiles(TEST_DIR, {[filename]: content});
   const {stderr, exitCode} = runJest(DIR, ['--expand']);
-  expect(exitCode).toBe(1);
 
   const {summary} = extractSummary(stderr);
   expect(wrap(cleanStderr(stderr))).toMatchSnapshot();
   expect(wrap(summary)).toMatchSnapshot();
+  expect(exitCode).toBe(1);
 });
 
 test('function as descriptor', () => {
@@ -236,9 +274,9 @@ test('function as descriptor', () => {
 
   writeFiles(TEST_DIR, {[filename]: content});
   const {stderr, exitCode} = runJest(DIR);
-  expect(exitCode).toBe(0);
 
   const {summary, rest} = extractSummary(stderr);
   expect(wrap(rest)).toMatchSnapshot();
   expect(wrap(summary)).toMatchSnapshot();
+  expect(exitCode).toBe(0);
 });
diff --git a/packages/jest-circus/src/eventHandler.ts b/packages/jest-circus/src/eventHandler.ts
@@ -60,24 +60,26 @@ const eventHandler: Circus.EventHandler = (
         });
       }
 
-      // inherit mode from its parent describe but
-      // do not inherit "only" mode when there is already tests with "only" mode
-      const shouldInheritMode = !(
+      // pass mode of currentDescribeBlock to tests
+      // but do not when there is already a single test with "only" mode
+      const shouldPassMode = !(
         currentDescribeBlock.mode === 'only' &&
-        currentDescribeBlock.tests.find(test => test.mode === 'only')
+        currentDescribeBlock.children.some(
+          child => child.type === 'test' && child.mode === 'only',
+        )
       );
-
-      if (shouldInheritMode) {
-        currentDescribeBlock.tests.forEach(test => {
-          if (!test.mode) {
-            test.mode = currentDescribeBlock.mode;
+      if (shouldPassMode) {
+        currentDescribeBlock.children.forEach(child => {
+          if (child.type === 'test' && !child.mode) {
+            child.mode = currentDescribeBlock.mode;
           }
         });
       }
-
       if (
         !state.hasFocusedTests &&
-        currentDescribeBlock.tests.some(test => test.mode === 'only')
+        currentDescribeBlock.children.some(
+          child => child.type === 'test' && child.mode === 'only',
+        )
       ) {
         state.hasFocusedTests = true;
       }
@@ -129,7 +131,7 @@ const eventHandler: Circus.EventHandler = (
       if (test.mode === 'only') {
         state.hasFocusedTests = true;
       }
-      currentDescribeBlock.tests.push(test);
+      currentDescribeBlock.children.push(test);
       break;
     }
     case 'hook_failure': {

diff --git a/packages/jest-circus/src/run.ts b/packages/jest-circus/src/run.ts
@@ -43,16 +43,25 @@ const _runTestsForDescribeBlock = async (
   const retryTimes = parseInt(global[RETRY_TIMES], 10) || 0;
   const deferredRetryTests = [];
 
-  for (const test of describeBlock.tests) {
-    const hasErrorsBeforeTestRun = test.errors.length > 0;
-    await _runTest(test);
-
-    if (
-      hasErrorsBeforeTestRun === false &&
-      retryTimes > 0 &&
-      test.errors.length > 0
-    ) {
-      deferredRetryTests.push(test);
+  for (const child of describeBlock.children) {
+    switch (child.type) {
+      case 'describeBlock': {
+        await _runTestsForDescribeBlock(child);
+        break;
+      }
+      case 'test': {
+        const hasErrorsBeforeTestRun = child.errors.length > 0;
+        await _runTest(child);
+
+        if (
+          hasErrorsBeforeTestRun === false &&
+          retryTimes > 0 &&
+          child.errors.length > 0
+        ) {
+          deferredRetryTests.push(child);
+        }
+        break;
+      }
     }
   }
 
@@ -69,10 +78,6 @@ const _runTestsForDescribeBlock = async (
     }
   }
 
-  for (const child of describeBlock.children) {
-    await _runTestsForDescribeBlock(child);
-  }
-
   for (const hook of afterAll) {
     await _callCircusHook({describeBlock, hook});
   }