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

await-async-utils false positive on chained cy.wait().its().then() #568

Closed
ThomasGHenry opened this issue Apr 6, 2022 · 8 comments
Closed
Assignees
Labels
awaiting response Waiting for a reply from the OP

Comments

@ThomasGHenry
Copy link

Plugin version

v5.1.0

ESLint version

v8.12.0

Node.js version

16.14.0

npm/yarn version

npm 8.3.1

Operating system

macOS Big Sur 11.6

Bug description

cy.wait getting picked up by testing-library/await-async-utils on a line like...

    cy.wait(`@${label}`, { timeout: 1000 * 60 * 3 })
      .its('status')
      .then(status => expect(status).to.be.oneOf([200, 304]));

The rule documentation says chaining is ok. I assume it's the its that's complicating things.

Thanks!

Steps to reproduce

Got to get back to work. I hope this helps. Thanks!

Error output/screenshots

No response

ESLint configuration

Got to get back to work. I hope this helps. Thanks!

Rule(s) affected

testing-library/await-async-utils

Anything else?

Thank you!

Do you want to submit a pull request to fix this bug?

No

@ThomasGHenry ThomasGHenry added the bug Something isn't working label Apr 6, 2022
@Belco90
Copy link
Member

Belco90 commented Apr 6, 2022

Hey @ThomasGHenry! eslint-plugin-testing-library doesn't support cypress-testing-library (there is not much to report from it actually).

We recommend you to update your ESLint config, so you only run eslint-plugin-testing-library on Testing Library files but not Cypress files. You can find how here: https://github.com/testing-library/eslint-plugin-testing-library#run-the-plugin-only-against-test-files

@Belco90 Belco90 self-assigned this Apr 6, 2022
@Belco90 Belco90 added awaiting response Waiting for a reply from the OP and removed bug Something isn't working labels Apr 6, 2022
@ThomasGHenry
Copy link
Author

ThomasGHenry commented Apr 6, 2022

The more I looked, I was beginning to get that impression. 😅 Thanks for the quick turnaround!

@cburgmer
Copy link

Could we re-open and find a better solution? After all this plugin seems to be very fuzzy at looking for occurrences of findByRole and similar functions used in testing-library. This could lead to errors in other implementations unrelated to testing-library as well.

Currently indeed this is necessary in your .eslintrc:

{
  "plugins": ["testing-library"],
  "overrides": [
    {
      "files": ["myCypressFiles/**/*.js"],
      "rules": {
        "testing-library/prefer-screen-queries": "off",
        "testing-library/await-async-query": "off"
      }
    }
  ]
}

@ThomasGHenry ThomasGHenry reopened this Apr 28, 2022
@Belco90
Copy link
Member

Belco90 commented Apr 28, 2022

Hey @cburgmer! Not sure if I'm missing something else, but the same result can be achieved by just enabling this plugin for jest files as suggested in the README https://github.com/testing-library/eslint-plugin-testing-library#run-the-plugin-only-against-test-files

@cburgmer
Copy link

the same result can be achieved by just enabling this plugin for jest files as suggested in the README

Ah, I see. I'm working on a legacy code base which was setup differently.

I personally feel using overrides is a bit of a workaround, but for now it seems to be the recommended approach here.

@Belco90
Copy link
Member

Belco90 commented Apr 29, 2022

There is another way of doing this, which is putting another .eslintrc file within your tests folder, so you can enable this plugin only for those files. In order to achieve this, you need to keep all your tests within a single folder tho, that's why I always recommend using the overrides approach.

Can we close this ticket then?

@cburgmer
Copy link

Yes please close again. Thanks for clarifying.

If there's one thing I may share, then that no other eslint plugin we are using uses that setup.

My assumption is that the AST parsing of eslint does allow for narrowing down code paths that derive from a 'testing-library' import, but that would still need to be implemented with considerable cost? My thinking is that if the rules can be applied strictly to code calls of testing-library only, the configuration becomes mere trivial?

@Belco90
Copy link
Member

Belco90 commented Apr 29, 2022

@cburgmer that was our original approach, but that would discard reporting related to Testing Library coming from non "testing-library" modules. It's pretty common to re-export Testing Library utils from a custom module (e.g. "test-utils"). The plugin wouldn't be aware of it. That's why we have this Aggressive Reporting in place, leaving it up to you to narrow the scope with ESLint settings or the plugin settings.

You can read more about it here: #198

@Belco90 Belco90 closed this as completed Apr 29, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
awaiting response Waiting for a reply from the OP
Projects
None yet
Development

No branches or pull requests

3 participants