Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

jest/valid-describe false positive for descibe.each variant #334

Closed
topaxi opened this issue Jul 22, 2019 · 15 comments
Closed

jest/valid-describe false positive for descibe.each variant #334

topaxi opened this issue Jul 22, 2019 · 15 comments

Comments

@topaxi
Copy link

topaxi commented Jul 22, 2019

The rule jest/valid-describe errors for the describe.each variant:

describe.each([...data])('Something', data => {
  // ...
})

leads to:

  3:15  error  First argument must be name                    jest/valid-describe
  3:15  error  Describe requires name and callback arguments  jest/valid-describe
@joshuadiablito
Copy link

Also have the same error, it's valid: https://jestjs.io/docs/en/api#describeeachtable-name-fn-timeout

@G-Rath
Copy link
Collaborator

G-Rath commented Jul 22, 2019

I suspect I know what's causing this, but I can't reproduce the error - I managed to have it show up once in my IDE, but it didn't have a stacktrace and went away after a second.

I hate to be a pain, but if you could provide a small repo that reproduced the error, or even a stacktrace, that'd be a great help!

@SimenB
Copy link
Member

SimenB commented Jul 22, 2019

Pasting in the snippet in the OP fails for me.

Diff with failing test:

diff --git i/src/rules/__tests__/valid-describe.test.ts w/src/rules/__tests__/valid-describe.test.ts
index adfbad5..5f63b49 100644
--- i/src/rules/__tests__/valid-describe.test.ts
+++ w/src/rules/__tests__/valid-describe.test.ts
@@ -37,6 +37,11 @@ ruleTester.run('valid-describe', rule, {
       test('bar', () => {})
     )
     `,
+    `
+    describe.each([...data])('Something', data => {
+      // ...
+    })
+    `,
   ],
   invalid: [
     {

EDIT: And yes, reverting #317 "fixes" it. Fixes in that describe.each is not recognized as a describe, so it's not linted at all 😅

@G-Rath
Copy link
Collaborator

G-Rath commented Jul 22, 2019

eeekkkk but totally what I expected - I'll try and wipe up a fix asap.

@SimenB
Copy link
Member

SimenB commented Jul 22, 2019

The real fix is to handle describe.each. The quick fix is to not recognize it as a describe - each is pretty different, so I think really fixing it is a bit more work than needed

@G-Rath
Copy link
Collaborator

G-Rath commented Jul 22, 2019

One step ahead of you ;)

I'm going to make a quick fix to ignore each, and open a new issue to support each.

The rule readme actually excludes each:

The following describe function aliases are also validated:

describe
describe.only
describe.skip
fdescribe
xdescribe

This was referenced Jul 22, 2019
@G-Rath
Copy link
Collaborator

G-Rath commented Jul 22, 2019

Sorry my quick fix ended up not being so quick as I got distracted finishing off the conversion for prefer-todo 😂

@SimenB SimenB closed this as completed in ed2a0f6 Jul 22, 2019
@SimenB
Copy link
Member

SimenB commented Jul 22, 2019

🎉 This issue has been resolved in version 22.13.1 🎉

The release is available on:

Your semantic-release bot 📦🚀

@topaxi
Copy link
Author

topaxi commented Jul 22, 2019

Wow! Thanks for the quick fix! <3

@topaxi
Copy link
Author

topaxi commented Jul 22, 2019

For some reason, this now triggers on completely unrelated code:

import { hasOwnProperty } from './has-own-property'

export function size(obj: object): number {
  let size = 0

  for (let key in obj) {
    if (hasOwnProperty(obj, key)) {
      size++
    }
  }

  return size
}
/home/travis/build/topaxi/learning-cs/utils/object/size.ts
  7:24  error  First argument must be name       jest/valid-describe
  7:24  error  Second argument must be function  jest/valid-describe

@SimenB
Copy link
Member

SimenB commented Jul 22, 2019

what in the world...

failing test:

diff --git i/src/rules/__tests__/valid-describe.test.ts w/src/rules/__tests__/valid-describe.test.ts
index b5e2cd7..66e299e 100644
--- i/src/rules/__tests__/valid-describe.test.ts
+++ w/src/rules/__tests__/valid-describe.test.ts
@@ -38,6 +38,10 @@ ruleTester.run('valid-describe', rule, {
       test('bar', () => {})
     )
     `,
+    `
+    if (hasOwnProperty(obj, key)) {
+    }
+    `,
   ],
   invalid: [
     {

@SimenB SimenB reopened this Jul 22, 2019
@SimenB
Copy link
Member

SimenB commented Jul 22, 2019

Oh, it's the in DescribeAlias things. It has a property called hasOwnProperty

@SimenB
Copy link
Member

SimenB commented Jul 22, 2019

I'm trying to push a fix, but GH is having issues... https://www.githubstatus.com/

@SimenB SimenB closed this as completed in b27c80d Jul 22, 2019
@SimenB
Copy link
Member

SimenB commented Jul 22, 2019

🎉 This issue has been resolved in version 22.13.4 🎉

The release is available on:

Your semantic-release bot 📦🚀

@topaxi
Copy link
Author

topaxi commented Jul 23, 2019

Thanks again for the quick fixes ❤️

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants