Validate hook functions first argument

[natealcedo]

Summary

This pull request handles the case where the callback function for the jest hooks beforeEach, beforeAll, afterEach, afterAll are 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

[codecov-io]

Codecov Report

Merging #6931 into master will not change coverage.
The diff coverage is 11.11%.

@@           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

Impacted Files	Coverage Δ
packages/jest-jasmine2/src/jasmine/Env.js	0% <ø> (ø)	⬆️
...ackages/jest-jasmine2/src/jasmine/jasmine_light.js	0% <0%> (ø)	⬆️
packages/jest-circus/src/index.js	66.66% <100%> (ø)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 24c173e...5c45f71. Read the comment docs.

[SimenB]

can change this to describe.each, then have the inner for-loop be test.each

[natealcedo]

@SimenB, I've pushed in a set of changes which includes adding types for describe.each and test.each. Let me know what you think!

[SimenB]

Perfect, thank you!

[SimenB]

@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(() => {

[natealcedo]

Hey, sorry could you elaborate on what you meant by upstreaming it? Do you mean pulling in the changes from here to flow-typed? This is the first project where I've used flow so I'm abit hazy on it. But if that's the case, I'm game on doing the work.

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 describe.each.

[SimenB]

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 🙂

[natealcedo]

In that case, let me update the typings locally here because table: Array<Array<mixed> | mixed> looks more accurate than table: Array<Array<mixed>>.

Here is what I plan on doing.

1. Update typings in this project and do a PR in this repo.
2. Upstream the changes to flow-typed

Is that fine?

[SimenB]

sounds good!

[github-actions]

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.
Please note this issue tracker is not a help forum. We recommend using StackOverflow or our discord channel for questions.