vatsrahul1001 commented on code in PR #59791: URL: https://github.com/apache/airflow/pull/59791#discussion_r2649672306
########## airflow-core/src/airflow/ui/tests/e2e/pages/BackfillPage.ts: ########## @@ -0,0 +1,209 @@ +/*! + * 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 } from "@playwright/test"; +import type { Locator, Page } from "@playwright/test"; +import { BasePage } from "tests/e2e/pages/BasePage"; + +export type ReprocessBehavior = "All Runs" | "Missing and Errored Runs" | "Missing Runs"; + +export type CreateBackfillOptions = { + fromDate: string; + reprocessBehavior: ReprocessBehavior; + toDate: string; +}; + +export type VerifyBackfillOptions = { + dagName: string; + expectedFromDate: string; + expectedToDate: string; + reprocessBehavior: ReprocessBehavior; +}; + +export class BackfillPage extends BasePage { + public readonly backfillDateError: Locator; + public readonly backfillFromDateInput: Locator; + public readonly backfillModeRadio: Locator; + public readonly backfillRunButton: Locator; + public readonly backfillsTable: Locator; + public readonly backfillToDateInput: Locator; + public readonly triggerButton: Locator; + + public constructor(page: Page) { + super(page); + this.triggerButton = page.locator('button[aria-label="Trigger Dag"]:has-text("Trigger")'); + this.backfillModeRadio = page.locator('label:has-text("Backfill")'); + this.backfillFromDateInput = page.locator('input[type="datetime-local"]').first(); + this.backfillToDateInput = page.locator('input[type="datetime-local"]').nth(1); + this.backfillRunButton = page.locator('button:has-text("Run Backfill")'); + this.backfillsTable = page.locator("table"); + this.backfillDateError = page.locator('text="Start Date must be before the End Date"'); + } + + public static getBackfillsUrl(dagName: string): string { + return `/dags/${dagName}/backfills`; + } + + public static getDagDetailUrl(dagName: string): string { + return `/dags/${dagName}`; + } + + public async createBackfill(dagName: string, options: CreateBackfillOptions): Promise<void> { + const { fromDate, reprocessBehavior, toDate } = options; + + await this.navigateToDagDetail(dagName); + await this.openBackfillDialog(); + + await this.backfillFromDateInput.fill(fromDate); + await this.backfillToDateInput.fill(toDate); + + await this.selectReprocessBehavior(reprocessBehavior); + + const runsMessage = this.page.locator("text=/\\d+ runs? will be triggered|No runs matching/"); + + await expect(runsMessage).toBeVisible({ timeout: 10_000 }); + + await expect(this.backfillRunButton).toBeEnabled({ timeout: 5000 }); + await this.backfillRunButton.click(); + } + + // Get backfill details from a specific row (default: first row) + public async getBackfillDetails(rowIndex: number = 0): Promise<{ + createdAt: string; + fromDate: string; + reprocessBehavior: string; + toDate: string; + }> { + const row = this.page.locator("table tbody tr").nth(rowIndex); + const cells = row.locator("td"); + + const fromDate = (await cells.nth(0).textContent()) ?? ""; + const toDate = (await cells.nth(1).textContent()) ?? ""; + const reprocessBehavior = (await cells.nth(2).textContent()) ?? ""; + const createdAt = (await cells.nth(3).textContent()) ?? ""; + + return { + createdAt: createdAt.trim(), + fromDate: fromDate.trim(), + reprocessBehavior: reprocessBehavior.trim(), + toDate: toDate.trim(), + }; + } + + public async getBackfillsTableRows(): Promise<number> { + const rows = this.page.locator("table tbody tr"); + + await rows.first().waitFor({ state: "visible", timeout: 10_000 }); + const count = await rows.count(); + + return count; + } + + // Get filter button + public getFilterButton(): Locator { + return this.page.locator('button[aria-label*="filter"], button[aria-label*="Filter"]'); + } + + // Get number of table columns + public async getTableColumnCount(): Promise<number> { + const headers = this.page.locator("table thead th"); + + return await headers.count(); + } + + public async isBackfillDateErrorVisible(): Promise<boolean> { + return this.backfillDateError.isVisible(); + } + + // Check if a specific column is visible in the table + public async isColumnVisible(columnName: string): Promise<boolean> { Review Comment: ColumnVisible() and isFilterAvailable() catch errors and return false. This makes debugging difficult 1. Test failures show unhelpful "Expected: true, Received: false" messages 2. You don't know if the element is missing, selector is wrong, or page didn't load 3. Combined with if conditions, tests can silently pass without testing anything ########## airflow-core/src/airflow/ui/tests/e2e/specs/backfill.spec.ts: ########## @@ -0,0 +1,120 @@ +/*! + * 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 { test, expect } from "@playwright/test"; +import { testConfig } from "playwright.config"; +import { BackfillPage } from "tests/e2e/pages/BackfillPage"; + +const getPastDate = (daysAgo: number): string => { + const date = new Date(); + + date.setDate(date.getDate() - daysAgo); + + return date.toISOString().slice(0, 16); +}; + +// Backfills E2E Tests + +test.describe("Backfills List Display", () => { + let backfillPage: BackfillPage; + + const testDagId = testConfig.testDag.id; + + test.beforeEach(async ({ page }) => { + backfillPage = new BackfillPage(page); + + const fromDate = getPastDate(2); + const toDate = getPastDate(1); + + await backfillPage.createBackfill(testDagId, { + fromDate, + reprocessBehavior: "All Runs", + toDate, + }); + + await backfillPage.navigateToBackfillsTab(testDagId); + }); + + test("should Verify backfills list displays (or empty state if none)", async () => { + const rowsCount = await backfillPage.getBackfillsTableRows(); + + if (rowsCount > 0) { + await expect(backfillPage.backfillsTable).toBeVisible(); + expect(rowsCount).toBeGreaterThanOrEqual(1); + } else { + await expect(backfillPage.page.locator('text="No backfills found"')).toBeVisible(); + } + }); + + test("Verify backfill details display: date range, status, created time", async () => { + const backfillDetails = await backfillPage.getBackfillDetails(0); + + expect(backfillDetails.fromDate).toBeTruthy(); + expect(backfillDetails.toDate).toBeTruthy(); + + expect(backfillDetails.reprocessBehavior).toBeTruthy(); + expect(["All Runs", "Missing Runs", "Missing and Errored Runs"]).toContain( + backfillDetails.reprocessBehavior, + ); + + expect(backfillDetails.createdAt).toBeTruthy(); + }); + + test("should verify Table filters", async () => { + const initialColumnCount = await backfillPage.getTableColumnCount(); + + expect(initialColumnCount).toBeGreaterThan(0); + + const isFilterAvailable = await backfillPage.isFilterAvailable(); + + expect(isFilterAvailable).toBe(true); + + await backfillPage.openFilterMenu(); + + const columnToToggle = "Duration"; + const isInitiallyVisible = await backfillPage.isColumnVisible(columnToToggle); + + if (isInitiallyVisible) { Review Comment: If Duration column doesn't exist, the entire test passes without testing anything. ########## airflow-core/src/airflow/ui/tests/e2e/specs/backfill.spec.ts: ########## @@ -0,0 +1,120 @@ +/*! + * 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 { test, expect } from "@playwright/test"; +import { testConfig } from "playwright.config"; +import { BackfillPage } from "tests/e2e/pages/BackfillPage"; + +const getPastDate = (daysAgo: number): string => { + const date = new Date(); + + date.setDate(date.getDate() - daysAgo); + + return date.toISOString().slice(0, 16); +}; + +// Backfills E2E Tests + +test.describe("Backfills List Display", () => { + let backfillPage: BackfillPage; + + const testDagId = testConfig.testDag.id; + + test.beforeEach(async ({ page }) => { Review Comment: Creates a new backfill before EVERY test (3 backfills total) Can we use beforeAll to create once, or check if backfill exists first. ########## airflow-core/src/airflow/ui/tests/e2e/specs/backfill.spec.ts: ########## @@ -0,0 +1,120 @@ +/*! + * 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 { test, expect } from "@playwright/test"; +import { testConfig } from "playwright.config"; +import { BackfillPage } from "tests/e2e/pages/BackfillPage"; + +const getPastDate = (daysAgo: number): string => { + const date = new Date(); + + date.setDate(date.getDate() - daysAgo); + + return date.toISOString().slice(0, 16); +}; + +// Backfills E2E Tests + +test.describe("Backfills List Display", () => { + let backfillPage: BackfillPage; + + const testDagId = testConfig.testDag.id; + + test.beforeEach(async ({ page }) => { + backfillPage = new BackfillPage(page); + + const fromDate = getPastDate(2); + const toDate = getPastDate(1); + + await backfillPage.createBackfill(testDagId, { + fromDate, + reprocessBehavior: "All Runs", + toDate, + }); + + await backfillPage.navigateToBackfillsTab(testDagId); + }); + + test("should Verify backfills list displays (or empty state if none)", async () => { Review Comment: Better will be `should verify backfills list displays` ########## airflow-core/src/airflow/ui/tests/e2e/specs/backfill.spec.ts: ########## @@ -0,0 +1,120 @@ +/*! + * 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 { test, expect } from "@playwright/test"; +import { testConfig } from "playwright.config"; +import { BackfillPage } from "tests/e2e/pages/BackfillPage"; + +const getPastDate = (daysAgo: number): string => { + const date = new Date(); + + date.setDate(date.getDate() - daysAgo); + + return date.toISOString().slice(0, 16); +}; + +// Backfills E2E Tests + +test.describe("Backfills List Display", () => { + let backfillPage: BackfillPage; + + const testDagId = testConfig.testDag.id; + + test.beforeEach(async ({ page }) => { + backfillPage = new BackfillPage(page); + + const fromDate = getPastDate(2); + const toDate = getPastDate(1); + + await backfillPage.createBackfill(testDagId, { + fromDate, + reprocessBehavior: "All Runs", + toDate, + }); + + await backfillPage.navigateToBackfillsTab(testDagId); + }); + + test("should Verify backfills list displays (or empty state if none)", async () => { + const rowsCount = await backfillPage.getBackfillsTableRows(); + + if (rowsCount > 0) { + await expect(backfillPage.backfillsTable).toBeVisible(); + expect(rowsCount).toBeGreaterThanOrEqual(1); + } else { + await expect(backfillPage.page.locator('text="No backfills found"')).toBeVisible(); + } + }); + + test("Verify backfill details display: date range, status, created time", async () => { + const backfillDetails = await backfillPage.getBackfillDetails(0); + + expect(backfillDetails.fromDate).toBeTruthy(); + expect(backfillDetails.toDate).toBeTruthy(); + + expect(backfillDetails.reprocessBehavior).toBeTruthy(); + expect(["All Runs", "Missing Runs", "Missing and Errored Runs"]).toContain( + backfillDetails.reprocessBehavior, + ); + + expect(backfillDetails.createdAt).toBeTruthy(); + }); + + test("should verify Table filters", async () => { + const initialColumnCount = await backfillPage.getTableColumnCount(); + + expect(initialColumnCount).toBeGreaterThan(0); + + const isFilterAvailable = await backfillPage.isFilterAvailable(); + + expect(isFilterAvailable).toBe(true); + + await backfillPage.openFilterMenu(); + + const columnToToggle = "Duration"; + const isInitiallyVisible = await backfillPage.isColumnVisible(columnToToggle); + + if (isInitiallyVisible) { + await backfillPage.toggleColumn(columnToToggle); + + await backfillPage.page.keyboard.press("Escape"); + await backfillPage.page.waitForTimeout(500); Review Comment: Instead of harcoding wait, we can Wait for specific conditions instead. ########## airflow-core/src/airflow/ui/tests/e2e/specs/backfill.spec.ts: ########## @@ -0,0 +1,120 @@ +/*! + * 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 { test, expect } from "@playwright/test"; +import { testConfig } from "playwright.config"; +import { BackfillPage } from "tests/e2e/pages/BackfillPage"; + +const getPastDate = (daysAgo: number): string => { + const date = new Date(); + + date.setDate(date.getDate() - daysAgo); + + return date.toISOString().slice(0, 16); +}; + +// Backfills E2E Tests + +test.describe("Backfills List Display", () => { + let backfillPage: BackfillPage; + + const testDagId = testConfig.testDag.id; + + test.beforeEach(async ({ page }) => { + backfillPage = new BackfillPage(page); + + const fromDate = getPastDate(2); + const toDate = getPastDate(1); + + await backfillPage.createBackfill(testDagId, { + fromDate, + reprocessBehavior: "All Runs", + toDate, + }); + + await backfillPage.navigateToBackfillsTab(testDagId); + }); + + test("should Verify backfills list displays (or empty state if none)", async () => { + const rowsCount = await backfillPage.getBackfillsTableRows(); + + if (rowsCount > 0) { Review Comment: Since beforeEach creates a backfill, rowsCount should always be > 0. If it's not, that's a bug that should fail, not pass with "empty state" check. ########## airflow-core/src/airflow/ui/tests/e2e/pages/BackfillPage.ts: ########## @@ -0,0 +1,209 @@ +/*! + * 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 } from "@playwright/test"; +import type { Locator, Page } from "@playwright/test"; +import { BasePage } from "tests/e2e/pages/BasePage"; + +export type ReprocessBehavior = "All Runs" | "Missing and Errored Runs" | "Missing Runs"; + +export type CreateBackfillOptions = { + fromDate: string; + reprocessBehavior: ReprocessBehavior; + toDate: string; +}; + +export type VerifyBackfillOptions = { + dagName: string; + expectedFromDate: string; + expectedToDate: string; + reprocessBehavior: ReprocessBehavior; +}; + +export class BackfillPage extends BasePage { + public readonly backfillDateError: Locator; + public readonly backfillFromDateInput: Locator; + public readonly backfillModeRadio: Locator; + public readonly backfillRunButton: Locator; + public readonly backfillsTable: Locator; + public readonly backfillToDateInput: Locator; + public readonly triggerButton: Locator; + + public constructor(page: Page) { + super(page); + this.triggerButton = page.locator('button[aria-label="Trigger Dag"]:has-text("Trigger")'); + this.backfillModeRadio = page.locator('label:has-text("Backfill")'); + this.backfillFromDateInput = page.locator('input[type="datetime-local"]').first(); + this.backfillToDateInput = page.locator('input[type="datetime-local"]').nth(1); + this.backfillRunButton = page.locator('button:has-text("Run Backfill")'); + this.backfillsTable = page.locator("table"); + this.backfillDateError = page.locator('text="Start Date must be before the End Date"'); + } + + public static getBackfillsUrl(dagName: string): string { + return `/dags/${dagName}/backfills`; + } + + public static getDagDetailUrl(dagName: string): string { + return `/dags/${dagName}`; + } + + public async createBackfill(dagName: string, options: CreateBackfillOptions): Promise<void> { + const { fromDate, reprocessBehavior, toDate } = options; + + await this.navigateToDagDetail(dagName); + await this.openBackfillDialog(); + + await this.backfillFromDateInput.fill(fromDate); + await this.backfillToDateInput.fill(toDate); + + await this.selectReprocessBehavior(reprocessBehavior); + + const runsMessage = this.page.locator("text=/\\d+ runs? will be triggered|No runs matching/"); + + await expect(runsMessage).toBeVisible({ timeout: 10_000 }); + + await expect(this.backfillRunButton).toBeEnabled({ timeout: 5000 }); + await this.backfillRunButton.click(); + } + + // Get backfill details from a specific row (default: first row) + public async getBackfillDetails(rowIndex: number = 0): Promise<{ + createdAt: string; + fromDate: string; + reprocessBehavior: string; + toDate: string; + }> { + const row = this.page.locator("table tbody tr").nth(rowIndex); + const cells = row.locator("td"); + + const fromDate = (await cells.nth(0).textContent()) ?? ""; Review Comment: Can we have a better way instead of assuming column 0 is fromDate? If in code order changes this will break -- 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]
