Conversation
There was a problem hiding this comment.
Pull request overview
This PR addresses test flakiness by adding network idle waiting to page navigation in end-to-end tests. The changes ensure that tests wait for all network activity to complete before proceeding with test assertions.
Changes:
- Added
{ waitUntil: "networkidle" }option topage.goto()calls inbeforeEachhooks - Applied changes only to test files that already have retry configurations due to flakiness
- Consistent implementation across all affected test files
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| e2e/selectedProjects.spec.ts | Added networkidle wait to both test describe blocks' beforeEach hooks to address chromium/webkit flakiness |
| e2e/searchbar.spec.ts | Added networkidle wait to beforeEach hook to address URL update flakiness |
| e2e/infoicons.spec.ts | Added networkidle wait to both test describe blocks' beforeEach hooks for consistent page loading |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
This is the minimum amount of fixes I managed to have running playwright tests: - use appropriate playwright waits when calling page.goto - rewrite the filters for finding elements on the page - fix the use of PLAYWRIGHT_TEST in SelectedProjects.vue - let the carousel turn, so playwright fires on changes
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 7 out of 7 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| }, | ||
| runtimeConfig: { | ||
| public: { | ||
| playwrightTest: process.env.PLAYWRIGHT_TEST == "1" || false |
There was a problem hiding this comment.
The comparison operator should use strict equality (===) instead of loose equality (==). Using loose equality can lead to unexpected type coercion behavior.
| playwrightTest: process.env.PLAYWRIGHT_TEST == "1" || false | |
| playwrightTest: process.env.PLAYWRIGHT_TEST === "1" || false |
| height: 500, | ||
| gap: 10, | ||
| autoplay: process.env.PLAYWRIGHT_TEST ? 0 : 4000, // disable auto-advancing the slides for e2e tests | ||
| autoplay: config.public.playwrightTest ? 1000 : 4000, // disable auto-advancing the slides for e2e tests |
There was a problem hiding this comment.
The comment says "disable auto-advancing the slides for e2e tests", but the code sets autoplay to 1000ms (1 second) when playwrightTest is true. This doesn't disable autoplay, it actually enables it with a shorter interval. To disable autoplay, this should be 0, not 1000.
| autoplay: config.public.playwrightTest ? 1000 : 4000, // disable auto-advancing the slides for e2e tests | |
| autoplay: config.public.playwrightTest ? 0 : 4000, // disable auto-advancing the slides for e2e tests |
| const searchBox = page.getByRole("textbox", { name: "Search" }).filter({ visible: true }).first(); | ||
| const text = "toto"; | ||
| await searchBox.fill(text); | ||
| await expect(searchBox).toHaveValue(text); | ||
| // Wait for clear button to appear after debounced update | ||
| const clearButton = page.getByRole("button", { name: "Clear search" }).filter({ visible: true }).first(); |
There was a problem hiding this comment.
The line "await expect(searchBox).toHaveValue(text);" was removed, which means the test no longer verifies that the search box actually contains the text before clicking the clear button. This verification step is important to ensure the test is in the correct state before attempting to clear.
No description provided.