E2E Improve Playwright test patterns in connections.spec.ts by Pyasma · Pull Request #63516 · apache/airflow

Pyasma · 2026-03-13T12:47:09Z

Continuation of #63113

issue #63088, part of #63036
Improve Playwright test patterns in connections.spec.ts and related page object to align with Playwright Best Practices.

This PR focuses on improving test patterns only.
It does not change test coverage or behavior.

Improved Selectors from page.locator() to getByRole, getByText, getByPlaceholder
removed confirmDeleteButton replaced with scoped dialog locator (previous locator was a page level locator finding any delete button on the whole page but this allows to scope the delete button to open dialog element)
Replaced connectionExists() with getConnectionrow -> it directly fetches text in row until timeout
Remove DOM manipulation in deleteConnection
Simplified Dropdown Option selection
used Claude 4.6

Pyasma · 2026-03-13T12:51:46Z

Hey, I rebased the branch incorrectly earlier and the commit history got messy.
I've opened this new PR with a clean history containing only the intended changes.

Sorry about the confusion we can continue the discussion on this new PR.

choo121600

It looks like the CI is failing. Could you check it?

Pyasma · 2026-03-22T18:09:10Z

hmm ? test pass ? @choo121600 what happened?

Pyasma · 2026-04-04T12:40:25Z

hey @choo121600 can you review this

choo121600 · 2026-04-10T05:41:25Z

The PR I mentioned on Slack #63826 has been merged.
Could you please take another look when you have a chance?
I expect this will help reduce the flakiness we saw in loading performance 🙂

https://apache-airflow.slack.com/archives/C06K9Q5G2UA/p1775306859262719?thread_ts=1775306460.014489&cid=C06K9Q5G2UA

Copilot

Pull request overview

Note

Copilot was unable to run its full agentic suite in this review.

