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

contains doesn't work with a selector that contains a comma #2407

Closed
xaviershay opened this issue Aug 28, 2018 · 8 comments · Fixed by #4077
Closed

contains doesn't work with a selector that contains a comma #2407

xaviershay opened this issue Aug 28, 2018 · 8 comments · Fixed by #4077
Assignees
Labels
pkg/driver This is due to an issue in the packages/driver directory type: unexpected behavior User expected result, but got another

Comments

@xaviershay
Copy link

xaviershay commented Aug 28, 2018

Current behavior:

I expect the second line to have equivalent behaviour to the first:

  cy.get('body').find('a,button').contains("Search").click();
  cy.contains('a,button', "Search").click();

Instead, it finds the first a and ignores the value. It appears the , is causing issues.

I'm very new to Cypress, so perhaps this expectation is unfounded.

Versions

3.1.0, Chrome 68, Ubuntu 18

@jennifer-shehane
Copy link
Member

So what happens under the hood when you pass in a 'selector' to cy.contains() is that it effectively transforms that into a check of selector:contains(text), so this would end up being "a,button:contains('Search')" and then we do some extra logic to only return the 'first deepest element' from those results.

The current implementation of cy.contains() is not really expecting comma separated values to be passed as the selector - cause you likely want it to be interpreted more like "a:contains('Search'), button:contains("Search"), which is not what it's doing.

@xaviershay
Copy link
Author

Thanks for the explanation! That makes sense.

We're still considering it a bug, yes?

@jennifer-shehane jennifer-shehane added stage: needs investigating Someone from Cypress needs to look at this type: unexpected behavior User expected result, but got another labels Aug 28, 2018
@jennifer-shehane
Copy link
Member

We will have to decide whether we want to change the implementation to support this.

@xeger
Copy link

xeger commented Apr 29, 2019

Supporting this would be very useful to me, because it handles the very common case where the test author sees a link, but it's actually implemented as a link-colored button due to react-router and bootstrap best practices. In that case, with a simple command, I can find either thing:

cy.contains('a,button.btn-link', 'Click Me').click()

It's not the end of the world for the test author to inspect the element, but if I want to write a cucumber step such as I click the {string} link to handle either type of link, it imposes a burden on me to use some synchronous find and jQuery .text() calls to find the right thing.

If you don't change the implementation, then my vote would be to fail the step if the selector contains a , and/or to document the quirky behavior fact. Better to break than to do something misleading.

(There are other cases such as cy.contains('h1,h2,h3,h4,h5,h6', 'Some Heading') that might be handy too...)

@brian-mann
Copy link
Member

We can probably just split on the , and then map that into multiple :contains(). Likely is easy to do unless I'm missing something.

@jennifer-shehane maybe you can take a shot at it and submit a PR

@cypress-bot cypress-bot bot added stage: ready for work The issue is reproducible and in scope and removed stage: needs investigating Someone from Cypress needs to look at this labels Apr 30, 2019
@jennifer-shehane jennifer-shehane added pkg/driver This is due to an issue in the packages/driver directory difficulty: 1️⃣ labels Apr 30, 2019
@jennifer-shehane jennifer-shehane self-assigned this Apr 30, 2019
@cypress-bot cypress-bot bot added stage: work in progress stage: needs review The PR code is done & tested, needs review and removed stage: ready for work The issue is reproducible and in scope stage: work in progress stage: needs review The PR code is done & tested, needs review labels Apr 30, 2019
@cypress-bot cypress-bot bot added stage: pending release and removed stage: needs review The PR code is done & tested, needs review labels May 2, 2019
@cypress-bot
Copy link
Contributor

cypress-bot bot commented May 2, 2019

The code for this is done in cypress-io/cypress#4077, but has yet to be released.
We'll update this issue and reference the changelog when it's released.

@xeger
Copy link

xeger commented May 2, 2019

Thank you @jennifer-shehane and @brian-mann for prioritizing a fix for this issue; it'll simplify my test cases significantly.

@cypress-bot
Copy link
Contributor

cypress-bot bot commented May 17, 2019

Released in 3.3.0.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pkg/driver This is due to an issue in the packages/driver directory type: unexpected behavior User expected result, but got another
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants