OBPIH-7531 Add assertions for putaway details and list pages #67
Conversation
|
|
||
| get rows() { | ||
| return this.table.getByRole('row'); | ||
| return this.table.getByRole('cell'); |
There was a problem hiding this comment.
I assume it's due to badly written HTML, not your fault
There was a problem hiding this comment.
Pull request overview
This PR adds comprehensive test assertions for putaway details and list pages in the e2e test suite, along with upgrading Playwright from version 1.34.0 to 1.57.0 and updating the Node.js runtime from version 14 to 18.
Key Changes:
- Added new test file with detailed assertions for putaway workflows including creation, viewing, editing, and completion
- Created new page object models for OrderHeaderTable and ItemStatusTable components
- Updated existing page objects to include additional UI element getters for enhanced test coverage
- Upgraded Playwright test framework and Node.js runtime to newer versions
Reviewed changes
Copilot reviewed 10 out of 11 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| src/tests/putaway/assertPutawayDetailsPage.test.ts | New comprehensive test file covering putaway details page functionality with multiple test steps for creating, viewing, and completing putaway orders |
| src/pages/putaway/putawayDetails/components/SummaryTable.ts | Modified table element selectors and added getColumnHeader method for column validation |
| src/pages/putaway/putawayDetails/components/OrderHeaderTable.ts | New page object for order header table with getters for order number, status, and order type |
| src/pages/putaway/putawayDetails/components/ItemStatusTable.ts | New page object for item status table with row navigation and status field getters |
| src/pages/putaway/putawayDetails/PutawayDetailsPage.ts | Added multiple button and action menu element getters, integrated new table components |
| src/pages/putaway/list/PutawayListTable.ts | Added statusTag getter for row status verification |
| src/pages/putaway/list/PutawayListPage.ts | Added filter and search element getters for list page interactions |
| package.json | Upgraded @playwright/test from ~1.34.0 to ~1.57.0 |
| package-lock.json | Updated lockfile to reflect Playwright upgrade and dependency changes |
| README.md | Updated Node requirement from 14.21.x to 18.19.x and NPM from 6.14.6 to 9.2.0 |
| .github/workflows/playwright.yml | Updated Node.js version from 14 to 18 in CI workflow |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
| get rows() { | ||
| return this.table.getByRole('row'); | ||
| return this.table.getByRole('cell'); |
There was a problem hiding this comment.
The rows getter returns cells instead of rows, which is semantically incorrect. It should return this.table.getByRole('row') to properly represent table rows. This mismatch between the getter name and what it returns could cause confusion and bugs when developers expect to work with rows but are actually working with cells.
| return this.table.getByRole('cell'); | |
| return this.table.getByRole('row'); |
There was a problem hiding this comment.
yeah, I mentioned that above
| } | ||
|
|
||
| get rows() { | ||
| return this.table.getByRole('cell'); |
There was a problem hiding this comment.
Similar issue: the rows getter returns cells (getByRole('cell')) instead of rows. This should be this.table.getByRole('row') to correctly represent table rows and maintain consistency with the getter name.
| return this.table.getByRole('cell'); | |
| return this.table.getByRole('row'); |
| return this.page | ||
| .getByTestId('item-status-table') | ||
| .getByTestId('order-item-status-code'); | ||
| } | ||
|
|
||
| get originBin() { | ||
| return this.page | ||
| .getByTestId('item-status-table') | ||
| .getByTestId('origin-bin-location'); | ||
| } | ||
|
|
||
| get destinationBin() { | ||
| return this.page | ||
| .getByTestId('item-status-table') | ||
| .getByTestId('destination-bin-location'); |
There was a problem hiding this comment.
The Row class getters (itemStatus, originBin, destinationBin) don't use the row property that was passed in the constructor. They all query from this.page.getByTestId('item-status-table') directly, making the Row instance's locator redundant. These getters will always return the first matching element in the table regardless of which row instance is being accessed, potentially causing incorrect assertions when there are multiple rows.
| return this.page | |
| .getByTestId('item-status-table') | |
| .getByTestId('order-item-status-code'); | |
| } | |
| get originBin() { | |
| return this.page | |
| .getByTestId('item-status-table') | |
| .getByTestId('origin-bin-location'); | |
| } | |
| get destinationBin() { | |
| return this.page | |
| .getByTestId('item-status-table') | |
| .getByTestId('destination-bin-location'); | |
| return this.row.getByTestId('order-item-status-code'); | |
| } | |
| get originBin() { | |
| return this.row.getByTestId('origin-bin-location'); | |
| } | |
| get destinationBin() { | |
| return this.row.getByTestId('destination-bin-location'); |
Add page elements and assertions for: