fix: .contains() properly respects multiple incoming subjects #25035
Conversation
|
Thanks for taking the time to open a PR!
|
BlueWinds
changed the title
| // https://github.com/cypress-io/cypress/issues/25025 | ||
| it('searches multiple subject elements', () => { | ||
| cy.get('ul').contains('li', 'asdf 3') | ||
| }) |
There was a problem hiding this comment.
to ensure this change touched within subject, can we we add something like this as well?
it('searches multiple subject elements in .within', () => {
cy.get('body').within(() => {
cy.get('ul').contains('li', 'asdf 3')
})
})There was a problem hiding this comment.
I believe this sort of case is already well covered inside the .within() tests.
it('scopes additional CONTAINS finders to the subject', () => {
const span = cy.$$('#nested-div span:contains(foo)')
cy.contains('foo').then(($span) => {
expect($span.get(0)).not.to.eq(span.get(0))
})
cy.get('#nested-div').within(() => {
cy.contains('foo').then(($span) => {
expect($span.get(0)).to.eq(span.get(0))
})
})
})
and a couple of other similar tests in within.cy.js I don't think it's valuable to make this test more complicated.
There was a problem hiding this comment.
@BlueWinds wasn't my suggestion for a test this issue? #25019
There was a problem hiding this comment.
That issue has to do with a <form> as the subject or withinSubject - it's incredibly specific to forms + contains, which took me quite a while to figure out (I did not expect HTML elements to have different properties).
let form = $("<form><input /><input /></form>").get(0) // Raw html element
let ul = $('<ul><li /><li /></ul>').get(0) // Raw html element
form[0] // An <input> element
ul[0] // undefined
There was a problem hiding this comment.
I hadn't figured that out when I made this PR, was taking them one at a time. But that other PR has its own test covering that case. .within() isn't necessary to hit either of these two bugs - they're entirely within .contains(), users just happened to run into them using .within() blocks.
There was a problem hiding this comment.
I had a lazy check that was looking for subject[0] as equivelent for "is a jquery object", which worked for every element... except <form>. Today I learned.
There was a problem hiding this comment.
The fact that .contains() hijacks the "withinSubject" property of .get() was pre-existing behavior that I really should have cleaned up / renamed when I moved them to be queries, but didn't think of at the time. It's really more a "container element to query inside" - which we call "withinSubject" because .within() is our "scope commands to a container element" command.
cypress-testing-library calls this a "container", and we probably should too. Some future variable renaming to consider.
|
Test summaryRun details
View run in Cypress Dashboard ➡️ This comment has been generated by cypress-bot as a result of this project's GitHub integration settings. You can manage this integration in this project's settings in the Cypress Dashboard |
User facing changelog
In Cypress 12.0.0, we introduced a regression where when
.contains()receives multiple elements as a subject, it only searched inside the first one. This PR fixes that issue.Additional details
This PR includes a new driver test covering the case. Another reproduction that doesn't rely on any of our fixtures:
Steps to test
How has the user experience changed?
PR Tasks
cypress-documentation?type definitions?