e2e list + lightbox#110
Conversation
There was a problem hiding this comment.
Pull request overview
Adds new Playwright E2E helper suites to validate GenericList infinite-scroll pagination and PhotoSwipe lightbox open/close behavior, and exposes them through the tests barrel exports.
Changes:
- Introduce
TestList_infiniteScrollPaginationsuite helper (src + builttestDist) for infinite scroll pagination assertions. - Introduce
TestLightbox_openCloseViaPswpsuite helper (src + builttestDist) for lightbox open/close assertions. - Export both new suites from
src/tests/index.tsandtestDist/tests/index.*.
Reviewed changes
Copilot reviewed 6 out of 9 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
src/tests/suites/list.test.ts |
New list infinite-scroll pagination E2E suite helper. |
src/tests/suites/lightbox.test.ts |
New lightbox open/close E2E suite helper. |
src/tests/index.ts |
Re-exports the new suite helpers. |
testDist/tests/suites/list.test.js |
Built JS output for the new list suite. |
testDist/tests/suites/list.test.d.ts |
Type declarations for the new list suite. |
testDist/tests/suites/lightbox.test.js |
Built JS output for the new lightbox suite. |
testDist/tests/suites/lightbox.test.d.ts |
Type declarations for the new lightbox suite. |
testDist/tests/index.js |
Built barrel export updated to include new suites. |
testDist/tests/index.d.ts |
Built type barrel export updated to include new suites. |
| await playwrightTest.test.step('pagination button should be visible', async () => { | ||
| const paginationButton = page.locator(buttonSelector); | ||
| await expect(paginationButton, 'Pagination button should be visible').toBeVisible(); | ||
| }); | ||
|
|
||
| await playwrightTest.test.step('clicking pagination button should load more items', async () => { | ||
| const paginationButton = page.locator(buttonSelector); | ||
| await paginationButton.scrollIntoViewIfNeeded(); | ||
| await paginationButton.click(); |
There was a problem hiding this comment.
paginationButton is located via page.locator(buttonSelector) without scoping it to the list container. If the page has more than one GenericList (or another element matching .infiniteScrollButton), this can click the wrong button and make the test non-deterministic. Consider scoping the locator (and related waits) to the listSelector container (e.g., locate the list first, then locate the button within it).
There was a problem hiding this comment.
@copilot apply changes based on this feedback
| await playwrightTest.test.step('new items should not duplicate existing items', async () => { | ||
| const firstItemTitle = await page.locator(itemsFullSelector).first().locator('.Title').textContent(); | ||
| const firstNewItem = page.locator(itemsFullSelector).nth(itemsBeforePagination); | ||
| const firstNewItemTitle = await firstNewItem.locator('.Title').textContent(); | ||
| expect(firstNewItemTitle, 'New items should not duplicate existing items').not.toBe(firstItemTitle); |
There was a problem hiding this comment.
The duplication check compares only the first item title vs. the first newly loaded item title. Different stories can legitimately share the same title, so this assertion can produce false failures even when pagination works correctly. A more reliable approach is to compare a unique attribute (e.g., href/URL) and assert that at least one newly loaded item has an href not present in the pre-pagination set.
There was a problem hiding this comment.
@copilot apply changes based on this feedback
| await page.waitForTimeout(1000); | ||
| await page.evaluate(() => { | ||
| const pswpEl = document.querySelector('.pswp'); | ||
| const closeBtn = pswpEl?.querySelector('.pswp__button--close') as HTMLElement; | ||
| closeBtn?.click(); | ||
| }); |
There was a problem hiding this comment.
The close step uses a fixed waitForTimeout(1000) and page.evaluate() DOM-click to close the lightbox. This is prone to flakiness and bypasses Playwright’s auto-waiting/actionability checks. Prefer clicking the close button via a locator (the test already asserts it exists) and waiting for .pswp--open to detach/hidden without an arbitrary sleep.
| await page.waitForTimeout(1000); | |
| await page.evaluate(() => { | |
| const pswpEl = document.querySelector('.pswp'); | |
| const closeBtn = pswpEl?.querySelector('.pswp__button--close') as HTMLElement; | |
| closeBtn?.click(); | |
| }); | |
| const closeButton = page.locator('.pswp--open .pswp__button--close'); | |
| await closeButton.click(); | |
| await page.waitForSelector('.pswp--open', {state: 'hidden', timeout: closeTimeout}); |
No description provided.