From e50280fc0022829cc4221d100357d064f6fb6c5d Mon Sep 17 00:00:00 2001 From: Alcedo Nathaniel De Guzman Jr Date: Fri, 31 Aug 2018 16:44:56 +0800 Subject: [PATCH 1/6] Move validation checks of callback function for jest hooks from Env.js to jasmine_light.js --- .../src/__tests__/hooks_error.test.js | 10 ++++---- packages/jest-jasmine2/src/jasmine/Env.js | 24 ------------------- .../src/jasmine/jasmine_light.js | 20 ++++++++++++++++ 3 files changed, 26 insertions(+), 28 deletions(-) diff --git a/packages/jest-jasmine2/src/__tests__/hooks_error.test.js b/packages/jest-jasmine2/src/__tests__/hooks_error.test.js index a173edd02029..50a4e3f277ed 100644 --- a/packages/jest-jasmine2/src/__tests__/hooks_error.test.js +++ b/packages/jest-jasmine2/src/__tests__/hooks_error.test.js @@ -13,10 +13,12 @@ describe('hooks error throwing', () => { test.each([['beforeEach'], ['beforeAll'], ['afterEach'], ['afterAll']])( '%s throws an error when the first argument is not a function', fn => { - expect(() => { - global[fn]('param'); - }).toThrowError( - 'Invalid first argument, param. It must be a callback function.', + ['String', 1, {}, Symbol('hello'), true, null, undefined].forEach(el => + expect(() => { + global[fn](el); + }).toThrowError( + 'Invalid first argument. It must be a callback function.', + ), ); }, ); diff --git a/packages/jest-jasmine2/src/jasmine/Env.js b/packages/jest-jasmine2/src/jasmine/Env.js index 06856d5c465d..d547c0a8f271 100644 --- a/packages/jest-jasmine2/src/jasmine/Env.js +++ b/packages/jest-jasmine2/src/jasmine/Env.js @@ -503,12 +503,6 @@ export default function(j$) { }; this.beforeEach = function(beforeEachFunction, timeout) { - if (typeof beforeEachFunction !== 'function') { - throw new Error( - `Invalid first argument, ${beforeEachFunction}. It must be a callback function.`, - ); - } - currentDeclarationSuite.beforeEach({ fn: beforeEachFunction, timeout() { @@ -518,12 +512,6 @@ export default function(j$) { }; this.beforeAll = function(beforeAllFunction, timeout) { - if (typeof beforeAllFunction !== 'function') { - throw new Error( - `Invalid first argument, ${beforeAllFunction}. It must be a callback function.`, - ); - } - currentDeclarationSuite.beforeAll({ fn: beforeAllFunction, timeout() { @@ -533,12 +521,6 @@ export default function(j$) { }; this.afterEach = function(afterEachFunction, timeout) { - if (typeof afterEachFunction !== 'function') { - throw new Error( - `Invalid first argument, ${afterEachFunction}. It must be a callback function.`, - ); - } - currentDeclarationSuite.afterEach({ fn: afterEachFunction, timeout() { @@ -548,12 +530,6 @@ export default function(j$) { }; this.afterAll = function(afterAllFunction, timeout) { - if (typeof afterAllFunction !== 'function') { - throw new Error( - `Invalid first argument, ${afterAllFunction}. It must be a callback function.`, - ); - } - currentDeclarationSuite.afterAll({ fn: afterAllFunction, timeout() { diff --git a/packages/jest-jasmine2/src/jasmine/jasmine_light.js b/packages/jest-jasmine2/src/jasmine/jasmine_light.js index ab1106bab399..9f6e5d2cb857 100644 --- a/packages/jest-jasmine2/src/jasmine/jasmine_light.js +++ b/packages/jest-jasmine2/src/jasmine/jasmine_light.js @@ -93,18 +93,38 @@ exports.interface = function(jasmine: Jasmine, env: any) { }, beforeEach() { + if (typeof arguments[0] !== 'function') { + throw new Error( + `Invalid first argument. It must be a callback function.`, + ); + } return env.beforeEach.apply(env, arguments); }, afterEach() { + if (typeof arguments[0] !== 'function') { + throw new Error( + `Invalid first argument. It must be a callback function.`, + ); + } return env.afterEach.apply(env, arguments); }, beforeAll() { + if (typeof arguments[0] !== 'function') { + throw new Error( + `Invalid first argument. It must be a callback function.`, + ); + } return env.beforeAll.apply(env, arguments); }, afterAll() { + if (typeof arguments[0] !== 'function') { + throw new Error( + `Invalid first argument. It must be a callback function.`, + ); + } return env.afterAll.apply(env, arguments); }, From d0c28bd03fb685e27675668aedcd3beee86126a4 Mon Sep 17 00:00:00 2001 From: Alcedo Nathaniel De Guzman Jr Date: Fri, 31 Aug 2018 16:45:35 +0800 Subject: [PATCH 2/6] Standardise error message for validating hook functions in circus similar to how jasmine_light does it --- packages/jest-circus/src/__tests__/hooks_error.test.js | 10 ++++++---- packages/jest-circus/src/index.js | 4 +--- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/packages/jest-circus/src/__tests__/hooks_error.test.js b/packages/jest-circus/src/__tests__/hooks_error.test.js index 10a5509aec6c..5ca64096d274 100644 --- a/packages/jest-circus/src/__tests__/hooks_error.test.js +++ b/packages/jest-circus/src/__tests__/hooks_error.test.js @@ -15,10 +15,12 @@ describe('hooks error throwing', () => { test.each([['beforeEach'], ['beforeAll'], ['afterEach'], ['afterAll']])( '%s throws an error when the first argument is not a function', fn => { - expect(() => { - circus[fn]('param'); - }).toThrowError( - 'Invalid first argument, param. It must be a callback function.', + ['String', 1, {}, Symbol('hello'), true, null, undefined].forEach(el => + expect(() => { + circus[fn](el); + }).toThrowError( + 'Invalid first argument. It must be a callback function.', + ), ); }, ); diff --git a/packages/jest-circus/src/index.js b/packages/jest-circus/src/index.js index feaf921d7481..f0a3f0ddb905 100644 --- a/packages/jest-circus/src/index.js +++ b/packages/jest-circus/src/index.js @@ -41,9 +41,7 @@ const _dispatchDescribe = (blockFn, blockName, mode?: BlockMode) => { const _addHook = (fn: HookFn, hookType: HookType, hookFn, timeout: ?number) => { if (typeof fn !== 'function') { - throw new Error( - `Invalid first argument, ${fn}. It must be a callback function.`, - ); + throw new Error(`Invalid first argument. It must be a callback function.`); } const asyncError = new Error(); From 6fec1d2c715f0b5e5aa5a703fbdf9b7482a2ad80 Mon Sep 17 00:00:00 2001 From: Alcedo Nathaniel De Guzman Jr Date: Fri, 31 Aug 2018 18:08:41 +0800 Subject: [PATCH 3/6] Update CHANGELOG --- CHANGELOG.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 53c3b50f6658..bedea88fdb91 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,8 +8,8 @@ - `[jest-resolve]` Only resolve realpath once in try-catch ([#6925](https://github.com/facebook/jest/pull/6925)) - `[expect]` Fix TypeError in `toBeInstanceOf` on `null` or `undefined` ([#6912](https://github.com/facebook/jest/pull/6912)) -- `[jest-jasmine2]` Throw a descriptive error if the first argument supplied to a hook was not a function ([#6917](https://github.com/facebook/jest/pull/6917)) -- `[jest-circus]` Throw a descriptive error if the first argument supplied to a hook was not a function ([#6917](https://github.com/facebook/jest/pull/6917)) +- `[jest-jasmine2]` Throw a descriptive error if the first argument supplied to a hook was not a function ([#6917](https://github.com/facebook/jest/pull/6917)) and ([#6931](https://github.com/facebook/jest/pull/6931)) +- `[jest-circus]` Throw a descriptive error if the first argument supplied to a hook was not a function ([#6917](https://github.com/facebook/jest/pull/6917)) and ([#6931](https://github.com/facebook/jest/pull/6931)) - `[expect]` Fix variadic custom asymmetric matchers ([#6898](https://github.com/facebook/jest/pull/6898)) - `[jest-cli]` Fix incorrect `testEnvironmentOptions` warning ([#6852](https://github.com/facebook/jest/pull/6852)) - `[jest-each]` Prevent done callback being supplied to describe ([#6843](https://github.com/facebook/jest/pull/6843)) From f15c5125086c92198e7f911eee19cce4021fd35b Mon Sep 17 00:00:00 2001 From: Alcedo Nathaniel De Guzman Jr Date: Sat, 1 Sep 2018 00:34:41 +0800 Subject: [PATCH 4/6] Change backtick to singlequote when not needed in circus and jasmine_light --- packages/jest-circus/src/index.js | 2 +- packages/jest-jasmine2/src/jasmine/jasmine_light.js | 8 ++++---- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/packages/jest-circus/src/index.js b/packages/jest-circus/src/index.js index f0a3f0ddb905..790987a43349 100644 --- a/packages/jest-circus/src/index.js +++ b/packages/jest-circus/src/index.js @@ -41,7 +41,7 @@ const _dispatchDescribe = (blockFn, blockName, mode?: BlockMode) => { const _addHook = (fn: HookFn, hookType: HookType, hookFn, timeout: ?number) => { if (typeof fn !== 'function') { - throw new Error(`Invalid first argument. It must be a callback function.`); + throw new Error('Invalid first argument. It must be a callback function.'); } const asyncError = new Error(); diff --git a/packages/jest-jasmine2/src/jasmine/jasmine_light.js b/packages/jest-jasmine2/src/jasmine/jasmine_light.js index 9f6e5d2cb857..f79918edcdeb 100644 --- a/packages/jest-jasmine2/src/jasmine/jasmine_light.js +++ b/packages/jest-jasmine2/src/jasmine/jasmine_light.js @@ -95,7 +95,7 @@ exports.interface = function(jasmine: Jasmine, env: any) { beforeEach() { if (typeof arguments[0] !== 'function') { throw new Error( - `Invalid first argument. It must be a callback function.`, + 'Invalid first argument. It must be a callback function.', ); } return env.beforeEach.apply(env, arguments); @@ -104,7 +104,7 @@ exports.interface = function(jasmine: Jasmine, env: any) { afterEach() { if (typeof arguments[0] !== 'function') { throw new Error( - `Invalid first argument. It must be a callback function.`, + 'Invalid first argument. It must be a callback function.', ); } return env.afterEach.apply(env, arguments); @@ -113,7 +113,7 @@ exports.interface = function(jasmine: Jasmine, env: any) { beforeAll() { if (typeof arguments[0] !== 'function') { throw new Error( - `Invalid first argument. It must be a callback function.`, + 'Invalid first argument. It must be a callback function.', ); } return env.beforeAll.apply(env, arguments); @@ -122,7 +122,7 @@ exports.interface = function(jasmine: Jasmine, env: any) { afterAll() { if (typeof arguments[0] !== 'function') { throw new Error( - `Invalid first argument. It must be a callback function.`, + 'Invalid first argument. It must be a callback function.', ); } return env.afterAll.apply(env, arguments); From 1ec72189eefcb3afb2f53d1485187b9fe7241cb6 Mon Sep 17 00:00:00 2001 From: Alcedo Nathaniel De Guzman Jr Date: Sat, 1 Sep 2018 01:43:38 +0800 Subject: [PATCH 5/6] Add types for test.each and describe.each --- flow-typed/npm/jest_v23.x.x.js | 27 +++++++++++++++++++++++++-- 1 file changed, 25 insertions(+), 2 deletions(-) diff --git a/flow-typed/npm/jest_v23.x.x.js b/flow-typed/npm/jest_v23.x.x.js index 23b66b07e582..e8e487175a05 100644 --- a/flow-typed/npm/jest_v23.x.x.js +++ b/flow-typed/npm/jest_v23.x.x.js @@ -914,7 +914,19 @@ declare var describe: { /** * Skip running this describe block */ - skip(name: JestTestName, fn: () => void): void + skip(name: JestTestName, fn: () => void): void, + + /** + * each runs this test against array of argument arrays per each run + * + * @param {table} table of Test + */ + each( + table: Array> + ): ( + name: JestTestName, + fn?: (...args: Array) => ?Promise + ) => void, }; /** An individual test unit */ @@ -984,7 +996,18 @@ declare var it: { name: JestTestName, fn?: (done: () => void) => ?Promise, timeout?: number - ): void + ): void, + /** + * each runs this test against array of argument arrays per each run + * + * @param {table} table of Test + */ + each( + table: Array> + ): ( + name: JestTestName, + fn?: (...args: Array) => ?Promise + ) => void, }; declare function fit( name: JestTestName, From 5c45f71c3c3606f0c452b40dbcfe96853861e94b Mon Sep 17 00:00:00 2001 From: Alcedo Nathaniel De Guzman Jr Date: Sat, 1 Sep 2018 10:42:00 +0800 Subject: [PATCH 6/6] Use describe.each along with test.each for testing hook argument validation --- .../src/__tests__/hooks_error.test.js | 30 ++++++++++++------- .../src/__tests__/hooks_error.test.js | 30 ++++++++++++------- 2 files changed, 40 insertions(+), 20 deletions(-) diff --git a/packages/jest-circus/src/__tests__/hooks_error.test.js b/packages/jest-circus/src/__tests__/hooks_error.test.js index 5ca64096d274..6faab9809eaf 100644 --- a/packages/jest-circus/src/__tests__/hooks_error.test.js +++ b/packages/jest-circus/src/__tests__/hooks_error.test.js @@ -11,17 +11,27 @@ const circus = require('../index.js'); -describe('hooks error throwing', () => { - test.each([['beforeEach'], ['beforeAll'], ['afterEach'], ['afterAll']])( - '%s throws an error when the first argument is not a function', - fn => { - ['String', 1, {}, Symbol('hello'), true, null, undefined].forEach(el => +describe.each([['beforeEach'], ['beforeAll'], ['afterEach'], ['afterAll']])( + '%s hooks error throwing', + fn => { + test.each([ + ['String'], + [1], + [[]], + [{}], + [Symbol('hello')], + [true], + [null], + [undefined], + ])( + `${fn} throws an error when %p is provided as a first argument to it`, + el => { expect(() => { circus[fn](el); }).toThrowError( 'Invalid first argument. It must be a callback function.', - ), - ); - }, - ); -}); + ); + }, + ); + }, +); diff --git a/packages/jest-jasmine2/src/__tests__/hooks_error.test.js b/packages/jest-jasmine2/src/__tests__/hooks_error.test.js index 50a4e3f277ed..c8bdf0f4908b 100644 --- a/packages/jest-jasmine2/src/__tests__/hooks_error.test.js +++ b/packages/jest-jasmine2/src/__tests__/hooks_error.test.js @@ -9,17 +9,27 @@ 'use strict'; -describe('hooks error throwing', () => { - test.each([['beforeEach'], ['beforeAll'], ['afterEach'], ['afterAll']])( - '%s throws an error when the first argument is not a function', - fn => { - ['String', 1, {}, Symbol('hello'), true, null, undefined].forEach(el => +describe.each([['beforeEach'], ['beforeAll'], ['afterEach'], ['afterAll']])( + '%s hooks error throwing', + fn => { + test.each([ + ['String'], + [1], + [[]], + [{}], + [Symbol('hello')], + [true], + [null], + [undefined], + ])( + `${fn} throws an error when %p is provided as a first argument to it`, + el => { expect(() => { global[fn](el); }).toThrowError( 'Invalid first argument. It must be a callback function.', - ), - ); - }, - ); -}); + ); + }, + ); + }, +);