vatsrahul1001 commented on code in PR #59738:
URL: https://github.com/apache/airflow/pull/59738#discussion_r2664809002
##########
airflow-core/src/airflow/ui/tests/e2e/pages/BackfillPage.ts:
##########
@@ -340,6 +369,10 @@ export class BackfillPage extends BasePage {
await menuItem.click();
}
+ public async waitForBackfillCompletion(timeout: number = 30_000):
Promise<void> {
Review Comment:
You can rebase and remove this method. We already have
`waitForNoActiveBackfill` in main
##########
airflow-core/src/airflow/ui/tests/e2e/pages/BackfillPage.ts:
##########
@@ -80,6 +83,9 @@ export class BackfillPage extends BasePage {
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"');
+ this.backfillBanner = page.locator('div:has-text("Backfill in progress")');
+ this.cancelButton = page.locator('button[aria-label*="ancel"]');
Review Comment:
Do we need partial selectors here?
##########
airflow-core/src/airflow/ui/tests/e2e/pages/BackfillPage.ts:
##########
@@ -290,6 +308,17 @@ export class BackfillPage extends BasePage {
return await headers.count();
}
+ public async isBackfillDateErrorVisible(): Promise<boolean> {
Review Comment:
Lets remove this its not being used in tests. Also if needed we can use
playwright isVisible() method
##########
airflow-core/src/airflow/ui/tests/e2e/specs/backfill-controls.spec.ts:
##########
@@ -0,0 +1,121 @@
+/*!
+ * 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 { testConfig, AUTH_FILE } from "playwright.config";
+import { BackfillPage } from "tests/e2e/pages/BackfillPage";
+
+const getPastDate = (daysAgo: number): string => {
+ const date = new Date();
+
+ date.setDate(date.getDate() - daysAgo);
+ date.setHours(0, 0, 0, 0);
+
+ return date.toISOString().slice(0, 16);
+};
+
+test.describe("Backfill Controls", () => {
Review Comment:
We can use beforeEach here instead of triggering backfill code in every test
```
test.describe("Backfill controls", () => {
test.describe.configure({ mode: "serial" }); // Prevents parallel conflicts
let backfillPage: BackfillPage;
const controlFromDate = getPastDate(15);
const controlToDate = getPastDate(14);
test.beforeEach(async ({ page }) => {
backfillPage = new BackfillPage(page);
await backfillPage.createBackfill(testDagId, {
fromDate: controlFromDate,
reprocessBehavior: "All Runs",
toDate: controlToDate,
});
await backfillPage.navigateToBackfillsTab(testDagId);
});
```
##########
airflow-core/src/airflow/ui/tests/e2e/specs/backfill-controls.spec.ts:
##########
@@ -0,0 +1,121 @@
+/*!
+ * Licensed to the Apache Software Foundation (ASF) under one
Review Comment:
We should add these test in backfill.spec.ts
##########
airflow-core/src/airflow/ui/tests/e2e/specs/backfill-controls.spec.ts:
##########
@@ -0,0 +1,121 @@
+/*!
+ * 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 { testConfig, AUTH_FILE } from "playwright.config";
+import { BackfillPage } from "tests/e2e/pages/BackfillPage";
+
+const getPastDate = (daysAgo: number): string => {
+ const date = new Date();
+
+ date.setDate(date.getDate() - daysAgo);
+ date.setHours(0, 0, 0, 0);
+
+ return date.toISOString().slice(0, 16);
+};
+
+test.describe("Backfill Controls", () => {
+ const testDagId = testConfig.testDag.id;
+ const fromDate = getPastDate(2);
+ const toDate = getPastDate(1);
+
+ test("should pause and resume a running backfill", async ({ page }) => {
+ test.setTimeout(180_000);
+ const backfillPage = new BackfillPage(page);
+
+ await backfillPage.navigateToDagDetail(testDagId);
+ await backfillPage.openBackfillDialog();
+
+ await backfillPage.backfillFromDateInput.fill(fromDate);
+ await backfillPage.backfillToDateInput.fill(toDate);
+ await backfillPage.selectReprocessBehavior("All Runs");
+
+ const runsMessage = page.locator("text=/\\d+ runs? will be triggered|No
runs matching/");
+
+ await expect(runsMessage).toBeVisible({ timeout: 10_000 });
+ await expect(backfillPage.backfillRunButton).toBeEnabled({ timeout: 15_000
});
+ await backfillPage.backfillRunButton.click();
+
+ await backfillPage.navigateToBackfillsTab(testDagId);
+ await expect(backfillPage.pauseButton).toBeVisible({ timeout: 10_000 });
+
+ await backfillPage.clickPauseButton();
+
+ const isPaused = await backfillPage.isBackfillPaused();
+
+ expect(isPaused).toBe(true);
+
+ await backfillPage.clickPauseButton();
+
+ const isResumed = await backfillPage.isBackfillPaused();
+
+ expect(isResumed).toBe(false);
+ });
+
+ test("should cancel an active backfill", async ({ page }) => {
+ test.setTimeout(180_000);
+ const backfillPage = new BackfillPage(page);
+
+ await backfillPage.navigateToDagDetail(testDagId);
+ await backfillPage.openBackfillDialog();
+
+ await backfillPage.backfillFromDateInput.fill(fromDate);
+ await backfillPage.backfillToDateInput.fill(toDate);
+ await backfillPage.selectReprocessBehavior("All Runs");
+
+ const runsMessage = page.locator("text=/\\d+ runs? will be triggered|No
runs matching/");
+
+ await expect(runsMessage).toBeVisible({ timeout: 10_000 });
+ await expect(backfillPage.backfillRunButton).toBeEnabled({ timeout: 15_000
});
+ await backfillPage.backfillRunButton.click();
+
+ await backfillPage.navigateToBackfillsTab(testDagId);
+ await expect(backfillPage.cancelButton).toBeVisible({ timeout: 10_000 });
+
+ await backfillPage.clickCancelButton();
+
+ await expect(backfillPage.pauseButton).not.toBeVisible({ timeout: 10_000
});
+ await expect(backfillPage.cancelButton).not.toBeVisible({ timeout: 10_000
});
+ });
+
+ test("should not be able to resume a cancelled backfill", async ({ page })
=> {
Review Comment:
This test looks like same as `should cancel an active backfill`
##########
airflow-core/src/airflow/ui/tests/e2e/pages/BackfillPage.ts:
##########
@@ -102,6 +108,18 @@ export class BackfillPage extends BasePage {
return `/dags/${dagName}`;
}
+ public async clickCancelButton(): Promise<void> {
+ await expect(this.cancelButton).toBeVisible({ timeout: 10_000 });
+ await this.cancelButton.click();
+ await this.page.waitForTimeout(1000);
Review Comment:
Replace `waitForTimeout(1000)` with explicit state waits like `await
expect(button).not.toBeVisible()`
##########
airflow-core/src/airflow/ui/tests/e2e/specs/backfill-controls.spec.ts:
##########
@@ -0,0 +1,121 @@
+/*!
+ * 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 { testConfig, AUTH_FILE } from "playwright.config";
+import { BackfillPage } from "tests/e2e/pages/BackfillPage";
+
+const getPastDate = (daysAgo: number): string => {
+ const date = new Date();
+
+ date.setDate(date.getDate() - daysAgo);
+ date.setHours(0, 0, 0, 0);
+
+ return date.toISOString().slice(0, 16);
+};
+
+test.describe("Backfill Controls", () => {
+ const testDagId = testConfig.testDag.id;
+ const fromDate = getPastDate(2);
Review Comment:
```
test.describe("Backfill controls", () => {
test.describe.configure({ mode: "serial" });
test.setTimeout(180_000);
const testDagId = testConfig.testDag.id;
const controlFromDate = getPastDate(15);
const controlToDate = getPastDate(14);
let backfillPage: BackfillPage;
test.beforeEach(async ({ page }) => {
backfillPage = new BackfillPage(page);
await backfillPage.createBackfill(testDagId, {
fromDate: controlFromDate,
reprocessBehavior: "All Runs",
toDate: controlToDate,
});
await backfillPage.navigateToBackfillsTab(testDagId);
});
test.afterEach(async () => {
try {
await backfillPage.clickCancelButton();
} catch {
}
});
test("verify pause and resume backfill", async () => {
await backfillPage.clickPauseButton();
expect(await backfillPage.isBackfillPaused()).toBe(true);
await backfillPage.clickPauseButton();
expect(await backfillPage.isBackfillPaused()).toBe(false);
});
test("verify cancel backfill", async () => {
await backfillPage.clickCancelButton();
await expect(backfillPage.pauseButton).not.toBeVisible();
await expect(backfillPage.cancelButton).not.toBeVisible();
});
});
```
You can update the above in backfill-spec.ts file, and all these tests
should be sufficient
--
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]
