From 01aef2f2836bbc230a64ecf7109bdf10c1601146 Mon Sep 17 00:00:00 2001 From: Matt Phillips Date: Tue, 14 Aug 2018 17:31:49 +0100 Subject: [PATCH 1/3] Add flag to prevent done callback being supplied to describe --- packages/jest-circus/src/index.js | 6 +- .../jest-each/src/__tests__/array.test.js | 103 +++++++++------- .../jest-each/src/__tests__/template.test.js | 115 ++++++++++-------- packages/jest-each/src/bind.js | 27 ++-- packages/jest-each/src/index.js | 10 +- packages/jest-jasmine2/src/each.js | 15 ++- 6 files changed, 166 insertions(+), 110 deletions(-) diff --git a/packages/jest-circus/src/index.js b/packages/jest-circus/src/index.js index 91e315b1241d..dd227af3a515 100644 --- a/packages/jest-circus/src/index.js +++ b/packages/jest-circus/src/index.js @@ -121,9 +121,9 @@ test.each = bindEach(test); test.only.each = bindEach(test.only); test.skip.each = bindEach(test.skip); -describe.each = bindEach(describe); -describe.only.each = bindEach(describe.only); -describe.skip.each = bindEach(describe.skip); +describe.each = bindEach(describe, false); +describe.only.each = bindEach(describe.only, false); +describe.skip.each = bindEach(describe.skip, false); module.exports = { afterAll, diff --git a/packages/jest-each/src/__tests__/array.test.js b/packages/jest-each/src/__tests__/array.test.js index 65a41f627d59..1077ffecd629 100644 --- a/packages/jest-each/src/__tests__/array.test.js +++ b/packages/jest-each/src/__tests__/array.test.js @@ -15,6 +15,26 @@ const expectFunction = expect.any(Function); const get = (object, lensPath) => lensPath.reduce((acc, key) => acc[key], object); +const getGlobalTestMocks = () => { + const globals = { + describe: jest.fn(), + fdescribe: jest.fn(), + fit: jest.fn(), + it: jest.fn(), + test: jest.fn(), + xdescribe: jest.fn(), + xit: jest.fn(), + xtest: jest.fn(), + }; + globals.test.only = jest.fn(); + globals.test.skip = jest.fn(); + globals.it.only = jest.fn(); + globals.it.skip = jest.fn(); + globals.describe.only = jest.fn(); + globals.describe.skip = jest.fn(); + return globals; +}; + describe('jest-each', () => { [ ['test'], @@ -27,20 +47,6 @@ describe('jest-each', () => { ['describe', 'only'], ].forEach(keyPath => { describe(`.${keyPath.join('.')}`, () => { - const getGlobalTestMocks = () => { - const globals = { - describe: jest.fn(), - fdescribe: jest.fn(), - fit: jest.fn(), - it: jest.fn(), - test: jest.fn(), - }; - globals.test.only = jest.fn(); - globals.it.only = jest.fn(); - globals.describe.only = jest.fn(); - return globals; - }; - test('calls global with given title', () => { const globalTestMocks = getGlobalTestMocks(); const eachObject = each.withGlobal(globalTestMocks)([[]]); @@ -237,18 +243,6 @@ describe('jest-each', () => { expect(testCallBack).toHaveBeenCalledWith('joe', 'bloggs'); }); - test('calls global with async done when cb function has more args than params of given test row', () => { - const globalTestMocks = getGlobalTestMocks(); - const eachObject = each.withGlobal(globalTestMocks)([['hello']]); - - const testFunction = get(eachObject, keyPath); - testFunction('expected string', (hello, done) => { - expect(hello).toBe('hello'); - expect(done).toBe('DONE'); - }); - get(globalTestMocks, keyPath).mock.calls[0][1]('DONE'); - }); - test('calls global with given timeout', () => { const globalTestMocks = getGlobalTestMocks(); const eachObject = each.withGlobal(globalTestMocks)([['hello']]); @@ -265,6 +259,45 @@ describe('jest-each', () => { }); }); + describe('done callback', () => { + test.each([ + [['test']], + [['test', 'only']], + [['it']], + [['fit']], + [['it', 'only']], + ])( + 'calls %O with done when cb function has more args than params of given test row', + keyPath => { + const globalTestMocks = getGlobalTestMocks(); + const eachObject = each.withGlobal(globalTestMocks)([['hello']]); + + const testFunction = get(eachObject, keyPath); + testFunction('expected string', (hello, done) => { + expect(hello).toBe('hello'); + expect(done).toBe('DONE'); + }); + get(globalTestMocks, keyPath).mock.calls[0][1]('DONE'); + }, + ); + + test.each([[['describe']], [['fdescribe']], [['describe', 'only']]])( + 'does not call %O with done when test function has more args than params of given test row', + keyPath => { + const globalTestMocks = getGlobalTestMocks(); + const eachObject = each.withGlobal(globalTestMocks)([['hello']]); + + const testFunction = get(eachObject, keyPath); + testFunction('expected string', function(hello, done) { + expect(hello).toBe('hello'); + expect(arguments.length).toBe(1); + expect(done).toBe(undefined); + }); + get(globalTestMocks, keyPath).mock.calls[0][1]('DONE'); + }, + ); + }); + [ ['xtest'], ['test', 'skip'], @@ -274,24 +307,6 @@ describe('jest-each', () => { ['describe', 'skip'], ].forEach(keyPath => { describe(`.${keyPath.join('.')}`, () => { - const getGlobalTestMocks = () => { - const globals = { - describe: { - skip: jest.fn(), - }, - it: { - skip: jest.fn(), - }, - test: { - skip: jest.fn(), - }, - xdescribe: jest.fn(), - xit: jest.fn(), - xtest: jest.fn(), - }; - return globals; - }; - test('calls global with given title', () => { const globalTestMocks = getGlobalTestMocks(); const eachObject = each.withGlobal(globalTestMocks)([[]]); diff --git a/packages/jest-each/src/__tests__/template.test.js b/packages/jest-each/src/__tests__/template.test.js index 7ec6b44dfe3e..30f368ef54ce 100644 --- a/packages/jest-each/src/__tests__/template.test.js +++ b/packages/jest-each/src/__tests__/template.test.js @@ -14,6 +14,26 @@ const expectFunction = expect.any(Function); const get = (object, lensPath) => lensPath.reduce((acc, key) => acc[key], object); +const getGlobalTestMocks = () => { + const globals = { + describe: jest.fn(), + fdescribe: jest.fn(), + fit: jest.fn(), + it: jest.fn(), + test: jest.fn(), + xdescribe: jest.fn(), + xit: jest.fn(), + xtest: jest.fn(), + }; + globals.test.only = jest.fn(); + globals.test.skip = jest.fn(); + globals.it.only = jest.fn(); + globals.it.skip = jest.fn(); + globals.describe.only = jest.fn(); + globals.describe.skip = jest.fn(); + return globals; +}; + describe('jest-each', () => { [ ['test'], @@ -26,20 +46,6 @@ describe('jest-each', () => { ['describe', 'only'], ].forEach(keyPath => { describe(`.${keyPath.join('.')}`, () => { - const getGlobalTestMocks = () => { - const globals = { - describe: jest.fn(), - fdescribe: jest.fn(), - fit: jest.fn(), - it: jest.fn(), - test: jest.fn(), - }; - globals.test.only = jest.fn(); - globals.it.only = jest.fn(); - globals.describe.only = jest.fn(); - return globals; - }; - test('throws error when there are fewer arguments than headings when given one row', () => { const globalTestMocks = getGlobalTestMocks(); const eachObject = each.withGlobal(globalTestMocks)` @@ -228,22 +234,6 @@ describe('jest-each', () => { expect(testCallBack).toHaveBeenCalledWith({a: 1, b: 1, expected: 2}); }); - test('calls global with async done when cb function has more than one argument', () => { - const globalTestMocks = getGlobalTestMocks(); - const eachObject = each.withGlobal(globalTestMocks)` - a | b | expected - ${0} | ${1} | ${1} - `; - const testFunction = get(eachObject, keyPath); - testFunction('expected string', ({a, b, expected}, done) => { - expect(a).toBe(0); - expect(b).toBe(1); - expect(expected).toBe(1); - expect(done).toBe('DONE'); - }); - get(globalTestMocks, keyPath).mock.calls[0][1]('DONE'); - }); - test('calls global with given timeout', () => { const globalTestMocks = getGlobalTestMocks(); const eachObject = each.withGlobal(globalTestMocks)` @@ -263,6 +253,53 @@ describe('jest-each', () => { }); }); + describe('done callback', () => { + test.each([ + [['test']], + [['test', 'only']], + [['it']], + [['fit']], + [['it', 'only']], + ])( + 'calls %O with done when cb function has more args than params of given test row', + keyPath => { + const globalTestMocks = getGlobalTestMocks(); + const eachObject = each.withGlobal(globalTestMocks)` + a | b | expected + ${0} | ${1} | ${1} + `; + const testFunction = get(eachObject, keyPath); + testFunction('expected string', ({a, b, expected}, done) => { + expect(a).toBe(0); + expect(b).toBe(1); + expect(expected).toBe(1); + expect(done).toBe('DONE'); + }); + get(globalTestMocks, keyPath).mock.calls[0][1]('DONE'); + }, + ); + + test.each([[['describe']], [['fdescribe']], [['describe', 'only']]])( + 'does not call %O with done when test function has more args than params of given test row', + keyPath => { + const globalTestMocks = getGlobalTestMocks(); + const eachObject = each.withGlobal(globalTestMocks)` + a | b | expected + ${0} | ${1} | ${1} + `; + const testFunction = get(eachObject, keyPath); + testFunction('expected string', function({a, b, expected}, done) { + expect(a).toBe(0); + expect(b).toBe(1); + expect(expected).toBe(1); + expect(done).toBe(undefined); + expect(arguments.length).toBe(1); + }); + get(globalTestMocks, keyPath).mock.calls[0][1]('DONE'); + }, + ); + }); + [ ['xtest'], ['test', 'skip'], @@ -272,24 +309,6 @@ describe('jest-each', () => { ['describe', 'skip'], ].forEach(keyPath => { describe(`.${keyPath.join('.')}`, () => { - const getGlobalTestMocks = () => { - const globals = { - describe: { - skip: jest.fn(), - }, - it: { - skip: jest.fn(), - }, - test: { - skip: jest.fn(), - }, - xdescribe: jest.fn(), - xit: jest.fn(), - xtest: jest.fn(), - }; - return globals; - }; - test('calls global with given title', () => { const globalTestMocks = getGlobalTestMocks(); const eachObject = each.withGlobal(globalTestMocks)` diff --git a/packages/jest-each/src/bind.js b/packages/jest-each/src/bind.js index df1221b38065..e92bc0aad5a8 100644 --- a/packages/jest-each/src/bind.js +++ b/packages/jest-each/src/bind.js @@ -23,14 +23,18 @@ const SUPPORTED_PLACEHOLDERS = /%[sdifjoOp%]/g; const PRETTY_PLACEHOLDER = '%p'; const INDEX_PLACEHOLDER = '%#'; -export default (cb: Function) => (...args: any) => +export default (cb: Function, supportsDone: boolean = true) => (...args: any) => function eachBind(title: string, test: Function, timeout: number): void { if (args.length === 1) { const table: Table = args[0].every(Array.isArray) ? args[0] : args[0].map(entry => [entry]); return table.forEach((row, i) => - cb(arrayFormat(title, i, ...row), applyRestParams(row, test), timeout), + cb( + arrayFormat(title, i, ...row), + applyRestParams(supportsDone, row, test), + timeout, + ), ); } @@ -66,7 +70,11 @@ export default (cb: Function) => (...args: any) => } return table.forEach(row => - cb(interpolate(title, row), applyObjectParams(row, test), timeout), + cb( + interpolate(title, row), + applyObjectParams(supportsDone, row, test), + timeout, + ), ); }; @@ -107,8 +115,13 @@ const arrayFormat = (title, rowIndex, ...args) => { ); }; -const applyRestParams = (params: Array, test: Function) => { - if (params.length < test.length) return done => test(...params, done); +const applyRestParams = ( + supportsDone: boolean, + params: Array, + test: Function, +) => { + if (supportsDone && params.length < test.length) + return done => test(...params, done); return () => test(...params); }; @@ -144,8 +157,8 @@ const interpolate = (title: string, data: any) => .reduce(getMatchingKeyPaths(title), []) // aka flatMap .reduce(replaceKeyPathWithValue(data), title); -const applyObjectParams = (obj: any, test: Function) => { - if (test.length > 1) return done => test(obj, done); +const applyObjectParams = (supportsDone: boolean, obj: any, test: Function) => { + if (supportsDone && test.length > 1) return done => test(obj, done); return () => test(obj); }; diff --git a/packages/jest-each/src/index.js b/packages/jest-each/src/index.js index 333651aae902..3b9f18a3fde9 100644 --- a/packages/jest-each/src/index.js +++ b/packages/jest-each/src/index.js @@ -36,11 +36,11 @@ const install = (g: GlobalCallbacks, ...args: Array) => { const xtest = bind(g.xtest)(...args); const describe = (title: string, suite: Function, timeout: number) => - bind(g.describe)(...args)(title, suite, timeout); - describe.skip = bind(g.describe.skip)(...args); - describe.only = bind(g.describe.only)(...args); - const fdescribe = bind(g.fdescribe)(...args); - const xdescribe = bind(g.xdescribe)(...args); + bind(g.describe, false)(...args)(title, suite, timeout); + describe.skip = bind(g.describe.skip, false)(...args); + describe.only = bind(g.describe.only, false)(...args); + const fdescribe = bind(g.fdescribe, false)(...args); + const xdescribe = bind(g.xdescribe, false)(...args); return {describe, fdescribe, fit, it, test, xdescribe, xit, xtest}; }; diff --git a/packages/jest-jasmine2/src/each.js b/packages/jest-jasmine2/src/each.js index 282af6d1faf7..d5237f0615c5 100644 --- a/packages/jest-jasmine2/src/each.js +++ b/packages/jest-jasmine2/src/each.js @@ -15,7 +15,16 @@ export default (environment: Environment): void => { environment.global.it.each = bindEach(environment.global.it); environment.global.fit.each = bindEach(environment.global.fit); environment.global.xit.each = bindEach(environment.global.xit); - environment.global.describe.each = bindEach(environment.global.describe); - environment.global.xdescribe.each = bindEach(environment.global.xdescribe); - environment.global.fdescribe.each = bindEach(environment.global.fdescribe); + environment.global.describe.each = bindEach( + environment.global.describe, + false, + ); + environment.global.xdescribe.each = bindEach( + environment.global.xdescribe, + false, + ); + environment.global.fdescribe.each = bindEach( + environment.global.fdescribe, + false, + ); }; From 0a81b2f50a599541aa84f9e46d4b9a192c09929d Mon Sep 17 00:00:00 2001 From: Matt Phillips Date: Tue, 14 Aug 2018 17:39:29 +0100 Subject: [PATCH 2/3] Add changelog entry --- CHANGELOG.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 74f5f4b96db2..320e40352fd5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,10 @@ - `[docs]` Add custom toMatchSnapshot matcher docs ([#6837](https://github.com/facebook/jest/pull/6837)) +### Fixes + +- `[jest-each`] Prevent done callback being supplied to describe ([#6843](https://github.com/facebook/jest/pull/6843)) + ## 23.5.0 ### Features From 9979f0e119c9834ae18452e92fd43001bb6913f2 Mon Sep 17 00:00:00 2001 From: Matt Phillips Date: Tue, 14 Aug 2018 18:05:28 +0100 Subject: [PATCH 3/3] Refactor with thymikee linter ;) --- packages/jest-each/src/bind.js | 17 ++++++----------- 1 file changed, 6 insertions(+), 11 deletions(-) diff --git a/packages/jest-each/src/bind.js b/packages/jest-each/src/bind.js index e92bc0aad5a8..fe4565b4c8c9 100644 --- a/packages/jest-each/src/bind.js +++ b/packages/jest-each/src/bind.js @@ -119,12 +119,10 @@ const applyRestParams = ( supportsDone: boolean, params: Array, test: Function, -) => { - if (supportsDone && params.length < test.length) - return done => test(...params, done); - - return () => test(...params); -}; +) => + supportsDone && params.length < test.length + ? done => test(...params, done) + : () => test(...params); const getHeadingKeys = (headings: string): Array => headings.replace(/\s/g, '').split('|'); @@ -157,11 +155,8 @@ const interpolate = (title: string, data: any) => .reduce(getMatchingKeyPaths(title), []) // aka flatMap .reduce(replaceKeyPathWithValue(data), title); -const applyObjectParams = (supportsDone: boolean, obj: any, test: Function) => { - if (supportsDone && test.length > 1) return done => test(obj, done); - - return () => test(obj); -}; +const applyObjectParams = (supportsDone: boolean, obj: any, test: Function) => + supportsDone && test.length > 1 ? done => test(obj, done) : () => test(obj); const pluralize = (word: string, count: number) => word + (count === 1 ? '' : 's');