feat(jest-circus,jest-jasmine2): throw when describe returns a value (#…

…10947)
jestjs · Dec 14, 2020 · f1ae805 · f1ae805
1 parent 79f4fe3
commit f1ae805
Show file tree

Hide file tree

Showing 7 changed files with 51 additions and 102 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -2,6 +2,7 @@
 
 ### Features
 
+- `[jest-circus, jest-jasmine2]` [**BREAKING**] Fail the test instead of just warning when describe returns a value ([#10947](https://github.com/facebook/jest/pull/10947))
 - `[jest-config]` [**BREAKING**] Default to Node testing environment instead of browser (JSDOM) ([#9874](https://github.com/facebook/jest/pull/9874))
 - `[jest-config]` [**BREAKING**] Use `jest-circus` as default test runner ([#10686](https://github.com/facebook/jest/pull/10686))
 - `[jest-config, jest-runtime]` Support ESM for files other than `.js` and `.mjs` ([#10823](https://github.com/facebook/jest/pull/10823))

diff --git a/e2e/__tests__/__snapshots__/declarationErrors.test.ts.snap b/e2e/__tests__/__snapshots__/declarationErrors.test.ts.snap
@@ -1,41 +1,29 @@
 // Jest Snapshot v1, https://goo.gl/fbAQLP
 
-exports[`warns if describe returns a Promise 1`] = `
-  console.log
-      ● Test suite failed to run
-    
-        Returning a Promise from "describe" is not supported. Tests must be defined synchronously.
-        Returning a value from "describe" will fail the test in a future version of Jest.
-    
-           9 | 'use strict';
-          10 | 
-        > 11 | describe('Promise describe warns', () => {
-             | ^
-          12 |   it('t', () => {});
-          13 |   return Promise.resolve();
-          14 | });
-    
-          at Object.describe (__tests__/describeReturnPromise.test.js:11:1)
+exports[`errors if describe returns a Promise 1`] = `
+    Returning a Promise from "describe" is not supported. Tests must be defined synchronously.
 
+       9 | 'use strict';
+      10 | 
+    > 11 | describe('Promise describe errors', () => {
+         | ^
+      12 |   it('t', () => {});
+      13 |   return Promise.resolve();
+      14 | });
 
+      at Object.describe (__tests__/describeReturnPromise.test.js:11:1)
 `;
 
-exports[`warns if describe returns something 1`] = `
-  console.log
-      ● Test suite failed to run
-    
-        A "describe" callback must not return a value.
-        Returning a value from "describe" will fail the test in a future version of Jest.
-    
-           9 | 'use strict';
-          10 | 
-        > 11 | describe('describe return warns', () => {
-             | ^
-          12 |   it('t', () => {});
-          13 |   return 42;
-          14 | });
-    
-          at Object.describe (__tests__/describeReturnSomething.test.js:11:1)
+exports[`errors if describe returns something 1`] = `
+    A "describe" callback must not return a value.
 
+       9 | 'use strict';
+      10 | 
+    > 11 | describe('describe return errors', () => {
+         | ^
+      12 |   it('t', () => {});
+      13 |   return 42;
+      14 | });
 
+      at Object.describe (__tests__/describeReturnSomething.test.js:11:1)
 `;
diff --git a/e2e/__tests__/declarationErrors.test.ts b/e2e/__tests__/declarationErrors.test.ts
@@ -6,38 +6,44 @@
  */
 
 import wrap from 'jest-snapshot-serializer-raw';
+import {extractSummary} from '../Utils';
 import runJest from '../runJest';
 
-const normalizeCircusJasmine = (str: string) =>
+const extractMessage = (str: string) =>
   wrap(
-    str
-      .replace(/console\.log .+:\d+/, 'console.log')
-      .replace(/.+addSpecsToSuite (.+:\d+:\d+).+\n/g, '')
-      .replace(/.+_dispatchDescribe (.+:\d+:\d+).+\n/g, ''),
+    extractSummary(str)
+      .rest.replace(
+        // circus-jasmine normalization
+        /.+addSpecsToSuite (.+:\d+:\d+).+\n/g,
+        '',
+      )
+      .match(
+        // all lines from the first to the last mentioned "describe" after the "●" line
+        /●(.|\n)*?\n(?<lines>.*describe((.|\n)*describe)*.*)(\n|$)/im,
+      )?.groups?.lines ?? '',
   );
 
-it('warns if describe returns a Promise', () => {
+it('errors if describe returns a Promise', () => {
   const result = runJest('declaration-errors', [
     'describeReturnPromise.test.js',
   ]);
 
-  expect(result.exitCode).toBe(0);
-  expect(normalizeCircusJasmine(result.stdout)).toMatchSnapshot();
+  expect(result.exitCode).toBe(1);
+  expect(extractMessage(result.stderr)).toMatchSnapshot();
 });
 
-it('warns if describe returns something', () => {
+it('errors if describe returns something', () => {
   const result = runJest('declaration-errors', [
     'describeReturnSomething.test.js',
   ]);
 
-  expect(result.exitCode).toBe(0);
-  expect(normalizeCircusJasmine(result.stdout)).toMatchSnapshot();
+  expect(result.exitCode).toBe(1);
+  expect(extractMessage(result.stderr)).toMatchSnapshot();
 });
 
 it('errors if describe throws', () => {
   const result = runJest('declaration-errors', ['describeThrow.test.js']);
 
   expect(result.exitCode).toBe(1);
-  expect(result.stdout).toBe('');
   expect(result.stderr).toContain('whoops');
 });
diff --git a/e2e/declaration-errors/__tests__/describeReturnPromise.test.js b/e2e/declaration-errors/__tests__/describeReturnPromise.test.js
@@ -8,7 +8,7 @@
 
 'use strict';
 
-describe('Promise describe warns', () => {
+describe('Promise describe errors', () => {
   it('t', () => {});
   return Promise.resolve();
 });
diff --git a/e2e/declaration-errors/__tests__/describeReturnSomething.test.js b/e2e/declaration-errors/__tests__/describeReturnSomething.test.js
@@ -8,7 +8,7 @@
 
 'use strict';
 
-describe('describe return warns', () => {
+describe('describe return errors', () => {
   it('t', () => {});
   return 42;
 });
diff --git a/packages/jest-circus/src/index.ts b/packages/jest-circus/src/index.ts
@@ -5,10 +5,8 @@
  * LICENSE file in the root directory of this source tree.
  */
 
-import chalk = require('chalk');
 import type {Circus, Global} from '@jest/types';
 import {bind as bindEach} from 'jest-each';
-import {formatExecError} from 'jest-message-util';
 import {ErrorWithStack, isPromise} from 'jest-util';
 import {dispatchSync} from './state';
 
@@ -60,36 +58,15 @@ const _dispatchDescribe = (
   });
   const describeReturn = blockFn();
 
-  // TODO throw in Jest 25
   if (isPromise(describeReturn)) {
-    // eslint-disable-next-line no-console
-    console.log(
-      formatExecError(
-        new ErrorWithStack(
-          chalk.yellow(
-            'Returning a Promise from "describe" is not supported. Tests must be defined synchronously.\n' +
-              'Returning a value from "describe" will fail the test in a future version of Jest.',
-          ),
-          describeFn,
-        ),
-        {rootDir: '', testMatch: []},
-        {noStackTrace: false},
-      ),
+    throw new ErrorWithStack(
+      'Returning a Promise from "describe" is not supported. Tests must be defined synchronously.',
+      describeFn,
     );
   } else if (describeReturn !== undefined) {
-    // eslint-disable-next-line no-console
-    console.log(
-      formatExecError(
-        new ErrorWithStack(
-          chalk.yellow(
-            'A "describe" callback must not return a value.\n' +
-              'Returning a value from "describe" will fail the test in a future version of Jest.',
-          ),
-          describeFn,
-        ),
-        {rootDir: '', testMatch: []},
-        {noStackTrace: false},
-      ),
+    throw new ErrorWithStack(
+      'A "describe" callback must not return a value.',
+      describeFn,
     );
   }
 

diff --git a/packages/jest-jasmine2/src/jasmine/Env.ts b/packages/jest-jasmine2/src/jasmine/Env.ts
@@ -31,8 +31,6 @@ WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
 /* eslint-disable sort-keys, local/prefer-spread-eventually, local/prefer-rest-params-eventually */
 
 import {AssertionError} from 'assert';
-import chalk = require('chalk');
-import {formatExecError} from 'jest-message-util';
 import {ErrorWithStack, isPromise} from 'jest-util';
 import assertionErrorMessage from '../assertionErrorMessage';
 import isError from '../isError';
@@ -444,34 +442,13 @@ export default function (j$: Jasmine) {
           declarationError = e;
         }
 
-        // TODO throw in Jest 25: declarationError = new Error
         if (isPromise(describeReturnValue)) {
-          // eslint-disable-next-line no-console
-          console.log(
-            formatExecError(
-              new Error(
-                chalk.yellow(
-                  'Returning a Promise from "describe" is not supported. Tests must be defined synchronously.\n' +
-                    'Returning a value from "describe" will fail the test in a future version of Jest.',
-                ),
-              ),
-              {rootDir: '', testMatch: []},
-              {noStackTrace: false},
-            ),
+          declarationError = new Error(
+            'Returning a Promise from "describe" is not supported. Tests must be defined synchronously.',
           );
         } else if (describeReturnValue !== undefined) {
-          // eslint-disable-next-line no-console
-          console.log(
-            formatExecError(
-              new Error(
-                chalk.yellow(
-                  'A "describe" callback must not return a value.\n' +
-                    'Returning a value from "describe" will fail the test in a future version of Jest.',
-                ),
-              ),
-              {rootDir: '', testMatch: []},
-              {noStackTrace: false},
-            ),
+          declarationError = new Error(
+            'A "describe" callback must not return a value.',
           );
         }