From 91e83e2bcd72c3f9cec9c005c107e63022e629fb Mon Sep 17 00:00:00 2001 From: Nathanael Beisiegel Date: Mon, 2 Jul 2018 13:56:35 -0400 Subject: [PATCH 1/2] Reallowed single string arguments for pending tests - Continues to validate arguments to avoid accidental skips from #5558 --- CHANGELOG.md | 2 + .../__snapshots__/globals.test.js.snap | 87 +++++++------------ e2e/__tests__/globals.test.js | 8 +- .../__tests__/circus_it_test_error.test.js | 20 ++--- packages/jest-circus/src/index.js | 5 +- .../src/__tests__/it_test_error.test.js | 8 +- packages/jest-jasmine2/src/jasmine/Env.js | 7 +- 7 files changed, 55 insertions(+), 82 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 8b65f07552de..9c4367ee1d0e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,8 @@ ### Features +- `[jest-jasmine2]` Updated error throwing with `it`/ `test` to re-allow the valid single string argument for pending tests. +- `[jest-circus]` Updated error throwing with `it`/ `test` to re-allow the valid single string argument for pending tests - `[jest-haste-map]` Add `computeDependencies` flag to avoid opening files if not needed ([#6667](https://github.com/facebook/jest/pull/6667)) - `[jest-runtime]` Support `require.resolve.paths` ([#6471](https://github.com/facebook/jest/pull/6471)) - `[jest-runtime]` Support `paths` option for `require.resolve` ([#6471](https://github.com/facebook/jest/pull/6471)) diff --git a/e2e/__tests__/__snapshots__/globals.test.js.snap b/e2e/__tests__/__snapshots__/globals.test.js.snap index 4e61b01522ec..29079f1b2b09 100644 --- a/e2e/__tests__/__snapshots__/globals.test.js.snap +++ b/e2e/__tests__/__snapshots__/globals.test.js.snap @@ -19,60 +19,6 @@ Time: <> 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. - - 1 | - 2 | it('it', () => {}); - > 3 | it('it, no implementation'); - | ^ - 4 | test('test, no implementation'); - 5 | - - 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: <> -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. - - 1 | - 2 | it('it', () => {}); - > 3 | it('it, no implementation'); - | ^ - 4 | test('test, no implementation'); - 5 | - - 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: <> -Ran all test suites. -" -`; - exports[`function as descriptor 1`] = ` "PASS __tests__/function-as-descriptor.test.js Foo @@ -186,3 +132,36 @@ Snapshots: 0 total Time: <> 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: <> +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: <> +Ran all test suites." +`; diff --git a/e2e/__tests__/globals.test.js b/e2e/__tests__/globals.test.js index af217505bd65..1ca02d31220d 100644 --- a/e2e/__tests__/globals.test.js +++ b/e2e/__tests__/globals.test.js @@ -112,7 +112,7 @@ test('only', () => { expect(summary).toMatchSnapshot(); }); -test('cannot test with no implementation', () => { +test('tests with no implementation', () => { const filename = 'only-constructs.test.js'; const content = ` it('it', () => {}); @@ -122,7 +122,7 @@ test('cannot test with no implementation', () => { writeFiles(TEST_DIR, {[filename]: content}); const {stderr, status} = runJest(DIR); - expect(status).toBe(1); + expect(status).toBe(0); const {summary} = extractSummary(stderr); expect(cleanStderr(stderr)).toMatchSnapshot(); @@ -190,7 +190,7 @@ test('only with expand arg', () => { expect(summary).toMatchSnapshot(); }); -test('cannot test with no implementation with expand arg', () => { +test('tests with no implementation with expand arg', () => { const filename = 'only-constructs.test.js'; const content = ` it('it', () => {}); @@ -200,7 +200,7 @@ test('cannot test with no implementation with expand arg', () => { writeFiles(TEST_DIR, {[filename]: content}); const {stderr, status} = runJest(DIR, ['--expand']); - expect(status).toBe(1); + expect(status).toBe(0); const {summary} = extractSummary(stderr); expect(cleanStderr(stderr)).toMatchSnapshot(); diff --git a/packages/jest-circus/src/__tests__/circus_it_test_error.test.js b/packages/jest-circus/src/__tests__/circus_it_test_error.test.js index 4fd4e2d99f27..45c920736831 100644 --- a/packages/jest-circus/src/__tests__/circus_it_test_error.test.js +++ b/packages/jest-circus/src/__tests__/circus_it_test_error.test.js @@ -33,21 +33,21 @@ describe('test/it error throwing', () => { circusIt('test1', () => {}); }).not.toThrowError(); }); - it(`it throws error with missing callback function`, () => { + it(`it doesn't throw 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.'); + }).not.toThrowError(); }); 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.`); }); 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.`, @@ -58,21 +58,21 @@ describe('test/it error throwing', () => { circusTest('test5', () => {}); }).not.toThrowError(); }); - it(`test throws error with missing callback function`, () => { + it(`test doesn't throw 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.'); + }).not.toThrowError(); }); 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.`); }); 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.`, diff --git a/packages/jest-circus/src/index.js b/packages/jest-circus/src/index.js index 91e315b1241d..104b3aad2639 100644 --- a/packages/jest-circus/src/index.js +++ b/packages/jest-circus/src/index.js @@ -63,10 +63,7 @@ const test = (testName: TestName, fn: TestFn, timeout?: number) => { `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') { + if (fn !== undefined && typeof fn !== 'function') { throw new Error( `Invalid second argument, ${fn}. It must be a callback function.`, ); diff --git a/packages/jest-jasmine2/src/__tests__/it_test_error.test.js b/packages/jest-jasmine2/src/__tests__/it_test_error.test.js index 9f1727645ffd..38b7906aea19 100644 --- a/packages/jest-jasmine2/src/__tests__/it_test_error.test.js +++ b/packages/jest-jasmine2/src/__tests__/it_test_error.test.js @@ -9,10 +9,10 @@ 'use strict'; describe('test/it error throwing', () => { - it(`it throws error with missing callback function`, () => { + it(`it doesn't throw error with missing callback function`, () => { expect(() => { it('test1'); - }).toThrowError('Missing second argument. It must be a callback function.'); + }).not.toThrowError(/argument/i); }); it(`it throws an error when first argument isn't a string`, () => { expect(() => { @@ -26,10 +26,10 @@ describe('test/it error throwing', () => { `Invalid second argument, test3b. It must be a callback function.`, ); }); - test(`test throws error with missing callback function`, () => { + test(`test doesn't throw error with missing callback function`, () => { expect(() => { test('test4'); - }).toThrowError('Missing second argument. It must be a callback function.'); + }).not.toThrowError(/argument/i); }); test(`test throws an error when first argument isn't a string`, () => { expect(() => { diff --git a/packages/jest-jasmine2/src/jasmine/Env.js b/packages/jest-jasmine2/src/jasmine/Env.js index d547c0a8f271..cddd32b63e00 100644 --- a/packages/jest-jasmine2/src/jasmine/Env.js +++ b/packages/jest-jasmine2/src/jasmine/Env.js @@ -448,12 +448,7 @@ export default function(j$) { `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') { + if (fn !== undefined && typeof fn !== 'function') { throw new Error( `Invalid second argument, ${fn}. It must be a callback function.`, ); From 0d0aad3879652a004a05a7c2b045e8d386ad6258 Mon Sep 17 00:00:00 2001 From: Nathanael Beisiegel Date: Mon, 2 Jul 2018 16:08:58 -0400 Subject: [PATCH 2/2] Updated jest-circus to set mode to 'skip' when string-only --- packages/jest-circus/src/index.js | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/jest-circus/src/index.js b/packages/jest-circus/src/index.js index 104b3aad2639..bf1ce883495d 100644 --- a/packages/jest-circus/src/index.js +++ b/packages/jest-circus/src/index.js @@ -77,6 +77,7 @@ const test = (testName: TestName, fn: TestFn, timeout?: number) => { return dispatch({ asyncError, fn, + mode: fn ? undefined : 'skip', name: 'add_test', testName, timeout,