Validate hook functions first argument #6931
Conversation
…s to jasmine_light.js
…ilar to how jasmine_light does it
Codecov Report
@@ Coverage Diff @@
## master #6931 +/- ##
=======================================
Coverage 66.97% 66.97%
=======================================
Files 250 250
Lines 10379 10379
Branches 3 3
=======================================
Hits 6951 6951
Misses 3427 3427
Partials 1 1
Continue to review full report at Codecov.
|
| @@ -15,10 +15,12 @@ describe('hooks error throwing', () => { | |||
| test.each([['beforeEach'], ['beforeAll'], ['afterEach'], ['afterAll']])( | |||
There was a problem hiding this comment.
can change this to describe.each, then have the inner for-loop be test.each
|
@SimenB, I've pushed in a set of changes which includes adding types for |
|
Perfect, thank you! |
|
@natealcedo thoughts on upstreaming the changes in the flow definitions to flow-typed? https://github.com/flow-typed/flow-typed/blob/master/definitions/npm/jest_v23.x.x/flow_v0.39.x-/jest_v23.x.x.js I also have this diff locally: diff --git c/flow-typed/npm/jest_v23.x.x.js w/flow-typed/npm/jest_v23.x.x.js
index e8e487175..abaa092b6 100644
--- c/flow-typed/npm/jest_v23.x.x.js
+++ w/flow-typed/npm/jest_v23.x.x.js
@@ -922,7 +922,7 @@ declare var describe: {
* @param {table} table of Test
*/
each(
- table: Array<Array<mixed>>
+ table: Array<Array<mixed> | mixed>
): (
name: JestTestName,
fn?: (...args: Array<any>) => ?Promise<mixed>
@@ -949,7 +949,7 @@ declare var it: {
* @param {table} table of Test
*/
each(
- table: Array<Array<mixed>>
+ table: Array<Array<mixed> | mixed>
): (
name: JestTestName,
fn?: (...args: Array<any>) => ?Promise<mixed>
@@ -967,7 +967,7 @@ declare var it: {
timeout?: number
): {
each(
- table: Array<Array<mixed>>
+ table: Array<Array<mixed> | mixed>
): (
name: JestTestName,
fn?: (...args: Array<any>) => ?Promise<mixed>
@@ -1003,7 +1003,7 @@ declare var it: {
* @param {table} table of Test
*/
each(
- table: Array<Array<mixed>>
+ table: Array<Array<mixed> | mixed>
): (
name: JestTestName,
fn?: (...args: Array<any>) => ?Promise<mixed>
diff --git c/packages/jest-circus/src/__tests__/hooks_error.test.js w/packages/jest-circus/src/__tests__/hooks_error.test.js
index 6faab9809..73d1d1ffe 100644
--- c/packages/jest-circus/src/__tests__/hooks_error.test.js
+++ w/packages/jest-circus/src/__tests__/hooks_error.test.js
@@ -11,19 +11,10 @@
const circus = require('../index.js');
-describe.each([['beforeEach'], ['beforeAll'], ['afterEach'], ['afterAll']])(
+describe.each(['beforeEach', 'beforeAll', 'afterEach', 'afterAll'])(
'%s hooks error throwing',
fn => {
- test.each([
- ['String'],
- [1],
- [[]],
- [{}],
- [Symbol('hello')],
- [true],
- [null],
- [undefined],
- ])(
+ 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(() => {
diff --git c/packages/jest-jasmine2/src/__tests__/hooks_error.test.js w/packages/jest-jasmine2/src/__tests__/hooks_error.test.js
index c8bdf0f49..1f6e2cc76 100644
--- c/packages/jest-jasmine2/src/__tests__/hooks_error.test.js
+++ w/packages/jest-jasmine2/src/__tests__/hooks_error.test.js
@@ -9,19 +9,10 @@
'use strict';
-describe.each([['beforeEach'], ['beforeAll'], ['afterEach'], ['afterAll']])(
+describe.each(['beforeEach', 'beforeAll', 'afterEach', 'afterAll'])(
'%s hooks error throwing',
fn => {
- test.each([
- ['String'],
- [1],
- [[]],
- [{}],
- [Symbol('hello')],
- [true],
- [null],
- [undefined],
- ])(
+ 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(() => { |
|
Hey, sorry could you elaborate on what you meant by upstreaming it? Do you mean pulling in the changes from here to On the topic of your changes locally, I don't really know what I'm looking at. Asides from the inner nesting of the arguments to |
|
Yes, the flow definitions in this project is just pulled from that repo. So changes we make here should be upstreamed so that others get to use them as well 🙂 My local changes are related to #6351 where we added support for dropping the extra nested arrays. That should also go into the typings 🙂 |
|
In that case, let me update the typings locally here because Here is what I plan on doing.
Is that fine? |
|
sounds good! |
|
This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs. |
Summary
This pull request handles the case where the callback function for the jest hooks
beforeEach, beforeAll, afterEach, afterAllare not functions in issue #6789. This PR also handles every other primitive type. PR #6917 only handles a smaller set of cases whereas this one does.Test plan
I've updated the test suites in both these files.
https://github.com/facebook/jest/blob/master/packages/jest-circus/src/__tests__/hooks_error.test.js
https://github.com/facebook/jest/blob/master/packages/jest-jasmine2/src/__tests__/hooks_error.test.js