Skip to content

Conversation

@asirvadAbrahamVarghese
Copy link
Contributor

PR to Add selectTableRowsByText command for miq data table row selection:
image

  • Text-based row selection: Select table rows by matching text content in any cell
  • Multiple item selection: Provide an array of text values to select multiple rows at once
  • Error handling: Throws an error immediately if any specified text is not found in the table, error messages include the specific text that wasn't found, making debugging easier
  • Skip already selected rows: Automatically skips rows that are already selected

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

textArray.forEach((textToFind) => {
let textFoundInTable = false;

cy.get('.miq-data-table table tbody tr')
Copy link
Member

Choose a reason for hiding this comment

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

Can we swap the each loops so we do one cy.get upfront to get all the rows and then iterate on the rows looking for any matches in the textArray.

Copy link
Member

Choose a reason for hiding this comment

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

in other words,

cy.get('.miq-data-table table tbody tr').then(rows => {
  // Process each text item
  textArray.forEach(textToFind => {
  ...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I could be wrong here, but my point is:
If this is the table we have:

Col1 Col2 Col3
sample-text1 False 123
sample-text2 True 456
sample-text3 False 789

and if our textArray is ['sample-text5', 'sample-text1']

With current State:

textArray.forEach((textToFind) => {
  cy.get('.miq-data-table table tbody tr').each(...)
  • For each text item, we query the DOM and get all rows
  • If a text item is not found, we throw an error immediately after scanning all rows for that specific text
  • We don't need to scan for subsequent text items if one fails
  • So Best case: O(R) if the first text item isn't found (R -> number of rows) & Worst case: O(T * R) if all text items are present (T -> number of text items, R -> number of rows)

With swapped State:

 cy.get('.miq-data-table table tbody tr').then(rows => {
  textArray.forEach(textToFind => {
  • We query the DOM once and get all rows
  • For each row, we check all text items
  • We can only determine if a text item is missing after going through all text items and scanning all rows
  • Always O(T * R)(T -> number of text items, R -> number of rows) regardless of whether items are found or not

Now I have updated to query the table rows only once and stores them for reuse but keeping the same looping order

  cy.get('.miq-data-table table tbody tr').then(($rows) => {
    const rows = Array.from($rows);
    textArray.forEach((textToFind, textItemIndex) => {
      rows.some((row) => {

One edge case is when the same text exists in multiple table rows, if our target is in the second row, but the selector would be grabbing the first one, that could cause issues. We'll need to merge the text with the row index in textArray when it's passed in as a parameter, but since the index isn’t predictable during tests, that’s tricky.

Col1 Col2 Col3
sample-text1 False 123
sample-text2 sample-text1 456
sample-text3 False 789

Copy link
Member

Choose a reason for hiding this comment

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

Now I have updated to query the table rows only once and stores them for reuse but keeping the same looping order

Thanks, I didn't say it directly but that's ultimately what I commenting on. I don't know the details but cy.get is likely orders of magnitude slower than doing in memory loops of text arrays looking for text. cy.get also needs the DOM fully loaded and has timing issues that should be non-existant in the in memory looping of the text array. I was hoping to minimize the cy.get calls and I think you did that so 👍

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.

LGTM

@jrafanie jrafanie merged commit 696d6cd into ManageIQ:master Oct 20, 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