choo121600 opened a new issue, #63036: URL: https://github.com/apache/airflow/issues/63036
### Body ## Motivation Thanks to the amazing efforts of the community, the UI E2E test suite has reached solid coverage across many areas of the Airflow UI. The work tracked in the previous meta issue #59028 successfully introduced Playwright-based tests and implemented a wide range of scenarios π However, as the test suite has grown, we have started to observe some inconsistencies in testing patterns and practices across different files. Before expanding coverage further or addressing flaky tests and performance optimizations, it would be beneficial to improve the overall consistency of our E2E tests. By improving consistency first, it will become easier to extend coverage, stabilize flaky tests, and maintain the E2E suite going forward. ## Description Our e2e test suite has grown over time, and we'd like to align it more closely with [Playwright's recommended best practices](https://playwright.dev/docs/best-practices). These changes are **not intended to alter test coverage**. The goal is to make tests more readable, more stable in CI, and easier to maintain. This issue tracks several improvements that can be addressed independently. ## Patterns to improve The following patterns appear in multiple tests and can be improved to better align with Playwright best practices. ### 1. `page.waitForFunction()` with DOM queries β locator-based waiting Using `document.querySelector()` inside `waitForFunction()` bypasses Playwright's built-in auto-waiting and retry logic. ```ts // current pattern await page.waitForFunction(() => { const rows = document.querySelectorAll("table tbody tr"); return rows.length > 0; }); // preferred await expect(page.locator("table tbody tr")).not.toHaveCount(0); ``` ### 2. `page.waitForTimeout()` β state-based waiting Fixed timeouts can slow tests and may introduce flakiness in CI. Each occurrence should be reviewed individually. In some cases the wait may be intentional (e.g., animations or debounced inputs), so the correct replacement may vary depending on context. ```ts // current pattern await page.waitForTimeout(500); // preferred β wait for the expected UI state await expect(element).toBeVisible(); ``` ### 3. `waitForLoadState("networkidle")` β wait for specific UI state Playwright discourages using `networkidle` as a general waiting strategy: https://playwright.dev/docs/navigations#networkidle It can be unreliable for modern SPAs that use polling, websockets, or other long-lived connections. ```ts // current pattern await page.waitForLoadState("networkidle"); // preferred β wait for the UI state you expect await expect(page.getByRole("table")).toBeVisible(); ``` ### 4. Manual assertions β web-first assertions Assertions such as `expect(await locator.isVisible()).toBe(true)` check the condition once. Playwrightβs web-first assertions retry automatically until the condition is met or a timeout occurs. ```ts // current pattern expect(await page.getByText("welcome").isVisible()).toBe(true); const count = await rows.count(); expect(count).toBeGreaterThan(0); // preferred await expect(page.getByText("welcome")).toBeVisible(); await expect(rows).not.toHaveCount(0); ``` ### 5. `page.evaluate()` for DOM manipulation β observe UI state instead Tests should observe application state rather than modifying the DOM directly. ```ts // current pattern await page.evaluate(() => { document.querySelector(".backdrop")?.remove(); }); // preferred await expect(page.locator(".backdrop")).toBeHidden(); ``` ### 6. CSS `:has-text()` β user-facing locators Playwright recommends user-facing locators such as `getByRole()` or `getByText()`. These tend to be more resilient to markup changes and better reflect how users interact with the UI. ```ts // current pattern page.locator('th:has-text("DAG ID")') page.locator('button:has-text("Filter")') // preferred page.getByRole("columnheader", { name: "DAG ID" }) page.getByRole("button", { name: "Filter" }) ``` ## Notes for contributors - All files are under `airflow-core/src/airflow/ui/tests/e2e/` - See [tests/e2e/README.md](https://github.com/apache/airflow/blob/main/airflow-core/src/airflow/ui/tests/e2e/README.md) for instructions on running tests locally. - Keeping PRs **small and focused** (for example, one pattern or one spec at a time) will make reviews easier. - Not every instance is a straightforward replacement β please review the surrounding context and choose the appropriate approach. - If you plan to work on a file, feel free to leave a comment so others know it is being worked on. Some of these tasks may also be **good first contributions** for people interested in improving the UI test suite π ## Files to review The following files may contain patterns that can be improved. ### Page Objects - [ ] `pages/AssetDetailPage.ts` - [ ] `pages/AssetListPage.ts` - [ ] `pages/BackfillPage.ts` - [ ] `pages/BasePage.ts` - [ ] `pages/ConnectionsPage.ts` - [ ] `pages/DagCalendarTab.ts` - [ ] `pages/DagCodePage.ts` - [ ] `pages/DagRunsPage.ts` - [ ] `pages/DagRunsTabPage.ts` - [ ] `pages/DagsPage.ts` - [ ] `pages/EventsPage.ts` - [ ] `pages/GridPage.ts` - [ ] `pages/HomePage.ts` - [ ] `pages/LoginPage.ts` - [ ] `pages/PluginsPage.ts` - [ ] `pages/PoolsPage.ts` - [ ] `pages/ProvidersPage.ts` - [ ] `pages/RequiredActionsPage.ts` - [ ] `pages/TaskInstancePage.ts` - [ ] `pages/TaskInstancesPage.ts` - [ ] `pages/VariablePage.ts` - [ ] `pages/XComsPage.ts` - [ ] `pages/configurationpage.ts` ### Specs - [ ] `specs/asset.spec.ts` - [ ] `specs/backfill.spec.ts` - [ ] `specs/configuration.spec.ts` - [ ] `specs/connections.spec.ts` - [ ] `specs/dag-audit-log.spec.ts` - [ ] `specs/dag-calendar-tab.spec.ts` - [ ] `specs/dag-code-tab.spec.ts` - [ ] `specs/dag-grid-view.spec.ts` - [ ] `specs/dag-runs-tab.spec.ts` - [ ] `specs/dag-runs.spec.ts` - [ ] `specs/dag-tasks.spec.ts` - [ ] `specs/dags-list.spec.ts` - [ ] `specs/events-page.spec.ts` - [ ] `specs/home-dashboard.spec.ts` - [ ] `specs/plugins.spec.ts` - [ ] `specs/pools.spec.ts` - [ ] `specs/providers.spec.ts` - [ ] `specs/requiredAction.spec.ts` - [ ] `specs/task-instances.spec.ts` - [ ] `specs/task-logs.spec.ts` - [ ] `specs/variable.spec.ts` - [ ] `specs/xcoms.spec.ts` ## References - https://playwright.dev/docs/best-practices - https://playwright.dev/docs/locators - https://playwright.dev/docs/test-assertions ### Committer - [x] I acknowledge that I am a maintainer/committer of the Apache Airflow project. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
