Skip to content

Conversation

@asirvadAbrahamVarghese
Copy link
Contributor

Enhanced the selectAccordionItem command to wait for list items to load prior to initiating the lookup
This issue was noticed as part of #8916 where it failed to find li nodes when there is a delay:
image
Fix: re-query the dom when no li nodes are found initially:
image

@miq-bot add-label cypress
@miq-bot add-label refactoring
@miq-bot assign @jrafanie

cy.get('div.panel-collapse.collapse.in').then((freshAccordion) => {
cy.wrap(freshAccordion)
.find('li.list-group-item')
.should('have.length.greaterThan', 0)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This causes Cypress to retry the .find() command until this assertion passes for a minimum time (4Sec here)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a question, in addition to the .find, does the freshAccordion get refreshed if the list is not yet populated with more than 0 entries?

});

cy.get('div.panel-collapse.collapse.in').then((freshAccordion) => {
cy.wrap(freshAccordion)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Even though freshAccordion was captured once, when we do cy.wrap(freshAccordion).find(), jQuery's .find() method queries the current state of the DOM tree under that element, not a snapshot because jQuery maintains a reference to the DOM elements.

Note: We could also query the whole accordion repeatedly here, but since this approach seems to work, let’s stick with it for now

@jrafanie
Copy link
Member

Wow, this is awesome! Great find! I saw this on other screens doing some of the conversions from UI driven test setup to using factories and I'm pretty sure this will decrease the sporadic failures in many tests. 👍

Copy link
Member

@jrafanie jrafanie left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is great, merging. I had a question but it's not more for my education than anything else.

@jrafanie jrafanie merged commit 06625de into ManageIQ:master Nov 12, 2025
19 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants