Add error throwing to 'it'/'test' for incorrect arguments.

.flowconfig

@@ -1,6 +1,7 @@
 [ignore]
 .*/examples/.*
 .*/node_modules/metro-bundler/.*
+.*/circus_it_test_error.test.js*.
 
 [options]
 module.name_mapper='^pretty-format$' -> '<PROJECT_ROOT>/packages/pretty-format/src/index.js'

CHANGELOG.md

@@ -2,6 +2,9 @@
 
 ### Features
 
+* `[jest-jasmine2]` Adds error throwing and descriptive errors to `it`/ `test` 
+  for invalid arguements. `[jest-circus]` Adds error throwing and descriptive 
+  errors to `it`/ `test` for invalid arguements. 
 * `[jest-util]` Add the following methods to the "console" implementations:
   `assert`, `count`, `countReset`, `dir`, `dirxml`, `group`, `groupCollapsed`,
   `groupEnd`, `time`, `timeEnd`

integration-tests/__tests__/__snapshots__/globals.test.js.snap

@@ -20,6 +20,79 @@ Ran all test suites.
 "
 `;
 
+exports[`cannot test with no implementation 1`] = `
+"FAIL __tests__/only-constructs.test.js
+  ● Test suite failed to run
+
+    Missing second argument. It must be a callback function.
+
+      431 |       }
+      432 |       if (fn === undefined) {
+    > 433 |         throw new Error(
+      434 |         'Missing second argument. It must be a callback function.');
+      435 | 
+      436 |       }
+
+
+      at __tests__/only-constructs.test.js:3:5
+
+"
+`;
+
+exports[`cannot test 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 with expand arg 1`] = `
+"FAIL __tests__/only-constructs.test.js
+  ● Test suite failed to run
+
+    Missing second argument. It must be a callback function.
+
+      431 |       }
+      432 |       if (fn === undefined) {
+    > 433 |         throw new Error(
+      434 |         'Missing second argument. It must be a callback function.');
+      435 | 
+      436 |       }
+
+
+      at __tests__/only-constructs.test.js:3:5
+
+"
+`;
+
+exports[`cannot test with no implementation with expand arg 2`] = `
+"Test Suites: 1 failed, 1 total
+Tests:       0 total
+Snapshots:   0 total
+Time:        <<REPLACED>>
+Ran all test suites.
+"
+`;
+
+exports[`function as descriptor 1`] = `
+"PASS __tests__/function-as-descriptor.test.js
+  Foo
+    ✓ it
+
+"
+`;
+
+exports[`function as descriptor 2`] = `
+"Test Suites: 1 passed, 1 total
+Tests:       1 passed, 1 total
+Snapshots:   0 total
+Time:        <<REPLACED>>
+Ran all test suites.
+"
+`;
+
 exports[`only 1`] = `
 "PASS __tests__/only-constructs.test.js
   ✓ test.only
@@ -121,55 +194,3 @@ Time:        <<REPLACED>>
 Ran all test suites.
 "
 `;
-
-exports[`tests with no implementation 1`] = `
-"PASS __tests__/only-constructs.test.js
-  ✓ it
-  ○ skipped 2 tests
-
-"
-`;
-
-exports[`tests with no implementation 2`] = `
-"Test Suites: 1 passed, 1 total
-Tests:       2 skipped, 1 passed, 3 total
-Snapshots:   0 total
-Time:        <<REPLACED>>
-Ran all test suites.
-"
-`;
-
-exports[`tests with no implementation with expand arg 1`] = `
-"PASS __tests__/only-constructs.test.js
-  ✓ it
-  ○ it, no implementation
-  ○ test, no implementation
-
-"
-`;
-
-exports[`tests with no implementation with expand arg 2`] = `
-"Test Suites: 1 passed, 1 total
-Tests:       2 skipped, 1 passed, 3 total
-Snapshots:   0 total
-Time:        <<REPLACED>>
-Ran all test suites.
-"
-`;
-
-exports[`function as descriptor 1`] = `
-"PASS __tests__/function-as-descriptor.test.js
-  Foo
-    ✓ it
-
-"
-`;
-
-exports[`function as descriptor 2`] = `
-"Test Suites: 1 passed, 1 total
-Tests:       1 passed, 1 total
-Snapshots:   0 total
-Time:        <<REPLACED>>
-Ran all test suites.
-"
-`;

integration-tests/__tests__/globals.test.js

@@ -110,7 +110,7 @@ test('only', () => {
   expect(summary).toMatchSnapshot();
 });
 
-test('tests with no implementation', () => {
+test('cannot test with no implementation', () => {
   const filename = 'only-constructs.test.js';
   const content = `
     it('it', () => {});
