Better error message when a describe block is empty (#6372)

CHANGELOG.md

@@ -27,6 +27,8 @@
 - `[jest-jasmine]` Show proper error message from async `assert` errors ([#6821](https://github.com/facebook/jest/pull/6821))
 - `[jest-circus]` Fail synchronous hook timeouts ([#7074](https://github.com/facebook/jest/pull/7074))
 - `[jest-jasmine2]` Fail synchronous test timeouts ([#7074](https://github.com/facebook/jest/pull/7074))
+- `[jest-jasmine2]` Better error message when a describe block is empty ([#6372](https://github.com/facebook/jest/pull/6372))
+- `[jest-circus]` Better error message when a describe block is empty ([#6372](https://github.com/facebook/jest/pull/6372))
 
 ### Chore & Maintenance
 

e2e/Utils.js

@@ -119,14 +119,7 @@ const createEmptyPackage = (
   );
 };
 
-type ExtractSummaryOptions = {|
-  stripLocation: boolean,
-|};
-
-const extractSummary = (
-  stdout: string,
-  {stripLocation = false}: ExtractSummaryOptions = {},
-) => {
+const extractSummary = (stdout: string) => {
   const match = stdout.match(
     /Test Suites:.*\nTests.*\nSnapshots.*\nTime.*(\nRan all test suites)*.*\n*$/gm,
   );
@@ -144,15 +137,11 @@ const extractSummary = (
     .replace(/\d*\.?\d+m?s/g, '<<REPLACED>>')
     .replace(/, estimated <<REPLACED>>/g, '');
 
-  let rest = cleanupStackTrace(
+  const rest = cleanupStackTrace(
     // remove all timestamps
     stdout.replace(match[0], '').replace(/\s*\(\d*\.?\d+m?s\)$/gm, ''),
   );
 
-  if (stripLocation) {
-    rest = rest.replace(/(at .*):\d+:\d+/g, '$1:<<LINE>>:<<COLUMN>>');
-  }
-
   return {rest, summary};
 };
 

e2e/__tests__/__snapshots__/globals.test.js.snap

@@ -19,6 +19,31 @@ Time:        <<REPLACED>>
 Ran all test suites."
 `;
 
+exports[`cannot have describe with no implementation 1`] = `
+"FAIL __tests__/only-constructs.test.js
+  ● Test suite failed to run
+
+    Missing second argument. It must be a callback function.
+
+      1 | 
+    > 2 |     describe('describe, no implementation');
+        |     ^
+      3 |   
+
+      at __tests__/only-constructs.test.js:2:5
+
+"
+`;
+
+exports[`cannot have describe with no implementation 2`] = `
+"Test Suites: 1 failed, 1 total
+Tests:       0 total
+Snapshots:   0 total
+Time:        <<REPLACED>>
+Ran all test suites.
+"
+`;
+
 exports[`cannot test with no implementation 1`] = `
 "FAIL __tests__/only-constructs.test.js
   ● Test suite failed to run

e2e/__tests__/globals.test.js

@@ -20,7 +20,7 @@ const TEST_DIR = path.resolve(DIR, '__tests__');
 
 function cleanStderr(stderr) {
   const {rest} = extractSummary(stderr);
-  return rest.replace(/.*(jest-jasmine2|jest-circus).*\n/g, '');
+  return rest.replace(/.*(jest-jasmine2).*\n/g, '');
 }
 
 beforeEach(() => {
@@ -112,6 +112,22 @@ test('only', () => {
   expect(summary).toMatchSnapshot();
 });
 
+test('cannot have describe with no implementation', () => {
+  const filename = 'only-constructs.test.js';
+  const content = `
+    describe('describe, no implementation');
+  `;
+
+  writeFiles(TEST_DIR, {[filename]: content});
+  const {stderr, status} = runJest(DIR);
+  expect(status).toBe(1);
+
+  const rest = cleanStderr(stderr);
+  const {summary} = extractSummary(stderr);
+  expect(rest).toMatchSnapshot();
+  expect(summary).toMatchSnapshot();
+});
+
 test('cannot test with no implementation', () => {
   const filename = 'only-constructs.test.js';
   const content = `

packages/jest-circus/src/__tests__/circus_it_test_error.test.js

@@ -35,24 +35,24 @@ describe('test/it error throwing', () => {
   });
   it(`it throws error with missing callback function`, () => {
     expect(() => {
-      // $FlowFixMe: Easy, we're testing runitme errors here
+      // $FlowFixMe: Easy, we're testing runtime errors here
       circusIt('test2');
     }).toThrowError(
       'Missing second argument. It must be a callback function. Perhaps you want to use `test.todo` for a test placeholder.',
     );
   });
   it(`it throws an error when first argument isn't a string`, () => {
     expect(() => {
-      // $FlowFixMe: Easy, we're testing runitme errors here
+      // $FlowFixMe: Easy, we're testing runtime errors here
       circusIt(() => {});
-    }).toThrowError(`Invalid first argument, () => {}. It must be a string.`);
+    }).toThrowError('Invalid first argument, () => {}. It must be a string.');
   });
   it('it throws an error when callback function is not a function', () => {
     expect(() => {
-      // $FlowFixMe: Easy, we're testing runitme errors here
+      // $FlowFixMe: Easy, we're testing runtime errors here
       circusIt('test4', 'test4b');
     }).toThrowError(
-      `Invalid second argument, test4b. It must be a callback function.`,
+      'Invalid second argument, test4b. It must be a callback function.',
     );
   });
   it(`test doesn't throw an error with valid arguments`, () => {
@@ -62,24 +62,24 @@ describe('test/it error throwing', () => {
   });
   it(`test throws error with missing callback function`, () => {
     expect(() => {
-      // $FlowFixMe: Easy, we're testing runitme errors here
+      // $FlowFixMe: Easy, we're testing runtime errors here
       circusTest('test6');
     }).toThrowError(
       'Missing second argument. It must be a callback function. Perhaps you want to use `test.todo` for a test placeholder.',
     );
   });
   it(`test throws an error when first argument isn't a string`, () => {
     expect(() => {
-      // $FlowFixMe: Easy, we're testing runitme errors here
+      // $FlowFixMe: Easy, we're testing runtime errors here
       circusTest(() => {});
-    }).toThrowError(`Invalid first argument, () => {}. It must be a string.`);
+    }).toThrowError('Invalid first argument, () => {}. It must be a string.');
   });
   it('test throws an error when callback function is not a function', () => {
     expect(() => {
-      // $FlowFixMe: Easy, we're testing runitme errors here
+      // $FlowFixMe: Easy, we're testing runtime errors here
       circusTest('test8', 'test8b');
     }).toThrowError(
-      `Invalid second argument, test8b. It must be a callback function.`,
+      'Invalid second argument, test8b. It must be a callback function.',
     );
   });
 });

packages/jest-circus/src/index.js

@@ -15,6 +15,7 @@ import type {
   BlockMode,
   BlockName,
   TestName,
+  TestMode,
 } from 'types/Circus';
 import {bind as bindEach} from 'jest-each';
 import {ErrorWithStack} from 'jest-util';
@@ -23,15 +24,29 @@ import {dispatch} from './state';
 type THook = (fn: HookFn, timeout?: number) => void;
 
 const describe = (blockName: BlockName, blockFn: BlockFn) =>
-  _dispatchDescribe(blockFn, blockName);
+  _dispatchDescribe(blockFn, blockName, describe);
 describe.only = (blockName: BlockName, blockFn: BlockFn) =>
-  _dispatchDescribe(blockFn, blockName, 'only');
+  _dispatchDescribe(blockFn, blockName, describe.only, 'only');
 describe.skip = (blockName: BlockName, blockFn: BlockFn) =>
-  _dispatchDescribe(blockFn, blockName, 'skip');
-
-const _dispatchDescribe = (blockFn, blockName, mode?: BlockMode) => {
+  _dispatchDescribe(blockFn, blockName, describe.skip, 'skip');
+
+const _dispatchDescribe = (
+  blockFn,
+  blockName,
+  describeFn,
+  mode?: BlockMode,
+) => {
+  const asyncError = new ErrorWithStack(undefined, describeFn);
+  if (blockFn === undefined) {
+    asyncError.message = `Missing second argument. It must be a callback function.`;
+    throw asyncError;
+  }
+  if (typeof blockFn !== 'function') {
+    asyncError.message = `Invalid second argument, ${blockFn}. It must be a callback function.`;
+    throw asyncError;
+  }
   dispatch({
-    asyncError: new Error(),
+    asyncError,
     blockName,
     mode,
     name: 'start_describe_definition',
@@ -41,11 +56,15 @@ const _dispatchDescribe = (blockFn, blockName, mode?: BlockMode) => {
 };
 
 const _addHook = (fn: HookFn, hookType: HookType, hookFn, timeout: ?number) => {
+  const asyncError = new ErrorWithStack(undefined, hookFn);
+
   if (typeof fn !== 'function') {
-    throw new Error('Invalid first argument. It must be a callback function.');
+    asyncError.message =
+      'Invalid first argument. It must be a callback function.';
+
+    throw asyncError;
   }
 
-  const asyncError = new ErrorWithStack(undefined, hookFn);
   dispatch({asyncError, fn, hookType, name: 'add_hook', timeout});
 };
 
@@ -59,76 +78,56 @@ const afterEach: THook = (fn, timeout) =>
 const afterAll: THook = (fn, timeout) =>
   _addHook(fn, 'afterAll', afterAll, timeout);
 
-const test = (testName: TestName, fn: TestFn, timeout?: number) => {
-  if (typeof testName !== 'string') {
-    throw new Error(
-      `Invalid first argument, ${testName}. It must be a string.`,
-    );
-  }
-  if (fn === undefined) {
-    throw new Error(
-      'Missing second argument. It must be a callback function. Perhaps you want to use `test.todo` for a test placeholder.',
-    );
-  }
-  if (typeof fn !== 'function') {
-    throw new Error(
-      `Invalid second argument, ${fn}. It must be a callback function.`,
-    );
-  }
-
-  const asyncError = new ErrorWithStack(undefined, test);
-
-  return dispatch({
-    asyncError,
-    fn,
-    name: 'add_test',
-    testName,
-    timeout,
-  });
-};
+const test = (testName: TestName, fn: TestFn, timeout?: number) =>
+  _addTest(testName, undefined, fn, test, timeout);
 const it = test;
-test.skip = (testName: TestName, fn?: TestFn, timeout?: number) => {
-  const asyncError = new ErrorWithStack(undefined, test);
-
-  return dispatch({
-    asyncError,
-    fn,
-    mode: 'skip',
-    name: 'add_test',
-    testName,
-    timeout,
-  });
-};
-test.only = (testName: TestName, fn: TestFn, timeout?: number) => {
-  const asyncError = new ErrorWithStack(undefined, test);
-
-  return dispatch({
-    asyncError,
-    fn,
-    mode: 'only',
-    name: 'add_test',
-    testName,
-    timeout,
-  });
-};
-
+test.skip = (testName: TestName, fn?: TestFn, timeout?: number) =>
+  _addTest(testName, 'skip', fn, test.skip, timeout);
+test.only = (testName: TestName, fn: TestFn, timeout?: number) =>
+  _addTest(testName, 'only', fn, test.only, timeout);
 test.todo = (testName: TestName, ...rest: Array<mixed>) => {
   if (rest.length > 0 || typeof testName !== 'string') {
     throw new ErrorWithStack(
       'Todo must be called with only a description.',
       test.todo,
     );
   }
+  return _addTest(testName, 'todo', () => {}, test.todo);
+};
+
+const _addTest = (
+  testName: TestName,
+  mode: TestMode,
+  fn?: TestFn,
+  testFn,
+  timeout: ?number,
+) => {
+  const asyncError = new ErrorWithStack(undefined, testFn);
+
+  if (typeof testName !== 'string') {
+    asyncError.message = `Invalid first argument, ${testName}. It must be a string.`;
 
-  const asyncError = new ErrorWithStack(undefined, test);
+    throw asyncError;
+  }
+  if (fn === undefined) {
+    asyncError.message =
+      'Missing second argument. It must be a callback function. Perhaps you want to use `test.todo` for a test placeholder.';
+
+    throw asyncError;
+  }
+  if (typeof fn !== 'function') {
+    asyncError.message = `Invalid second argument, ${fn}. It must be a callback function.`;
+
+    throw asyncError;
+  }
 
   return dispatch({
     asyncError,
-    fn: () => {},
-    mode: 'todo',
+    fn,
+    mode,
     name: 'add_test',
     testName,
-    timeout: undefined,
+    timeout,
   });
 };
 

packages/jest-jasmine2/src/__tests__/it_test_error.test.js

@@ -25,7 +25,7 @@ describe('test/it error throwing', () => {
     expect(() => {
       it('test3', 'test3b');
     }).toThrowError(
-      `Invalid second argument, test3b. It must be a callback function.`,
+      'Invalid second argument, test3b. It must be a callback function.',
     );
   });
   test(`test throws error with missing callback function`, () => {
@@ -44,7 +44,7 @@ describe('test/it error throwing', () => {
     expect(() => {
       test('test6', 'test6b');
     }).toThrowError(
-      `Invalid second argument, test6b. It must be a callback function.`,
+      'Invalid second argument, test6b. It must be a callback function.',
     );
   });
 });

packages/jest-jasmine2/src/jasmine/Env.js

@@ -325,6 +325,16 @@ export default function(j$) {
 
     this.describe = function(description, specDefinitions) {
       const suite = suiteFactory(description);
+      if (specDefinitions === undefined) {
+        throw new Error(
+          `Missing second argument. It must be a callback function.`,
+        );
+      }
+      if (typeof specDefinitions !== 'function') {
+        throw new Error(
+          `Invalid second argument, ${specDefinitions}. It must be a callback function.`,
+        );
+      }
       if (specDefinitions.length > 0) {
         throw new Error('describe does not expect any arguments');
       }