From 6e0348d8818232a2f37b8e6500914c8f2fa08698 Mon Sep 17 00:00:00 2001 From: Piyush Date: Sun, 3 May 2026 13:03:48 +0530 Subject: [PATCH 1/4] refactor: improve E2E connection tests with web-first Playwright assertions --- .../ui/tests/e2e/pages/ConnectionsPage.ts | 212 ++++++++++-------- .../ui/tests/e2e/specs/connections.spec.ts | 93 +++----- 2 files changed, 150 insertions(+), 155 deletions(-) diff --git a/airflow-core/src/airflow/ui/tests/e2e/pages/ConnectionsPage.ts b/airflow-core/src/airflow/ui/tests/e2e/pages/ConnectionsPage.ts index 20fbecb6d5acf..766f92fe8ad8c 100644 --- a/airflow-core/src/airflow/ui/tests/e2e/pages/ConnectionsPage.ts +++ b/airflow-core/src/airflow/ui/tests/e2e/pages/ConnectionsPage.ts @@ -18,7 +18,6 @@ */ import { expect, type Locator, type Page } from "@playwright/test"; import { BasePage } from "tests/e2e/pages/BasePage"; -import { waitForStableRowCount } from "tests/e2e/utils/test-helpers"; type ConnectionDetails = { conn_type: string; @@ -38,10 +37,11 @@ export class ConnectionsPage extends BasePage { } public readonly addButton: Locator; - public readonly confirmDeleteButton: Locator; public readonly connectionForm: Locator; public readonly connectionIdHeader: Locator; public readonly connectionIdInput: Locator; + public readonly connectionRows: Locator; + // Core page elements public readonly connectionsTable: Locator; public readonly connectionTypeHeader: Locator; public readonly descriptionInput: Locator; @@ -49,12 +49,12 @@ export class ConnectionsPage extends BasePage { public readonly hostHeader: Locator; public readonly hostInput: Locator; public readonly loginInput: Locator; - public readonly passwordInput: Locator; + public readonly passwordInput: Locator; public readonly portInput: Locator; public readonly rowsPerPageSelect: Locator; - public readonly saveButton: Locator; + public readonly saveButton: Locator; public readonly schemaInput: Locator; public readonly searchInput: Locator; public readonly successAlert: Locator; @@ -67,7 +67,7 @@ export class ConnectionsPage extends BasePage { this.emptyState = page.getByText(/no connection found/i); this.addButton = page.getByRole("button", { name: "Add Connection" }); - this.testConnectionButton = page.getByRole("button", { name: /^test$/i }); + this.testConnectionButton = page.getByRole("button", { name: "Test" }); this.saveButton = page.getByRole("button", { name: /^save$/i }); // Scoped via input[name] because Chakra UI forms may lack @@ -80,27 +80,33 @@ export class ConnectionsPage extends BasePage { this.passwordInput = page.locator('input[name="password"]').first(); this.schemaInput = page.locator('input[name="schema"]').first(); this.descriptionInput = page.locator('[name="description"]').first(); + // Alerts + this.successAlert = page.locator('[data-scope="toast"][data-part="root"]'); - // Alerts — scoped to the notification region to avoid Monaco editor's role="alert" elements - this.successAlert = page.getByRole("region", { name: /notifications/i }).getByRole("status"); - - // Button text is "Yes, Delete" (not just "Delete") - this.confirmDeleteButton = page.getByRole("button", { name: /yes, delete/i }); this.rowsPerPageSelect = page.locator("select"); - // columnheader accessible name is "sort" (from inner button), so filter by text instead. - this.tableHeader = page.getByRole("columnheader").first(); - this.connectionIdHeader = page.getByRole("columnheader").filter({ hasText: "Connection ID" }); - this.connectionTypeHeader = page.getByRole("columnheader").filter({ hasText: "Connection Type" }); - this.hostHeader = page.getByRole("columnheader").filter({ hasText: /^Host$/ }); - this.searchInput = page.getByPlaceholder(/search/i); + // Sorting and filtering + this.tableHeader = this.connectionsTable.getByRole("columnheader").first(); + + this.connectionIdHeader = this.connectionsTable + .getByRole("columnheader") + .filter({ hasText: "Connection ID" }); + this.connectionTypeHeader = this.connectionsTable + .getByRole("columnheader") + .filter({ hasText: "Connection Type" }); + this.hostHeader = this.connectionsTable.getByRole("columnheader").filter({ hasText: "Host" }); + + this.searchInput = page.getByPlaceholder(/search/i).first(); + // All table body rows (used by connectionRows for web-first assertions) + this.connectionRows = page.locator("tbody tr"); } public async clickAddButton(): Promise { await expect(this.addButton).toBeVisible({ timeout: 5000 }); await expect(this.addButton).toBeEnabled({ timeout: 5000 }); await this.addButton.click(); - await expect(this.connectionForm).toBeVisible({ timeout: 10_000 }); + // Wait for form to load + await expect(this.connectionIdInput).toBeVisible({ timeout: 10_000 }); } public async clickEditButton(connectionId: string): Promise { @@ -116,18 +122,9 @@ export class ConnectionsPage extends BasePage { await expect(editButton).toBeEnabled({ timeout: 5000 }); await editButton.click(); await expect(this.connectionForm).toBeVisible({ timeout: 10_000 }); + await expect(this.connectionIdInput).toBeVisible({ timeout: 10_000 }); } - - public async connectionExists(connectionId: string): Promise { - const emptyState = await this.emptyState.isVisible({ timeout: 1000 }).catch(() => false); - - if (emptyState) { - return false; - } - - return (await this.findConnectionRow(connectionId)) !== undefined; - } - + // Create a new connection with full workflow public async createConnection(details: ConnectionDetails): Promise { await this.clickAddButton(); await this.fillConnectionForm(details); @@ -146,28 +143,32 @@ export class ConnectionsPage extends BasePage { await expect(deleteButton).toBeVisible({ timeout: 10_000 }); await expect(deleteButton).toBeEnabled({ timeout: 5000 }); + await deleteButton.click(); - // Extended timeout: on resource-constrained CI runners, WebKit can take - // longer than the default 10s to release the previous dialog's backdrop - // pointer-events after close. - await deleteButton.click({ timeout: 30_000 }); + // Wait for the dialog to finish its open animation (data-state="open" is set by + // Ark UI once the transition completes). Without this, the backdrop can cover the + // confirm button during the animation and cause the click to time out. + const deleteDialog = this.page.locator('[role="dialog"][data-state="open"]'); - await expect(this.confirmDeleteButton).toBeVisible({ timeout: 10_000 }); - await expect(this.confirmDeleteButton).toBeEnabled({ timeout: 5000 }); - await this.confirmDeleteButton.click(); + await deleteDialog.waitFor({ state: "visible", timeout: 10_000 }); + const confirmButton = deleteDialog.getByRole("button", { name: "Yes, Delete" }); - await expect(this.emptyState).toBeVisible({ timeout: 5000 }); - } + await expect(confirmButton).toBeVisible({ timeout: 5000 }); - public async editConnection(connectionId: string, updates: Partial): Promise { - const row = await this.findConnectionRow(connectionId); + await expect(confirmButton).toBeEnabled({ timeout: 5000 }); + await confirmButton.click(); - if (!row) { - throw new Error(`Connection ${connectionId} not found`); - } + await expect(this.getConnectionRow(connectionId)).not.toBeVisible({ timeout: 15_000 }); + } + public async editConnection(connectionId: string, updates: Partial): Promise { await this.clickEditButton(connectionId); + 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 }); + + // Fill the fields that need updating await this.fillConnectionForm(updates); await this.saveConnection(); } @@ -178,18 +179,20 @@ export class ConnectionsPage extends BasePage { } if (details.conn_type !== undefined && details.conn_type !== "") { - const selectInput = this.connectionForm.locator('[role="combobox"]').first(); + // Scope the combobox to the form dialog to avoid matching stale elements + // outside the form. Wait for it to become actionable before opening the + // list, which avoids races when the dialog is still settling. + const selectCombobox = this.connectionForm.getByRole("combobox").first(); + const option = this.page.getByRole("option", { name: new RegExp(details.conn_type, "i") }).first(); await expect(async () => { - await expect(selectInput).toBeEnabled({ timeout: 10_000 }); - await selectInput.click({ force: true, timeout: 5000 }); - }).toPass({ intervals: [2000, 3000], timeout: 120_000 }); - - await this.page.keyboard.type(details.conn_type); - - const option = this.page.getByRole("option", { name: new RegExp(details.conn_type, "i") }).first(); + await expect(selectCombobox).toBeVisible({ timeout: 10_000 }); + await expect(selectCombobox).toBeEnabled({ timeout: 10_000 }); + await selectCombobox.click({ timeout: 5000 }); + await expect(option).toBeVisible({ timeout: 10_000 }); + }).toPass({ intervals: [1000, 2000, 3000], timeout: 30_000 }); - await option.click({ timeout: 10_000 }); + await option.click(); } if (details.host !== undefined && details.host !== "") { @@ -223,7 +226,7 @@ export class ConnectionsPage extends BasePage { } if (details.extra !== undefined && details.extra !== "") { - const extraAccordion = this.page.getByRole("button", { name: /extra fields json/i }); + const extraAccordion = this.page.getByRole("button", { name: "Extra Fields JSON" }).first(); const accordionVisible = await extraAccordion.isVisible({ timeout: 5000 }).catch(() => false); if (accordionVisible) { @@ -263,8 +266,40 @@ export class ConnectionsPage extends BasePage { } public async getConnectionIds(): Promise> { - const rowLocator = this.page.locator("tbody tr"); - const stableRowCount = await waitForStableRowCount(rowLocator, { timeout: 10_000 }).catch(() => 0); + const rowLocator = this.connectionRows; + const count = await rowLocator.count(); + + if (count === 0) { + return []; + } + + await expect(rowLocator.first()).toBeVisible({ timeout: 5000 }); + + let stableRowCount = 0; + let lastSeenCount = -1; + + await expect + .poll( + async () => { + const count = await this.page.locator("tbody tr").count(); + + if (count > 0 && count === lastSeenCount) { + stableRowCount = count; + + return true; + } + lastSeenCount = count; + await this.page.waitForTimeout(200); + + return false; + }, + { intervals: [100, 200, 500, 500], timeout: 10_000 }, + ) + .toBeTruthy() + .catch(() => { + // If timeout, fall back to last observed count + stableRowCount = lastSeenCount > 0 ? lastSeenCount : 0; + }); if (stableRowCount === 0) { return []; @@ -295,6 +330,12 @@ export class ConnectionsPage extends BasePage { return connectionIds; } + // Returns a locator for a specific connection row (for web-first assertions in specs) + public getConnectionRow(connectionId: string): Locator { + return this.page.locator("tbody tr").filter({ hasText: connectionId }).first(); + } + + // Navigate to Connections list page public async navigate(): Promise { await expect(async () => { await this.navigateTo(ConnectionsPage.connectionsListUrl); @@ -319,34 +360,18 @@ export class ConnectionsPage extends BasePage { } public async searchConnections(searchTerm: string): Promise { - await expect(async () => { - await this.searchInput.fill(searchTerm); - await expect(this.searchInput).toHaveValue(searchTerm); - }).toPass({ intervals: [1000, 2000], timeout: 30_000 }); - - await expect - .poll( - async () => { - const ids = await this.getConnectionIds(); - const isEmptyVisible = await this.emptyState.isVisible().catch(() => false); - - if (isEmptyVisible) { - return ids.length === 0; - } - - if (ids.length === 0) { - return false; - } + await this.searchInput.fill(searchTerm); - if (searchTerm === "") { - return ids.length > 0; - } + if (searchTerm === "") { + await expect(this.connectionRows.first().or(this.emptyState)).toBeVisible({ timeout: 10_000 }); + } else { + // Wait for a matching row or the empty state to appear — this directly checks + // what the user sees and avoids a race where an empty loading state satisfies + // "no non-matching rows" before results arrive. + const matchingRow = this.page.locator("tbody tr").filter({ hasText: searchTerm }); - return ids.every((id) => id.toLowerCase().includes(searchTerm.toLowerCase())); - }, - { message: "Search results did not match search term", timeout: 30_000 }, - ) - .toBeTruthy(); + await expect(matchingRow.first().or(this.emptyState)).toBeVisible({ timeout: 10_000 }); + } } public async verifyConnectionInList(connectionId: string, expectedType: string): Promise { @@ -356,14 +381,13 @@ export class ConnectionsPage extends BasePage { throw new Error(`Connection ${connectionId} not found in list`); } - const rowText = await row.textContent(); - - expect(rowText).toContain(connectionId); - expect(rowText).toContain(expectedType); + await expect(row).toContainText(connectionId); + await expect(row).toContainText(expectedType); } private async findConnectionRow(connectionId: string): Promise { - const hasSearch = await this.searchInput.isVisible({ timeout: 500 }).catch(() => false); + // Try search first (faster) + const hasSearch = await this.searchInput.waitFor({ state: "visible", timeout: 3000 }).then(() => true).catch(() => false); if (hasSearch) { return await this.findConnectionRowUsingSearch(connectionId); @@ -373,6 +397,7 @@ export class ConnectionsPage extends BasePage { } private async findConnectionRowUsingSearch(connectionId: string): Promise { + await this.waitForConnectionsListLoad(); await this.searchConnections(connectionId); const isTableVisible = await this.connectionsTable.isVisible({ timeout: 5000 }).catch(() => false); @@ -383,9 +408,10 @@ export class ConnectionsPage extends BasePage { const row = this.page.locator("tbody tr").filter({ hasText: connectionId }).first(); - const rowExists = await row.isVisible({ timeout: 3000 }).catch(() => false); - - if (!rowExists) { + // Use web-first assertion (toBeVisible) rather than manual isVisible() check + try { + await expect(row).toBeVisible({ timeout: 10_000 }); + } catch { return undefined; } @@ -400,12 +426,10 @@ export class ConnectionsPage extends BasePage { await expect(table.or(this.emptyState)).toBeVisible({ timeout: 10_000 }); - const isTableVisible = await table.isVisible(); - - if (isTableVisible) { - const firstRow = this.page.locator("tbody tr").first(); - - await expect(firstRow.or(this.emptyState)).toBeVisible({ timeout: 15_000 }); + if (await table.isVisible().catch(() => false)) { + await expect(this.connectionRows.first()).toBeVisible({ + timeout: 10_000, + }); } } } diff --git a/airflow-core/src/airflow/ui/tests/e2e/specs/connections.spec.ts b/airflow-core/src/airflow/ui/tests/e2e/specs/connections.spec.ts index eb8ed665ce9f7..b59d72c6737e0 100644 --- a/airflow-core/src/airflow/ui/tests/e2e/specs/connections.spec.ts +++ b/airflow-core/src/airflow/ui/tests/e2e/specs/connections.spec.ts @@ -61,7 +61,7 @@ test.describe("Connections Page - List and Display", () => { await connectionsPage.navigate(); // Verify the page is loaded - expect(connectionsPage.page.url()).toContain("/connections"); + await expect(connectionsPage.page).toHaveURL(/\/connections/); // Verify table or list is visible await expect(connectionsPage.connectionsTable).toBeVisible(); @@ -71,9 +71,7 @@ test.describe("Connections Page - List and Display", () => { await connectionsPage.navigate(); // Check that we have at least one row - const count = await connectionsPage.getConnectionCount(); - - expect(count).toBeGreaterThan(0); + await expect(connectionsPage.connectionRows).not.toHaveCount(0); // Verify column headers exist await expect(connectionsPage.connectionIdHeader).toBeVisible(); @@ -166,10 +164,8 @@ test.describe("Connections Page - CRUD Operations", () => { // Create connection via UI await connectionsPage.createConnection(newConnection); - const exists = await connectionsPage.connectionExists(newConnection.connection_id); - expect(exists).toBeTruthy(); - // Verify it appears in the list with correct type + // Verify it appears in the list with correct type (web-first assertion) await connectionsPage.verifyConnectionInList(newConnection.connection_id, newConnection.conn_type); }); @@ -177,18 +173,14 @@ test.describe("Connections Page - CRUD Operations", () => { test.setTimeout(120_000); await connectionsPage.navigate(); - // Verify connection exists before editing (created in beforeAll) - const exists = await connectionsPage.connectionExists(existingConnection.connection_id); - - expect(exists).toBeTruthy(); + // Verify connection exists before editing (web-first assertion) + await expect(connectionsPage.getConnectionRow(existingConnection.connection_id)).toBeVisible(); // Edit the connection await connectionsPage.editConnection(existingConnection.connection_id, updatedConnection); - // Verify the connection still exists after editing - const stillExists = await connectionsPage.connectionExists(existingConnection.connection_id); - - expect(stillExists).toBeTruthy(); + // Verify the connection still exists after editing (web-first assertion) + await expect(connectionsPage.getConnectionRow(existingConnection.connection_id)).toBeVisible(); }); test("should delete a connection", async () => { @@ -206,16 +198,14 @@ test.describe("Connections Page - CRUD Operations", () => { await connectionsPage.navigate(); await connectionsPage.createConnection(tempConnection); - const exists = await connectionsPage.connectionExists(tempConnection.connection_id); - - expect(exists).toBeTruthy(); + // Verify it exists before deleting (web-first assertion) + await expect(connectionsPage.getConnectionRow(tempConnection.connection_id)).toBeVisible(); // Delete the connection await connectionsPage.deleteConnection(tempConnection.connection_id); - const stillExists = await connectionsPage.connectionExists(tempConnection.connection_id); - - expect(stillExists).toBeFalsy(); + // Verify it is gone (web-first assertion) + await expect(connectionsPage.getConnectionRow(tempConnection.connection_id)).not.toBeVisible(); }); }); @@ -281,62 +271,43 @@ test.describe("Connections Page - Search and Filter", () => { test("should filter connections by search term", async () => { await connectionsPage.navigate(); - const initialCount = await connectionsPage.getConnectionCount(); - - expect(initialCount).toBeGreaterThan(0); - - const searchTerm = "production"; + const targetConnection = searchTestConnections[0]!; + const searchTerm = targetConnection.connection_id; + // Check that we have at least one row before searching (web-first assertion) await connectionsPage.searchConnections(searchTerm); + await expect(connectionsPage.getConnectionRow(searchTerm)).toBeVisible(); await expect - .poll( - async () => { - const ids = await connectionsPage.getConnectionIds(); - - // Verify we have results AND they match the search term - return ids.length > 0 && ids.every((id) => id.toLowerCase().includes(searchTerm.toLowerCase())); - }, - { intervals: [500], timeout: 10_000 }, - ) + .poll(async () => { + const rowTexts = await connectionsPage.connectionRows.allTextContents(); + + return ( + rowTexts.length > 0 && + rowTexts.every((text) => text.toLowerCase().includes(searchTerm.toLowerCase())) + ); + }) .toBe(true); - - const filteredIds = await connectionsPage.getConnectionIds(); - - expect(filteredIds.length).toBeGreaterThan(0); - for (const id of filteredIds) { - expect(id.toLowerCase()).toContain(searchTerm.toLowerCase()); - } }); test("should display all connections when search is cleared", async () => { test.setTimeout(120_000); await connectionsPage.navigate(); + // Verify rows exist before searching (web-first assertion) + await expect(connectionsPage.connectionRows).not.toHaveCount(0); const initialCount = await connectionsPage.getConnectionCount(); - expect(initialCount).toBeGreaterThan(0); - - // Search for something + // Search for something and wait for results await connectionsPage.searchConnections("production"); + await expect(connectionsPage.connectionRows).not.toHaveCount(0); - // Wait for search results - await expect - .poll( - async () => { - const count = await connectionsPage.getConnectionCount(); - - return count > 0; // Just verify we have some results - }, - { intervals: [500], timeout: 10_000 }, - ) - .toBe(true); - - // Clear search + // Clear search and verify at least as many rows as before await connectionsPage.searchConnections(""); + await expect(async () => { + const finalCount = await connectionsPage.getConnectionCount(); - const finalCount = await connectionsPage.getConnectionCount(); - - expect(finalCount).toBeGreaterThanOrEqual(initialCount); + expect(finalCount).toBeGreaterThanOrEqual(initialCount); + }).toPass({ timeout: 10_000 }); }); }); From 4b37635d944bc354eaefc2efad457b4ed6db7705 Mon Sep 17 00:00:00 2001 From: Piyush Date: Sun, 3 May 2026 20:04:41 +0530 Subject: [PATCH 2/4] run lin --- .../src/airflow/ui/tests/e2e/pages/ConnectionsPage.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/airflow-core/src/airflow/ui/tests/e2e/pages/ConnectionsPage.ts b/airflow-core/src/airflow/ui/tests/e2e/pages/ConnectionsPage.ts index 766f92fe8ad8c..1f8a728987949 100644 --- a/airflow-core/src/airflow/ui/tests/e2e/pages/ConnectionsPage.ts +++ b/airflow-core/src/airflow/ui/tests/e2e/pages/ConnectionsPage.ts @@ -267,9 +267,9 @@ export class ConnectionsPage extends BasePage { public async getConnectionIds(): Promise> { const rowLocator = this.connectionRows; - const count = await rowLocator.count(); + const countRow = await rowLocator.count(); - if (count === 0) { + if (countRow === 0) { return []; } From a19923ccbb87b47cf61af578f20dbd96727a5445 Mon Sep 17 00:00:00 2001 From: Piyush Date: Tue, 5 May 2026 15:35:43 +0530 Subject: [PATCH 3/4] run prettier --- .../src/airflow/ui/tests/e2e/pages/ConnectionsPage.ts | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/airflow-core/src/airflow/ui/tests/e2e/pages/ConnectionsPage.ts b/airflow-core/src/airflow/ui/tests/e2e/pages/ConnectionsPage.ts index 1f8a728987949..34e2baef6a2e8 100644 --- a/airflow-core/src/airflow/ui/tests/e2e/pages/ConnectionsPage.ts +++ b/airflow-core/src/airflow/ui/tests/e2e/pages/ConnectionsPage.ts @@ -387,7 +387,10 @@ export class ConnectionsPage extends BasePage { private async findConnectionRow(connectionId: string): Promise { // Try search first (faster) - const hasSearch = await this.searchInput.waitFor({ state: "visible", timeout: 3000 }).then(() => true).catch(() => false); + const hasSearch = await this.searchInput + .waitFor({ state: "visible", timeout: 3000 }) + .then(() => true) + .catch(() => false); if (hasSearch) { return await this.findConnectionRowUsingSearch(connectionId); From 153f565221a40a4ecfa21d4bd1dea489dc5e6f9f Mon Sep 17 00:00:00 2001 From: Piyush Date: Wed, 6 May 2026 11:34:58 +0530 Subject: [PATCH 4/4] simplified delete connection --- .../src/airflow/ui/tests/e2e/pages/ConnectionsPage.ts | 11 ++--------- 1 file changed, 2 insertions(+), 9 deletions(-) diff --git a/airflow-core/src/airflow/ui/tests/e2e/pages/ConnectionsPage.ts b/airflow-core/src/airflow/ui/tests/e2e/pages/ConnectionsPage.ts index 34e2baef6a2e8..67a2668d6052e 100644 --- a/airflow-core/src/airflow/ui/tests/e2e/pages/ConnectionsPage.ts +++ b/airflow-core/src/airflow/ui/tests/e2e/pages/ConnectionsPage.ts @@ -145,18 +145,11 @@ export class ConnectionsPage extends BasePage { await expect(deleteButton).toBeEnabled({ timeout: 5000 }); await deleteButton.click(); - // Wait for the dialog to finish its open animation (data-state="open" is set by - // Ark UI once the transition completes). Without this, the backdrop can cover the - // confirm button during the animation and cause the click to time out. const deleteDialog = this.page.locator('[role="dialog"][data-state="open"]'); - await deleteDialog.waitFor({ state: "visible", timeout: 10_000 }); - const confirmButton = deleteDialog.getByRole("button", { name: "Yes, Delete" }); + await expect(deleteDialog).toBeVisible({ timeout: 10_000 }); - await expect(confirmButton).toBeVisible({ timeout: 5000 }); - - await expect(confirmButton).toBeEnabled({ timeout: 5000 }); - await confirmButton.click(); + await deleteDialog.getByRole("button", { name: "Yes, Delete" }).click(); await expect(this.getConnectionRow(connectionId)).not.toBeVisible({ timeout: 15_000 }); }