Skip to content

Conversation

@GilbertCherrie
Copy link
Member

Fixed the gtl cypress helper functions and updated their documentation. Also refactored rates.cy.js to use these new gtl helper functions.

@GilbertCherrie GilbertCherrie requested a review from a team as a code owner January 12, 2024 19:54
@GilbertCherrie GilbertCherrie changed the title Fixed gtl functions and refactor rates.cy.js Fixed gtl cypress functions and refactor rates.cy.js Jan 12, 2024
* `cy.gtl_no_record` - check that `No data` message is present
* `cy.gtl_click('Name')` - click on a GTL
* `cy.gtlGetTable` - return GTL table
* `cy.gtlGetRows(columns)` - return GTL table row data in an array. columns: Array of ints of columns to read. Example: [1,2,3] will return all row data from columns 1,2 and 3.
Copy link
Member

Choose a reason for hiding this comment

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

Is this index value 0-based or 1-based?

Copy link
Member

Choose a reason for hiding this comment

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

Assuming it's 0-based, I think this might be a nice way to present that:

Suggested change
* `cy.gtlGetRows(columns)` - return GTL table row data in an array. columns: Array of ints of columns to read. Example: [1,2,3] will return all row data from columns 1,2 and 3.
* `cy.gtlGetRows(columns)` - return GTL table row data in an array. columns: Array of 0-based indexes of the columns to read (e.g. [1, 2, 3] will return all row data from columns 1, 2, and 3).

Copy link
Member

Choose a reason for hiding this comment

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

I also noticed getRows can be called without the parameter, so we may want to add something like "Default value for columns is to return all columns."

Copy link
Member Author

@GilbertCherrie GilbertCherrie Jan 12, 2024

Choose a reason for hiding this comment

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

Default value right now is column 1 (usually the name/description column). Should I change that?

EDIT: Actually would it make more sense to just remove the default parameter and just always have the user input the columns they want to read? This might be better since we wouldn't have to find the number of columns for the table

Copy link
Member Author

@GilbertCherrie GilbertCherrie Jan 12, 2024

Choose a reason for hiding this comment

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

Is this index value 0-based or 1-based?

@Fryguy It is 0 based but 0 is the checkbox column for this table. Tables without the checkbox would have the first column as 0 still

Copy link
Member

Choose a reason for hiding this comment

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

Ok, then I think we should still say 0-based in the comment.

Copy link
Member

Choose a reason for hiding this comment

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

Actually would it make more sense to just remove the default parameter and just always have the user input the columns they want to read? This might be better since we wouldn't have to find the number of columns for the table

Yeah that's better, because the caller (the test) is really the only one that knows the "correct" column

Copy link
Member Author

Choose a reason for hiding this comment

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

Sounds good, made the fix and re-pushed

@GilbertCherrie GilbertCherrie force-pushed the cypress_gtl_fixes branch 5 times, most recently from 3adb75a to 0a5f28d Compare January 12, 2024 20:42
return cy.get('#miq-gtl-view > .miq-data-table > .miq-data-table > .bx--data-table-content > table');
});

// columns: Array of ints of columns to read. Example: [1,2,3] will return all row data from columns 1,2 and 3.
Copy link
Member

Choose a reason for hiding this comment

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

Once we've worked out the README text, these comments should be updated to match (same with the other method below)

@miq-bot
Copy link
Member

miq-bot commented Jan 12, 2024

Checked commit GilbertCherrie@6cf843e with ruby 2.7.8, rubocop 1.56.3, haml-lint 0.51.0, and yamllint
0 files checked, 0 offenses detected
Everything looks fine. 🍰

@Fryguy Fryguy merged commit fb2eb38 into ManageIQ:master Jan 18, 2024
@GilbertCherrie GilbertCherrie deleted the cypress_gtl_fixes branch January 19, 2024 14:40
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