testing-library / cypress-testing-library

🐅 Simple and complete custom Cypress commands and utilities that encourage good testing practices.
http://npm.im/@testing-library/cypress
MIT License
1.8k stars 153 forks source link

`cy.findByText(...).should('not.exist')` returns true for non-selected dropdown option #146

Closed vfonic closed 3 years ago

vfonic commented 4 years ago

This issue (in a way) builds on top of @coryhouse's issue #135 cy.findByText always returns true with should('not.exist')

I've forked the repo and added the new failing test to master: https://github.com/vfonic/cy-doesnt-exist-bug

Repro steps: clone, npm i, npm run cy

You can also see the commit diff: https://github.com/vfonic/cy-doesnt-exist-bug/commit/7cf22ca3189b831010e3c306a0475e8c5da0efc1

Ok, I just made one more commit: https://github.com/coryhouse/cy-doesnt-exist-bug/compare/master...vfonic:master

(There's "Learn React" a tag below that gets correctly selected when the above elements are missing.)

Basically, findByText returns the first DOM element by that text. should('not.exist') returns true ("it doesn't exist") if the element is hidden or non-selected dropdown option.

I'm not sure if this is cypress-testing-library issue or cypress/chai issue? It just sounds misleading, especially for the second case (when findByText finds select option)

vfonic commented 4 years ago

The solution we found to be working well is:

cy.findAllByText(...).should('not.exist')
kentcdodds commented 4 years ago

I'm thinking this is probably a cypress/chai issue because it appears that Cypress Testing Library is getting the DOM nodes for you properly. The assertion is cypress's domain.

For that reason, I'll close this.

vfonic commented 4 years ago

The difference is between:

cy.findByText(...).should('not.exist')

and

cy.findAllByText(...).should('not.exist')

Just wanted to point that out. Perhaps the issue is within this library after all?

kentcdodds commented 4 years ago

Hmmm... Maybe. I'm afraid I don't completely understand the issue and I haven't had time to dig in. I'll re-open this for anyone who wants to take a crack at it though.

davidseow commented 3 years ago

Possibly the reason why this is behaving the way it did.

*By* (in this case findByText), throws an error when there are more than one element. See: link

Error thrown is then captured and an empty jQuery NodeList is returned causing assertion to be true when it should not. See: link

At a glance documentation seems to suggest that the first matching node is returned, this is not the case based on above. Would it be clearer if documentation is updated instead?

From

getBy* queries return the first matching node for a query, and throw an error if no elements match or if more than one match is found (use getAllBy instead).

To

getBy* queries return the matching node for a query, and throw an error if no elements match or if more than one match is found (use getAllBy instead).
kentcdodds commented 3 years ago

Ah, yes that would be a good change to the docs 👍

davidseow commented 3 years ago

Thanks, raising a PR now

davidseow commented 3 years ago

https://github.com/testing-library/testing-library-docs/pull/728