Refactors the Connections E2E Playwright spec + page object to follow more “web-first” Playwright patterns (role/text/placeholder locators, expect-based waits) while aiming to keep behavior unchanged (continuation of #63113 / issues #63088 and #63036).

Changes:

Replaced manual counts/polling with web-first assertions (toHaveURL, toHaveCount, toBeVisible, toPass).
Updated selectors to getByRole / getByText / getByPlaceholder and added connectionRows helper locator.
Reworked delete/search/edit flows to reduce DOM manipulation and scope locators to the active dialog/form.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 5 comments.

File	Description
airflow-core/src/airflow/ui/tests/e2e/specs/connections.spec.ts	Updates assertions in CRUD/search tests to be more Playwright “web-first” and to use new row locators.
airflow-core/src/airflow/ui/tests/e2e/pages/ConnectionsPage.ts	Modernizes locators and refactors delete/search/edit helpers to rely more on Playwright assertions and scoped locators.

Copilot · 2026-04-10T19:58:27Z

+    // Verify filtered results contain the search term
+    await expect(connectionsPage.connectionRows).toHaveCount(1);
+    await expect(connectionsPage.getConnectionRow(searchTerm)).toBeVisible();


toHaveCount(1) bakes in an assumption that searching by connection_id yields exactly one row. If the UI search matches other columns (or other IDs contain the searched substring), this becomes flaky while the feature still works. Prefer asserting that a matching row is visible and (optionally) that all visible rows contain the search term (e.g., via expect(connectionsPage.connectionRows).toContainText(...) or a small expect.poll over row texts) instead of enforcing a fixed count of 1.

Suggested change

// Verify filtered results contain the search term

await expect(connectionsPage.connectionRows).toHaveCount(1);

await expect(connectionsPage.getConnectionRow(searchTerm)).toBeVisible();

// Verify filtered results contain the search term without assuming a single exact match

await expect(connectionsPage.getConnectionRow(searchTerm)).toBeVisible();

await expect

.poll(async () => {

const rowTexts = await connectionsPage.connectionRows.allTextContents();

return rowTexts.length > 0 && rowTexts.every((text) => text.toLowerCase().includes(searchTerm.toLowerCase()));

})

.toBe(true);

Copilot · 2026-04-10T19:58:28Z

+    this.tableHeader = page.getByRole("columnheader").first();
+
+    this.connectionIdHeader = page.getByText("Connection ID").first();
+    this.connectionTypeHeader = page.getByText("Connection Type").first();
+    this.hostHeader = page.getByText("Host").first();


Using getByText(...).first() for column headers can match non-header text (e.g., labels/help text in dialogs) and then “first()” may hide selector ambiguity. Since these are table headers, prefer role-based selectors scoped to the header context (e.g., getByRole("columnheader", { name: "Connection ID" })), which is typically more resilient and aligns with Playwright best practices.

Suggested change

this.tableHeader = page.getByRole("columnheader").first();

this.connectionIdHeader = page.getByText("Connection ID").first();

this.connectionTypeHeader = page.getByText("Connection Type").first();

this.hostHeader = page.getByText("Host").first();

this.tableHeader = this.connectionsTable.getByRole("columnheader").first();

this.connectionIdHeader = this.connectionsTable.getByRole("columnheader", { name: "Connection ID" });

this.connectionTypeHeader = this.connectionsTable.getByRole("columnheader", { name: "Connection Type" });

this.hostHeader = this.connectionsTable.getByRole("columnheader", { name: "Host" });

Copilot · 2026-04-10T19:58:28Z

    await expect
      .poll(
        async () => {
-          const count1 = await this.page.locator("tbody tr").count();
-
-          await this.page.evaluate(() => new Promise((r) => setTimeout(r, 200)));
-          const count2 = await this.page.locator("tbody tr").count();
+          const count = await this.page.locator("tbody tr").count();

-          if (count1 === count2 && count1 > 0) {
-            stableRowCount = count1;
+          if (count > 0) {
+            stableRowCount = count;

            return true;
          }


This no longer waits for the row count to stabilize—only for it to become non-zero. That can reintroduce timing races (e.g., rows still rendering/sorting/paginating) and make downstream actions flaky. Consider reinstating a stabilization check (two consecutive counts separated by a short delay) or waiting on a UI “loading/ready” signal specific to the table instead of returning as soon as any row exists.

Copilot · 2026-04-10T19:58:28Z

+    // Wait for form to be fully populated with existing connection data before interacting
+    await expect(this.connectionIdInput).toHaveValue(connectionId, { timeout: 10_000 });
+
+    // Wait for the form to stabilize — React re-renders the conn_type combobox as
+    // existing connection data loads from the API, which causes "detached from DOM"
+    // errors if we interact with it too early.
+    await this.page.waitForLoadState("networkidle", { timeout: 10_000 });



waitForLoadState("networkidle") is often unreliable in SPA-style apps and can hang or be flaky due to background polling/telemetry requests. Prefer waiting for a concrete UI condition that indicates the form is ready (e.g., the conn_type combobox/input has the expected value, a loading indicator disappears, or waiting for the specific API response that populates the form).

Suggested change

// Wait for form to be fully populated with existing connection data before interacting

await expect(this.connectionIdInput).toHaveValue(connectionId, { timeout: 10_000 });

// Wait for the form to stabilize — React re-renders the conn_type combobox as

// existing connection data loads from the API, which causes "detached from DOM"

// errors if we interact with it too early.

await this.page.waitForLoadState("networkidle", { timeout: 10_000 });

// Wait for form to be fully populated with existing connection data before interacting.

// Use concrete UI readiness checks instead of `networkidle`, which is flaky in SPA-style

// apps that may continue background requests while the form is already ready.

await expect(this.connectionIdInput).toBeVisible({ timeout: 10_000 });

await expect(this.connectionIdInput).toBeEnabled({ timeout: 10_000 });

await expect(this.connectionIdInput).toHaveValue(connectionId, { timeout: 10_000 });

Copilot · 2026-04-10T19:58:29Z

+    // force: true bypasses Playwright's hit-testing check — the button is correct but
+    // the dialog backdrop can still briefly overlap it right after the animation ends.
+    await confirmButton.click({ force: true });


Clicking with force: true bypasses Playwright’s actionability checks and can mask real UX regressions (e.g., the button is genuinely not clickable). If possible, prefer waiting until the button is actually actionable (visible + enabled + not covered) and then clicking without force. If overlap is transient, a more targeted wait (e.g., for the backdrop/transition state) is usually safer than forcing the click.

Suggested change

// force: true bypasses Playwright's hit-testing check — the button is correct but

// the dialog backdrop can still briefly overlap it right after the animation ends.

await confirmButton.click({ force: true });

await expect(confirmButton).toBeEnabled({ timeout: 5000 });

// Wait until Playwright's actionability checks pass, including hit-testing, before

// performing the real click. This avoids masking genuine UI regressions with force.

await confirmButton.click({ trial: true });

await confirmButton.click();

choo121600 · 2026-04-16T03:53:41Z

Looks like this needs a rebase. Could you update it?

choo121600 · 2026-04-22T16:20:04Z

Feel free to mention me once everything is ready :)

Pyasma · 2026-04-23T04:01:38Z

Feel free to mention me once everything is ready :)

Sure I just have to review the code myself and then I will mention you

…rtions

Pyasma requested review from choo121600 and vatsrahul1001 as code owners March 13, 2026 12:47

boring-cyborg Bot added the area:UI Related to UI/UX. For Frontend Developers. label Mar 13, 2026

choo121600 reviewed Mar 13, 2026

View reviewed changes

Pyasma force-pushed the NewLocatorBased branch from 7679291 to 093c341 Compare March 14, 2026 05:42

Pyasma changed the title ~~refactor: Update E2E tests to use web-first Playwright assertions and…~~ E2E Improve Playwright test patterns in connections.spec.ts Mar 14, 2026

Pyasma marked this pull request as draft March 14, 2026 06:36

Pyasma marked this pull request as ready for review March 15, 2026 12:02

choo121600 linked an issue Mar 16, 2026 that may be closed by this pull request

E2E: Improve Playwright test patterns in connections.spec.ts #63088

Closed

9 tasks

choo121600 reviewed Mar 16, 2026

View reviewed changes

Pyasma marked this pull request as draft March 18, 2026 12:28

Pyasma marked this pull request as ready for review March 21, 2026 10:24

Pyasma force-pushed the NewLocatorBased branch 2 times, most recently from c5f6ec8 to a8523e1 Compare March 22, 2026 19:09

choo121600 reviewed Mar 23, 2026

View reviewed changes

Comment thread airflow-core/src/airflow/ui/tests/e2e/pages/ConnectionsPage.ts

Pyasma force-pushed the NewLocatorBased branch 4 times, most recently from 61626c0 to 4885525 Compare March 25, 2026 09:12

Pyasma marked this pull request as draft March 25, 2026 09:34

Pyasma marked this pull request as ready for review March 30, 2026 11:33

Pyasma force-pushed the NewLocatorBased branch from e9ec3e2 to fb4935c Compare March 30, 2026 13:47

Pyasma marked this pull request as draft April 1, 2026 12:33

Pyasma marked this pull request as ready for review April 3, 2026 16:01

Pyasma force-pushed the NewLocatorBased branch from fb4935c to c63deaf Compare April 4, 2026 11:03

potiuk added the ready for maintainer review Set after triaging when all criteria pass. label Apr 6, 2026

kaxil requested a review from Copilot April 10, 2026 19:55

Copilot AI reviewed Apr 10, 2026

View reviewed changes

Pyasma marked this pull request as draft April 13, 2026 16:29

Pyasma force-pushed the NewLocatorBased branch 2 times, most recently from 67e6b7a to e390fa6 Compare April 14, 2026 06:11

Pyasma marked this pull request as ready for review April 21, 2026 06:32

Pyasma marked this pull request as draft May 2, 2026 14:40

refactor: improve E2E connection tests with web-first Playwright asse…

6e0348d

…rtions

Pyasma force-pushed the NewLocatorBased branch from 941464c to 6e0348d Compare May 3, 2026 07:33

run lin

4b37635

Pyasma force-pushed the NewLocatorBased branch from 5a16e74 to 4b37635 Compare May 5, 2026 09:56

Pyasma added 3 commits May 5, 2026 15:35

run prettier

a19923c

Merge branch 'main' into NewLocatorBased

2f4ec08

simplified delete connection

153f565

-    // force: true bypasses Playwright's hit-testing check — the button is correct but
-    // the dialog backdrop can still briefly overlap it right after the animation ends.
-    await confirmButton.click({ force: true });
+    await expect(confirmButton).toBeEnabled({ timeout: 5000 });
+    // Wait until Playwright's actionability checks pass, including hit-testing, before
+    // performing the real click. This avoids masking genuine UI regressions with force.
+    await confirmButton.click({ trial: true });
+    await confirmButton.click();

Conversation

Pyasma commented Mar 13, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Pyasma commented Mar 13, 2026

Uh oh!

choo121600 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Pyasma commented Mar 22, 2026

Uh oh!

Uh oh!

Pyasma commented Apr 4, 2026

Uh oh!

choo121600 commented Apr 10, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Apr 10, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 10, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 10, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 10, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 10, 2026

Choose a reason for hiding this comment

Uh oh!

choo121600 commented Apr 16, 2026

Uh oh!

choo121600 commented Apr 22, 2026

Uh oh!

Pyasma commented Apr 23, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Pyasma commented Mar 13, 2026 •

edited

Loading