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
Closed

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

xaviershay opened this issue Aug 28, 2018 · 8 comments

Comments

@xaviershay
Copy link

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

@jennifer-shehane jennifer-shehane commented Aug 28, 2018

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

@xaviershay xaviershay commented Aug 28, 2018

Thanks for the explanation! That makes sense.

We're still considering it a bug, yes?

@jennifer-shehane
Copy link
Member

@jennifer-shehane jennifer-shehane commented Aug 28, 2018

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

@xeger
Copy link

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

@brian-mann brian-mann commented Apr 30, 2019

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
Copy link

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

@cypress-bot 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
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

4 participants