@@ -120,9 +120,10 @@ test('tests with no implementation', () => {
 
   writeFiles(TEST_DIR, {[filename]: content});
   const {stderr, status} = runJest(DIR);
-  expect(status).toBe(0);
+  expect(status).toBe(1);
 
   const {summary, rest} = extractSummary(stderr);
+
   expect(rest).toMatchSnapshot();
   expect(summary).toMatchSnapshot();
 });
@@ -188,7 +189,7 @@ test('only with expand arg', () => {
   expect(summary).toMatchSnapshot();
 });
 
-test('tests with no implementation with expand arg', () => {
+test('cannot test with no implementation with expand arg', () => {
   const filename = 'only-constructs.test.js';
   const content = `
     it('it', () => {});
@@ -198,7 +199,7 @@ test('tests with no implementation with expand arg', () => {
 
   writeFiles(TEST_DIR, {[filename]: content});
   const {stderr, status} = runJest(DIR, ['--expand']);
-  expect(status).toBe(0);
+  expect(status).toBe(1);
 
   const {summary, rest} = extractSummary(stderr);
   expect(rest).toMatchSnapshot();

integration-tests/utils.js

@@ -11,7 +11,7 @@
 
 import type {Path} from 'types/Config';
 
-const {spawnSync} = require('child_process');
+const {sync: spawnSync} = require('cross-spawn');
 const fs = require('fs');
 const path = require('path');
 const mkdirp = require('mkdirp');
@@ -43,8 +43,9 @@ const linkJestPackage = (packageName: string, cwd: Path) => {
   const packagesDir = path.resolve(__dirname, '../packages');
   const packagePath = path.resolve(packagesDir, packageName);
   const destination = path.resolve(cwd, 'node_modules/');
-  run(`mkdir -p ${destination}`);
-  return run(`ln -sf ${packagePath} ${destination}`);
+  mkdirp.sync(destination);
+  rimraf.sync(destination);
+  fs.symlinkSync(packagePath, destination, 'dir');
 };
 
 const fileExists = (filePath: Path) => {
@@ -158,7 +159,9 @@ const extractSummary = (stdout: string) => {
 // unifies their output to make it possible to snapshot them.
 // TODO: Remove when we drop support for node 4
 const cleanupStackTrace = (output: string) => {
-  return output.replace(/^.*at.*[\s][\(]?(\S*\:\d*\:\d*).*$/gm, '      at $1');
+  return output
+    .replace(/^.*\b(at Env.it)\b.*$/gm, '')
+    .replace(/^.*at.*[\s][\(]?(\S*\:\d*\:\d*).*$/gm, '      at $1');
 };
 
 module.exports = {

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

@@ -0,0 +1,75 @@
+/**
+ * Copyright (c) 2015-present, Facebook, Inc. All rights reserved.
+ *
+ * This source code is licensed under the MIT license found in the
+ * LICENSE file in the root directory of this source tree.
+ *
+ * *@flow
+ */
+
+'use strict';
+
+let circusIt;
+let circusTest;
+
+//using jest-jasmine2's 'it' to test jest-circus's 'it'. Had to differentiate
+//the two with this aliaser.
+
+const aliasCircusIt = () => {
+  const {it, test} = require('../index.js');
+  circusIt = it;
+  circusTest = test;
+};
+
+aliasCircusIt();
+
+//A few of these tests require incorrect types to throw errors and thus pass
+//the test. The typechecks on jest-circus would prevent that, so
+//this file has been listed in the .flowconfig ignore section.
+
+describe('test/it error throwing', () => {
+  it(`it doesn't throw an error with valid arguments`, () => {
+    expect(() => {
+      circusIt('test1', () => {});
+    }).not.toThrowError();
+  });
+  it(`it throws error with missing callback function`, () => {
+    expect(() => {
+      circusIt('test2');
+    }).toThrowError('Missing second argument. It must be a callback function.');
+  });
+  it(`it throws an error when first argument isn't a string`, () => {
+    expect(() => {
+      circusIt(() => {});
+    }).toThrowError(`Invalid first argument, () => {}. It must be a string.`);
+  });
+  it('it throws an error when callback function is not a function', () => {
+    expect(() => {
+      circusIt('test4', 'test4b');
+    }).toThrowError(
+      `Invalid second argument, test4b. It must be a callback function.`,
+    );
+  });
+  it(`test doesn't throw an error with valid arguments`, () => {
+    expect(() => {
+      circusTest('test5', () => {});
+    }).not.toThrowError();
+  });
+  it(`test throws error with missing callback function`, () => {
+    expect(() => {
+      circusTest('test6');
+    }).toThrowError('Missing second argument. It must be a callback function.');
+  });
+  it(`test throws an error when first argument isn't a string`, () => {
+    expect(() => {
+      circusTest(() => {});
+    }).toThrowError(`Invalid first argument, () => {}. It must be a string.`);
+  });
+  it('test throws an error when callback function is not a function', () => {
+    expect(() => {
+      circusTest('test8', 'test8b');
+    }).toThrowError(
+      `Invalid second argument, test8b. It must be a callback function.`,
+    );
+  });
+});

