vatsrahul1001 commented on code in PR #59381:
URL: https://github.com/apache/airflow/pull/59381#discussion_r2649576070
##########
airflow-core/src/airflow/ui/tests/e2e/specs/task-instances.spec.ts:
##########
@@ -0,0 +1,66 @@
+import { expect, test } from "@playwright/test";
+import { testConfig } from "playwright.config";
+import { LoginPage } from "tests/e2e/pages/LoginPage";
+
+test.describe("Task Instances Page", () => {
+ test.setTimeout(60_000);
+
+ test("should render task instances table and verify details", async ({ page
}) => {
+
+ const loginPage = new LoginPage(page);
+ const testCredentials = testConfig.credentials;
+ const testDagId = testConfig.testDag.id;
+
+ // Login using the Shared Page Object (Fixes Reviewer Comment)
+ await loginPage.navigateAndLogin(testCredentials.username,
testCredentials.password);
+ await loginPage.expectLoginSuccess();
+
+
+ await page.goto("/task_instances");
+
+
+ //render task instances table
+ // Verify Table Rendering
+ const table = page.getByRole("table");
+ await expect(table).toBeVisible({ timeout: 15_000 });
+
+ //Verify Headers
+ const expectedHeaders = [
+ /DAG ID/i,
+ /Task ID/i,
+ /Run ID|Dag Run/i,
+ /State/i,
+ /Start Date/i,
+ /Duration/i,
+ ];
+
+ for (const headerName of expectedHeaders) {
+
+ await expect(page.getByText(headerName).first()).toBeVisible({ timeout:
10_000 });
+ }
+
+
+ //Verify Visual Distinction (Fixes Reviewer Comment)
+ // Check that the "Success" badge has a specific background color
+ const successBadge = page.locator('.state-success,
[data-state="success"]').first();
+
+ if (await successBadge.isVisible()) {
+ await expect(successBadge).toHaveCSS("background-color", /rgb/);
Review Comment:
- isVisible() returns immediately without waiting - test passes even if
badge doesn't exist
- /rgb/ matches any color - doesn't actually verify it's green/success color
- Selectors .state-success, [data-state="success"] may not match actual UI
Use toBeVisible() assertion or verify specific expected color.
##########
airflow-core/src/airflow/ui/tests/e2e/specs/task-instances.spec.ts:
##########
@@ -0,0 +1,66 @@
+import { expect, test } from "@playwright/test";
+import { testConfig } from "playwright.config";
+import { LoginPage } from "tests/e2e/pages/LoginPage";
+
+test.describe("Task Instances Page", () => {
+ test.setTimeout(60_000);
+
+ test("should render task instances table and verify details", async ({ page
}) => {
+
+ const loginPage = new LoginPage(page);
Review Comment:
We do not need to use a login anymore As we have already have auth sharing
implemented
##########
airflow-core/src/airflow/ui/tests/e2e/specs/task-instances.spec.ts:
##########
@@ -0,0 +1,66 @@
+import { expect, test } from "@playwright/test";
+import { testConfig } from "playwright.config";
+import { LoginPage } from "tests/e2e/pages/LoginPage";
+
+test.describe("Task Instances Page", () => {
+ test.setTimeout(60_000);
+
+ test("should render task instances table and verify details", async ({ page
}) => {
Review Comment:
One test does everything: navigation, headers, badge, filtering. Better to
split:
1. should render task instances table
2. should display correct headers
3. should filter by DAG ID
##########
airflow-core/src/airflow/ui/tests/e2e/specs/task-instances.spec.ts:
##########
@@ -0,0 +1,66 @@
+import { expect, test } from "@playwright/test";
+import { testConfig } from "playwright.config";
Review Comment:
The test uses raw page.locator() and page.getByRole() calls. Consider
creating a TaskInstancesPage class like other tests do (DagsPage, LoginPage).
##########
airflow-core/src/airflow/ui/tests/e2e/specs/task-instances.spec.ts:
##########
@@ -0,0 +1,66 @@
+import { expect, test } from "@playwright/test";
+import { testConfig } from "playwright.config";
+import { LoginPage } from "tests/e2e/pages/LoginPage";
+
+test.describe("Task Instances Page", () => {
+ test.setTimeout(60_000);
+
+ test("should render task instances table and verify details", async ({ page
}) => {
+
+ const loginPage = new LoginPage(page);
+ const testCredentials = testConfig.credentials;
+ const testDagId = testConfig.testDag.id;
+
+ // Login using the Shared Page Object (Fixes Reviewer Comment)
+ await loginPage.navigateAndLogin(testCredentials.username,
testCredentials.password);
+ await loginPage.expectLoginSuccess();
+
+
+ await page.goto("/task_instances");
+
+
+ //render task instances table
+ // Verify Table Rendering
+ const table = page.getByRole("table");
+ await expect(table).toBeVisible({ timeout: 15_000 });
+
+ //Verify Headers
+ const expectedHeaders = [
+ /DAG ID/i,
+ /Task ID/i,
+ /Run ID|Dag Run/i,
+ /State/i,
+ /Start Date/i,
+ /Duration/i,
+ ];
+
+ for (const headerName of expectedHeaders) {
+
+ await expect(page.getByText(headerName).first()).toBeVisible({ timeout:
10_000 });
+ }
+
+
+ //Verify Visual Distinction (Fixes Reviewer Comment)
+ // Check that the "Success" badge has a specific background color
+ const successBadge = page.locator('.state-success,
[data-state="success"]').first();
+
+ if (await successBadge.isVisible()) {
+ await expect(successBadge).toHaveCSS("background-color", /rgb/);
+ }
+
+ // 7. Verify Filter Functionality (Fixes Reviewer Comment)
+ // Open Filter -> Select Dag ID -> Type Configured Dag ID -> Verify Row
Appears
+ await page.getByRole("button", { name: /filter/i }).first().click();
+ await page.getByRole("menuitem", { name: "Dag ID" }).click();
+
+ const searchInput = page.getByRole("textbox").first();
+ await expect(searchInput).toBeVisible();
+
+ await searchInput.fill(testDagId);
+ await searchInput.press("Enter");
+
+ // Assert the table actually filtered by finding the DAG ID in a cell
+ await expect(page.getByRole("table")).toBeVisible();
+ await expect(page.getByRole("cell", { name: testDagId
}).first()).toBeVisible();
Review Comment:
This only checks if the DAG ID appears somewhere. It doesn't verify:
- The filter actually reduced the result count
- Only matching rows are shown
--
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]
