vatsrahul1001 commented on code in PR #59678: URL: https://github.com/apache/airflow/pull/59678#discussion_r2649639798
########## airflow-core/src/airflow/ui/tests/e2e/pages/HomePage.ts: ########## @@ -0,0 +1,181 @@ +/*! + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +import type { Locator, Page } from "@playwright/test"; +import { BasePage } from "tests/e2e/pages/BasePage"; + +/** + * Home/Dashboard Page Object + */ +export class HomePage extends BasePage { + // Page URLs + public static get homeUrl(): string { + return "/"; + } + + public readonly activeDagsCard: Locator; + public readonly dagImportErrorsCard: Locator; + public readonly dagProcessorHealth: Locator; + public readonly dagRunMetrics: Locator; + public readonly failedDagsCard: Locator; + + // Health section elements + public readonly healthSection: Locator; + // Historical Metrics section (recent runs) + public readonly historicalMetricsSection: Locator; + public readonly metaDatabaseHealth: Locator; + // Pool Summary section + public readonly poolSummarySection: Locator; + public readonly runningDagsCard: Locator; + + public readonly schedulerHealth: Locator; + + // Dashboard Stats elements + public readonly statsSection: Locator; + public readonly taskInstanceMetrics: Locator; + public readonly triggererHealth: Locator; + + public constructor(page: Page) { + super(page); + + // Stats section - using the heading text + this.statsSection = page.locator('text="Stats"').locator("..").locator(".."); + + // Stats cards - using link patterns that match the StatsCard component + this.failedDagsCard = page.locator('a[href*="last_dag_run_state=failed"]'); + this.runningDagsCard = page.locator('a[href*="last_dag_run_state=running"]'); + this.activeDagsCard = page.locator('a[href*="paused=false"]'); + this.dagImportErrorsCard = page.locator('button:has-text("DAG Import Errors")'); + + // Health section - using text content matching + this.healthSection = page.locator('text="Health"').locator("..").locator(".."); + this.metaDatabaseHealth = page.getByText("Metadatabase").first(); + this.schedulerHealth = page.getByText("Scheduler").first(); + this.triggererHealth = page.getByText("Triggerer").first(); + this.dagProcessorHealth = page.getByText("DAG Processor").first(); + + // Pool Summary section + this.poolSummarySection = page.locator('text="Pool Summary"').locator("..").locator(".."); + + // Historical Metrics section (recent runs) + this.historicalMetricsSection = page.getByText("History").first().locator("..").locator(".."); + this.dagRunMetrics = page.getByRole("heading", { name: /dag run/i }).first(); + this.taskInstanceMetrics = page.getByRole("heading", { name: /task instance/i }).first(); + } + + /** + * Check if health badges are visible + */ + public async areHealthBadgesVisible(): Promise<boolean> { + try { + await this.metaDatabaseHealth.waitFor({ state: "visible", timeout: 10_000 }); + await this.schedulerHealth.waitFor({ state: "visible", timeout: 5000 }); + + return true; + } catch { + return false; + } + } + + /** + * Get Active DAGs count + */ + public async getActiveDagsCount(): Promise<number> { + return this.getStatsCardCount(this.activeDagsCard); + } + + /** + * Get Failed DAGs count + */ + public async getFailedDagsCount(): Promise<number> { + return this.getStatsCardCount(this.failedDagsCard); + } + + /** + * Get Running DAGs count + */ + public async getRunningDagsCount(): Promise<number> { + return this.getStatsCardCount(this.runningDagsCard); + } + + /** + * Check if DAG Import Errors are displayed (only visible when errors exist) + */ + public async isDagImportErrorsVisible(): Promise<boolean> { + try { + return await this.dagImportErrorsCard.isVisible(); + } catch { + return false; + } + } + + /** + * Check if historical metrics (recent runs) section is visible + */ + public async isHistoricalMetricsSectionVisible(): Promise<boolean> { + try { + await this.dagRunMetrics.waitFor({ state: "visible", timeout: 10_000 }); + + return true; + } catch { + return false; + } + } + + /** + * Check if stats section is visible + */ + public async isStatsSectionVisible(): Promise<boolean> { Review Comment: Instead of creating own func. I think we should use Playwright assertions directly for clearer error messages. This will fail silently ########## airflow-core/src/airflow/ui/tests/e2e/specs/home-dashboard.spec.ts: ########## @@ -0,0 +1,147 @@ +/*! + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +import { expect, test } from "@playwright/test"; +import { HomePage } from "tests/e2e/pages/HomePage"; + +test.describe("Dashboard Metrics Display", () => { + let homePage: HomePage; + + test.beforeEach(({ page }) => { + homePage = new HomePage(page); + }); + + test("should display dashboard stats section with DAG metrics", async () => { + await homePage.navigate(); + await homePage.waitForDashboardLoad(); + + const isStatsVisible = await homePage.isStatsSectionVisible(); + + expect(isStatsVisible).toBe(true); + + await expect(homePage.activeDagsCard).toBeVisible(); + const activeDagsCount = await homePage.getActiveDagsCount(); + + expect(activeDagsCount).toBeGreaterThanOrEqual(0); + + await expect(homePage.runningDagsCard).toBeVisible(); + const runningDagsCount = await homePage.getRunningDagsCount(); + + expect(runningDagsCount).toBeGreaterThanOrEqual(0); + + await expect(homePage.failedDagsCard).toBeVisible(); + const failedDagsCount = await homePage.getFailedDagsCount(); + + expect(failedDagsCount).toBeGreaterThanOrEqual(0); + }); + + test("should display health status badges", async () => { + await homePage.navigate(); + await homePage.waitForDashboardLoad(); + + const areHealthBadgesVisible = await homePage.areHealthBadgesVisible(); + + expect(areHealthBadgesVisible).toBe(true); + + await expect(homePage.metaDatabaseHealth).toBeVisible(); + await expect(homePage.schedulerHealth).toBeVisible(); + await expect(homePage.triggererHealth).toBeVisible(); + }); + + test("should navigate to filtered DAGs list when clicking stats cards", async () => { + await homePage.navigate(); + await homePage.waitForDashboardLoad(); + + await homePage.activeDagsCard.click(); + await homePage.page.waitForURL(/paused=false/); + + expect(homePage.page.url()).toContain("paused=false"); + + await homePage.navigate(); + await homePage.waitForDashboardLoad(); + + await homePage.runningDagsCard.click(); + await homePage.page.waitForURL(/last_dag_run_state=running/); + + expect(homePage.page.url()).toContain("last_dag_run_state=running"); + }); + + test("should display welcome heading on dashboard", async () => { + await homePage.navigate(); + await homePage.waitForDashboardLoad(); + + await expect(homePage.welcomeHeading).toBeVisible(); + }); + + test("should refresh and display updated metrics on page reload", async () => { + await homePage.navigate(); + await homePage.waitForDashboardLoad(); + + const initialActiveCount = await homePage.getActiveDagsCount(); + const initialRunningCount = await homePage.getRunningDagsCount(); + const initialFailedCount = await homePage.getFailedDagsCount(); + + expect(initialActiveCount).toBeGreaterThanOrEqual(0); + expect(initialRunningCount).toBeGreaterThanOrEqual(0); + expect(initialFailedCount).toBeGreaterThanOrEqual(0); + + await homePage.page.reload(); + await homePage.waitForDashboardLoad(); + + const isStatsVisible = await homePage.isStatsSectionVisible(); + + expect(isStatsVisible).toBe(true); + + const reloadedActiveCount = await homePage.getActiveDagsCount(); + const reloadedRunningCount = await homePage.getRunningDagsCount(); + const reloadedFailedCount = await homePage.getFailedDagsCount(); + + expect(reloadedActiveCount).toBeGreaterThanOrEqual(0); + expect(reloadedRunningCount).toBeGreaterThanOrEqual(0); + expect(reloadedFailedCount).toBeGreaterThanOrEqual(0); + }); + + test("should display historical metrics section with recent runs", async () => { + await homePage.navigate(); + await homePage.waitForDashboardLoad(); + + const isHistoricalMetricsVisible = await homePage.isHistoricalMetricsSectionVisible(); + + expect(isHistoricalMetricsVisible).toBe(true); + + await expect(homePage.dagRunMetrics).toBeVisible(); + await expect(homePage.taskInstanceMetrics).toBeVisible(); + }); + + test("should handle DAG import errors display when errors exist", async () => { + await homePage.navigate(); + await homePage.waitForDashboardLoad(); + + // DAG Import Errors button only appears when there are actual import errors + const isDagImportErrorsVisible = await homePage.isDagImportErrorsVisible(); + + if (isDagImportErrorsVisible) { Review Comment: If no import errors exist, the assertion is skipped. Consider using test.skip() with a clear message instead. ########## airflow-core/src/airflow/ui/tests/e2e/pages/HomePage.ts: ########## @@ -0,0 +1,181 @@ +/*! + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +import type { Locator, Page } from "@playwright/test"; +import { BasePage } from "tests/e2e/pages/BasePage"; + +/** + * Home/Dashboard Page Object + */ +export class HomePage extends BasePage { + // Page URLs + public static get homeUrl(): string { + return "/"; + } + + public readonly activeDagsCard: Locator; + public readonly dagImportErrorsCard: Locator; + public readonly dagProcessorHealth: Locator; + public readonly dagRunMetrics: Locator; + public readonly failedDagsCard: Locator; + + // Health section elements + public readonly healthSection: Locator; + // Historical Metrics section (recent runs) + public readonly historicalMetricsSection: Locator; + public readonly metaDatabaseHealth: Locator; + // Pool Summary section + public readonly poolSummarySection: Locator; + public readonly runningDagsCard: Locator; + + public readonly schedulerHealth: Locator; + + // Dashboard Stats elements + public readonly statsSection: Locator; + public readonly taskInstanceMetrics: Locator; + public readonly triggererHealth: Locator; + + public constructor(page: Page) { + super(page); + + // Stats section - using the heading text + this.statsSection = page.locator('text="Stats"').locator("..").locator(".."); + + // Stats cards - using link patterns that match the StatsCard component + this.failedDagsCard = page.locator('a[href*="last_dag_run_state=failed"]'); + this.runningDagsCard = page.locator('a[href*="last_dag_run_state=running"]'); + this.activeDagsCard = page.locator('a[href*="paused=false"]'); + this.dagImportErrorsCard = page.locator('button:has-text("DAG Import Errors")'); + + // Health section - using text content matching + this.healthSection = page.locator('text="Health"').locator("..").locator(".."); + this.metaDatabaseHealth = page.getByText("Metadatabase").first(); + this.schedulerHealth = page.getByText("Scheduler").first(); + this.triggererHealth = page.getByText("Triggerer").first(); + this.dagProcessorHealth = page.getByText("DAG Processor").first(); + + // Pool Summary section + this.poolSummarySection = page.locator('text="Pool Summary"').locator("..").locator(".."); + + // Historical Metrics section (recent runs) + this.historicalMetricsSection = page.getByText("History").first().locator("..").locator(".."); + this.dagRunMetrics = page.getByRole("heading", { name: /dag run/i }).first(); + this.taskInstanceMetrics = page.getByRole("heading", { name: /task instance/i }).first(); + } + + /** + * Check if health badges are visible + */ + public async areHealthBadgesVisible(): Promise<boolean> { + try { + await this.metaDatabaseHealth.waitFor({ state: "visible", timeout: 10_000 }); + await this.schedulerHealth.waitFor({ state: "visible", timeout: 5000 }); + + return true; + } catch { + return false; + } + } + + /** + * Get Active DAGs count + */ + public async getActiveDagsCount(): Promise<number> { + return this.getStatsCardCount(this.activeDagsCard); + } + + /** + * Get Failed DAGs count + */ + public async getFailedDagsCount(): Promise<number> { + return this.getStatsCardCount(this.failedDagsCard); + } + + /** + * Get Running DAGs count + */ + public async getRunningDagsCount(): Promise<number> { + return this.getStatsCardCount(this.runningDagsCard); + } + + /** + * Check if DAG Import Errors are displayed (only visible when errors exist) + */ + public async isDagImportErrorsVisible(): Promise<boolean> { + try { + return await this.dagImportErrorsCard.isVisible(); + } catch { + return false; + } + } + + /** + * Check if historical metrics (recent runs) section is visible + */ + public async isHistoricalMetricsSectionVisible(): Promise<boolean> { + try { + await this.dagRunMetrics.waitFor({ state: "visible", timeout: 10_000 }); + + return true; + } catch { + return false; + } + } + + /** + * Check if stats section is visible + */ + public async isStatsSectionVisible(): Promise<boolean> { + try { + await this.activeDagsCard.waitFor({ state: "visible", timeout: 10_000 }); + + return true; + } catch { + return false; + } + } + + /** + * Navigate to Home/Dashboard page + */ + public async navigate(): Promise<void> { + await this.navigateTo(HomePage.homeUrl); + } + + /** + * Wait for dashboard to fully load + */ + public async waitForDashboardLoad(): Promise<void> { + await this.welcomeHeading.waitFor({ state: "visible", timeout: 30_000 }); + } + + /** + * Get the count from a stats card + */ + // eslint-disable-next-line @typescript-eslint/class-methods-use-this + private async getStatsCardCount(card: Locator): Promise<number> { Review Comment: If the card doesn't exist, it returns 0, and expect(count).toBeGreaterThanOrEqual(0) still passes. The test never verifies the element actually exists. ```suggestion private async getStatsCardCount(card: Locator): Promise<number> { await card.waitFor({ state: "visible" }); // Fail fast if card doesn't exist const badgeText = await card.locator("span").first().textContent(); const match = badgeText?.match(/\d+/); if (!match) { throw new Error("Could not find count in stats card"); } return parseInt(match[0], 10); } ``` ########## airflow-core/src/airflow/ui/tests/e2e/pages/HomePage.ts: ########## @@ -0,0 +1,181 @@ +/*! + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +import type { Locator, Page } from "@playwright/test"; +import { BasePage } from "tests/e2e/pages/BasePage"; + +/** + * Home/Dashboard Page Object + */ +export class HomePage extends BasePage { + // Page URLs + public static get homeUrl(): string { + return "/"; + } + + public readonly activeDagsCard: Locator; + public readonly dagImportErrorsCard: Locator; + public readonly dagProcessorHealth: Locator; + public readonly dagRunMetrics: Locator; + public readonly failedDagsCard: Locator; + + // Health section elements + public readonly healthSection: Locator; + // Historical Metrics section (recent runs) + public readonly historicalMetricsSection: Locator; + public readonly metaDatabaseHealth: Locator; + // Pool Summary section + public readonly poolSummarySection: Locator; + public readonly runningDagsCard: Locator; + + public readonly schedulerHealth: Locator; + + // Dashboard Stats elements + public readonly statsSection: Locator; + public readonly taskInstanceMetrics: Locator; + public readonly triggererHealth: Locator; + + public constructor(page: Page) { + super(page); + + // Stats section - using the heading text + this.statsSection = page.locator('text="Stats"').locator("..").locator(".."); Review Comment: Using .. to navigate parents is fragile. Consider more stable selectors. -- 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]
