This is an automated email from the ASF dual-hosted git repository.
choo121600 pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/airflow.git
The following commit(s) were added to refs/heads/main by this push:
new b29e61f83e4 Fix Playwright anti-pattern in AssetListPage (#63084)
b29e61f83e4 is described below
commit b29e61f83e4d98d4b61cb422fb8120f90685aff8
Author: Shubham Gondane <[email protected]>
AuthorDate: Sun Mar 8 22:43:59 2026 -0700
Fix Playwright anti-pattern in AssetListPage (#63084)
* Fix Playwright anti-pattern in AssetListPage waitForTableData
* Replace manual DOM assertions with Playwright web-first assertions in
asset tests
* Remove dead methods and redundant assertions from asset tests
---
.../airflow/ui/tests/e2e/pages/AssetDetailPage.ts | 28 ++++++++++------------
.../airflow/ui/tests/e2e/pages/AssetListPage.ts | 28 ++--------------------
.../src/airflow/ui/tests/e2e/specs/asset.spec.ts | 26 ++++----------------
3 files changed, 20 insertions(+), 62 deletions(-)
diff --git a/airflow-core/src/airflow/ui/tests/e2e/pages/AssetDetailPage.ts
b/airflow-core/src/airflow/ui/tests/e2e/pages/AssetDetailPage.ts
index a649d067e14..964774e6a80 100644
--- a/airflow-core/src/airflow/ui/tests/e2e/pages/AssetDetailPage.ts
+++ b/airflow-core/src/airflow/ui/tests/e2e/pages/AssetDetailPage.ts
@@ -41,19 +41,19 @@ export class AssetDetailPage extends BasePage {
await expect(this.page.getByRole("heading", { name })).toBeVisible();
}
- public async verifyProducingTasks(minCount: number): Promise<void> {
- await this.verifyStatSection("Producing Tasks", minCount);
+ public async verifyProducingTasks(): Promise<void> {
+ await this.verifyStatSection("Producing Tasks");
}
- public async verifyScheduledDags(minCount: number): Promise<void> {
- await this.verifyStatSection("Scheduled Dags", minCount);
+ public async verifyScheduledDags(): Promise<void> {
+ await this.verifyStatSection("Scheduled Dags");
}
/**
* Common helper to verify stat sections (Producing Tasks, Scheduled Dags)
* Uses stable selectors based on text content and ARIA roles
*/
- private async verifyStatSection(labelText: string, minCount: number):
Promise<void> {
+ private async verifyStatSection(labelText: string): Promise<void> {
const label = this.page.getByText(labelText, { exact: true });
await expect(label).toBeVisible();
@@ -64,19 +64,17 @@ export class AssetDetailPage extends BasePage {
const button = statContainer.getByRole("button").first();
await expect(button).toBeVisible();
+ await expect(button).toHaveText(/^[1-9]/);
+
const text = await button.textContent();
const count = parseInt(text?.split(" ")[0] ?? "0", 10);
- expect(count).toBeGreaterThanOrEqual(minCount);
-
- if (count > 0) {
- await button.click();
- await expect(button).toHaveAttribute("aria-expanded", "true", { timeout:
5000 });
- const popoverLinks =
this.page.getByRole("dialog").last().getByRole("link");
+ await button.click();
+ await expect(button).toHaveAttribute("aria-expanded", "true", { timeout:
5000 });
+ const popoverLinks =
this.page.getByRole("dialog").last().getByRole("link");
- await expect(popoverLinks).toHaveCount(count);
- await button.click();
- await expect(button).toHaveAttribute("aria-expanded", "false", {
timeout: 5000 });
- }
+ await expect(popoverLinks).toHaveCount(count);
+ await button.click();
+ await expect(button).toHaveAttribute("aria-expanded", "false", { timeout:
5000 });
}
}
diff --git a/airflow-core/src/airflow/ui/tests/e2e/pages/AssetListPage.ts
b/airflow-core/src/airflow/ui/tests/e2e/pages/AssetListPage.ts
index dd6c30c753b..2b9ab8ee6e0 100644
--- a/airflow-core/src/airflow/ui/tests/e2e/pages/AssetListPage.ts
+++ b/airflow-core/src/airflow/ui/tests/e2e/pages/AssetListPage.ts
@@ -16,7 +16,7 @@
* specific language governing permissions and limitations
* under the License.
*/
-import type { Locator, Page } from "@playwright/test";
+import { expect, type Locator, type Page } from "@playwright/test";
import { BasePage } from "./BasePage";
@@ -42,14 +42,6 @@ export class AssetListPage extends BasePage {
this.emptyState = page.getByText(/no items/i);
}
- public async assetCount(): Promise<number> {
- return this.rows.count();
- }
-
- public async assetNames(): Promise<Array<string>> {
- return this.rows.locator("td a").allTextContents();
- }
-
public async navigate(): Promise<void> {
await this.navigateTo("/assets");
}
@@ -80,22 +72,6 @@ export class AssetListPage extends BasePage {
}
private async waitForTableData(): Promise<void> {
- // Wait for actual data links to appear (not skeleton loaders)
- await this.page.waitForFunction(
- () => {
- const table = document.querySelector('[data-testid="table-list"]');
-
- if (!table) {
- return false;
- }
-
- // Check for actual links in tbody (real data, not skeleton)
- const links = table.querySelectorAll("tbody tr td a");
-
- return links.length > 0;
- },
- undefined,
- { timeout: 30_000 },
- );
+ await expect(this.rows.locator("td a").first()).toBeVisible({ timeout:
30_000 });
}
}
diff --git a/airflow-core/src/airflow/ui/tests/e2e/specs/asset.spec.ts
b/airflow-core/src/airflow/ui/tests/e2e/specs/asset.spec.ts
index be99eebab23..59046947c57 100644
--- a/airflow-core/src/airflow/ui/tests/e2e/specs/asset.spec.ts
+++ b/airflow-core/src/airflow/ui/tests/e2e/specs/asset.spec.ts
@@ -64,17 +64,11 @@ test.describe("Assets Page", () => {
});
test("verify asset rows when data exists", async () => {
- const count = await assets.assetCount();
-
- expect(count).toBeGreaterThanOrEqual(0);
+ await expect(assets.rows.first()).toBeVisible();
});
test("verify asset has a visible name link", async () => {
- const names = await assets.assetNames();
-
- for (const name of names) {
- expect(name.trim().length).toBeGreaterThan(0);
- }
+ await expect(assets.rows.locator("td a").first()).toBeVisible();
});
test("verify clicking an asset navigates to detail page", async ({ page })
=> {
@@ -85,9 +79,7 @@ test.describe("Assets Page", () => {
});
test("verify assets using search", async () => {
- const initialCount = await assets.assetCount();
-
- expect(initialCount).toBeGreaterThan(0);
+ await expect(assets.rows.first()).toBeVisible();
const searchTerm = testConfig.asset.name;
@@ -107,14 +99,6 @@ test.describe("Assets Page", () => {
{ intervals: [500], timeout: 30_000 },
)
.toBe(true);
-
- const names = await assets.assetNames();
-
- expect(names.length).toBeGreaterThan(0);
-
- for (const name of names) {
- expect(name.toLowerCase()).toContain(searchTerm.toLowerCase());
- }
});
test("verify asset details and dependencies", async ({ page }) => {
@@ -127,8 +111,8 @@ test.describe("Assets Page", () => {
await assetDetailPage.verifyAssetDetails(assetName);
- await assetDetailPage.verifyProducingTasks(1);
+ await assetDetailPage.verifyProducingTasks();
- await assetDetailPage.verifyScheduledDags(1);
+ await assetDetailPage.verifyScheduledDags();
});
});