packages/jest-circus/src/index.js

@@ -38,8 +38,22 @@ const beforeAll = (fn: HookFn) => _addHook(fn, 'beforeAll');
 const afterEach = (fn: HookFn) => _addHook(fn, 'afterEach');
 const afterAll = (fn: HookFn) => _addHook(fn, 'afterAll');
 
-const test = (testName: TestName, fn?: TestFn) =>
-  dispatch({fn, name: 'add_test', testName});
+const test = (testName: TestName, fn: TestFn) => {
+  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.');
+  }
+  if (typeof fn !== 'function') {
+    throw new Error(
+      `Invalid second argument, ${fn}. It must be a callback function.`,
+    );
+  }
+  return dispatch({fn, name: 'add_test', testName});
+};
 const it = test;
 test.skip = (testName: TestName, fn?: TestFn) =>
   dispatch({fn, mode: 'skip', name: 'add_test', testName});

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

@@ -0,0 +1,46 @@
+/**
+ * Copyright (c) 2015-present, Facebook, Inc. All rights reserved.
+ *
+ * This source code is licensed under the MIT license found in the
+ * LICENSE file in the root directory of this source tree.
+ *
+ */
+
+'use strict';
+
+describe('test/it error throwing', () => {
+  it(`it throws error with missing callback function`, () => {
+    expect(() => {
+      it('test1');
+    }).toThrowError('Missing second argument. It must be a callback function.');
+  });
+  it(`it throws an error when first argument isn't a string`, () => {
+    expect(() => {
+      it(() => {});
+    }).toThrowError(`Invalid first argument, () => {}. It must be a string.`);
+  });
+  it('it throws an error when callback function is not a function', () => {
+    expect(() => {
+      it('test3', 'test3b');
+    }).toThrowError(
+      `Invalid second argument, test3b. It must be a callback function.`,
+    );
+  });
+  test(`test throws error with missing callback function`, () => {
+    expect(() => {
+      test('test4');
+    }).toThrowError('Missing second argument. It must be a callback function.');
+  });
+  test(`test throws an error when first argument isn't a string`, () => {
+    expect(() => {
+      test(() => {});
+    }).toThrowError(`Invalid first argument, () => {}. It must be a string.`);
+  });
+  test('test throws an error when callback function is not a function', () => {
+    expect(() => {
+      test('test6', 'test6b');
+    }).toThrowError(
+      `Invalid second argument, test6b. It must be a callback function.`,
+    );
+  });
+});

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

@@ -424,6 +424,21 @@ export default function(j$) {
     };
 
     this.it = function(description, fn, timeout) {
+      if (typeof description !== 'string') {
+        throw new Error(
+          `Invalid first argument, ${description}. It must be a string.`,
+        );
+      }
+      if (fn === undefined) {
+        throw new Error(
+          'Missing second argument. It must be a callback function.',
+        );
+      }
+      if (typeof fn !== 'function') {
+        throw new Error(
+          `Invalid second argument, ${fn}. It must be a callback function.`,
+        );
+      }
       const spec = specFactory(
         description,
         fn,