You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by "sarth-akvaish (via GitHub)" <gi...@apache.org> on 2025/12/31 17:27:19 UTC
[PR] Test: Add E2E tests for Assets Page [airflow]
sarth-akvaish opened a new pull request, #59990:
URL: https://github.com/apache/airflow/pull/59990
This PR adds the E2E tests for Assets Page.
The test now verfies:
- Assets are listed in the table
- Asset name are visible
- Clicking on an asset name navigates to asset detail page
- working of search filters and pagination
Closes : https://github.com/apache/airflow/issues/59929
<!--
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.
-->
<!--
Thank you for contributing! Please make sure that your code changes
are covered with tests. And in case of new features or big changes
remember to adjust the documentation.
Feel free to ping committers for the review!
In case of an existing issue, reference it using one of the following:
closes: #ISSUE
related: #ISSUE
How to write a good git commit message:
http://chris.beams.io/posts/git-commit/
-->
<!-- Please keep an empty line above the dashes. -->
---
**^ Add meaningful description above**
Read the **[Pull Request Guidelines](https://github.com/apache/airflow/blob/main/contributing-docs/05_pull_requests.rst#pull-request-guidelines)** for more information.
In case of fundamental code changes, an Airflow Improvement Proposal ([AIP](https://cwiki.apache.org/confluence/display/AIRFLOW/Airflow+Improvement+Proposals)) is needed.
In case of a new dependency, check compliance with the [ASF 3rd Party License Policy](https://www.apache.org/legal/resolved.html#category-x).
In case of backwards incompatible changes please leave a note in a newsfragment file, named `{pr_number}.significant.rst` or `{issue_number}.significant.rst`, in [airflow-core/newsfragments](https://github.com/apache/airflow/tree/main/airflow-core/newsfragments).
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Test: Add E2E tests for Assets Page [airflow]
Posted by "sarth-akvaish (via GitHub)" <gi...@apache.org>.
sarth-akvaish commented on PR #59990:
URL: https://github.com/apache/airflow/pull/59990#issuecomment-3715129652
> @sarth-akvaish Thanks for the PR! A few things to address:
>
> 1. Add `beforeAll` to trigger producer DAG - Tests assume assets exist. Add setup that unpause consumer DAG and triggers `asset_produces_1` to create real asset data:
> test.beforeAll(async ({ browser }) => {
> // Trigger asset_produces_1 to create asset data
> });
>
> Also maybe trigger multiple dags which uses assets and it will help in search test to filter 2. Coordinate with PR #59939 - Both PRs create different page objects. Work together to use single `AssetsPage.ts` and `assets.spec.ts`
>
> 3. Test naming - Use "verify" prefix
> 4. Fix fragile selectors - Be more specific
> 5. Pagination - Use `?offset=0&limit=1` URL param
>
> Let me know if you have questions
Hi @vatsrahul1001, Thanks for the review. I’ll incorporate the suggested changes and update the PR.
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Test: Add E2E tests for Assets Page [airflow]
Posted by "vatsrahul1001 (via GitHub)" <gi...@apache.org>.
vatsrahul1001 commented on PR #59990:
URL: https://github.com/apache/airflow/pull/59990#issuecomment-3705137560
@sarth-akvaish can you look at failing tests?
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Test: Add E2E tests for Assets Page [airflow]
Posted by "vatsrahul1001 (via GitHub)" <gi...@apache.org>.
vatsrahul1001 commented on PR #59990:
URL: https://github.com/apache/airflow/pull/59990#issuecomment-3748504106
Great work @sarth-akvaish merging the PR
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Test: Add E2E tests for Assets Page [airflow]
Posted by "vatsrahul1001 (via GitHub)" <gi...@apache.org>.
vatsrahul1001 commented on PR #59990:
URL: https://github.com/apache/airflow/pull/59990#issuecomment-3728410702
@sarth-akvaish we are missing test for `verify clicking asset navigates to detail page`
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Test: Add E2E tests for Assets Page [airflow]
Posted by "vatsrahul1001 (via GitHub)" <gi...@apache.org>.
vatsrahul1001 commented on PR #59990:
URL: https://github.com/apache/airflow/pull/59990#issuecomment-3714687059
@sarth-akvaish Thanks for the PR! A few things to address:
1. Add `beforeAll` to trigger producer DAG - Tests assume assets exist. Add setup that unpause consumer DAG and triggers `asset_produces_1` to create real asset data:
test.beforeAll(async ({ browser }) => {
// Trigger asset_produces_1 to create asset data
});
2. Coordinate with PR #59939 - Both PRs create different page objects. Work together to use single `AssetsPage.ts` and `assets.spec.ts`
3. Test naming - Use "verify" prefix
4. Fix fragile selectors - Be more specific
5. Pagination - Use `?offset=0&limit=1` URL param
Let me know if you have questions
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Test: Add E2E tests for Assets Page [airflow]
Posted by "vatsrahul1001 (via GitHub)" <gi...@apache.org>.
vatsrahul1001 merged PR #59990:
URL: https://github.com/apache/airflow/pull/59990
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Test: Add E2E tests for Assets Page [airflow]
Posted by "vatsrahul1001 (via GitHub)" <gi...@apache.org>.
vatsrahul1001 commented on code in PR #59990:
URL: https://github.com/apache/airflow/pull/59990#discussion_r2675692634
##########
airflow-core/src/airflow/ui/tests/e2e/specs/asset.spec.ts:
##########
@@ -0,0 +1,169 @@
+/*!
+ * 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";
Review Comment:
missing Auth state import
`import { AUTH_FILE } from "playwright.config";`
##########
airflow-core/src/airflow/ui/tests/e2e/specs/asset.spec.ts:
##########
@@ -0,0 +1,169 @@
+/*!
+ * 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 { AssetListPage } from "../pages/AssetListPage";
+
+test.describe("Assets Page", () => {
+ let assets: AssetListPage;
+
+ test.beforeAll(async ({ browser }) => {
+ const page = await browser.newPage();
+
+ const DAG_ID = "asset_consumes_1";
+
+ await page.goto(`/dags`);
+ await page.waitForLoadState("networkidle");
+
+ // Navigate to DAG details page
+ await page.goto(`/dags/${DAG_ID}`);
+ await page.waitForLoadState("networkidle");
+
+ // Unpause DAG if it is paused
+ const checkbox = page.locator('input[type="checkbox"]').first();
+ const toggleControl = page.locator('[data-part="control"]').first();
+
+ await expect(checkbox).toBeVisible({ timeout: 20_000 });
+
+ const isChecked = await checkbox.isChecked();
+
+ if (!isChecked) {
+ // Click visible control
+ await toggleControl.click();
+
+ const confirmUnpause = page.getByRole("button", { name: /confirm|unpause/i });
+
+ if (await confirmUnpause.isVisible({ timeout: 2000 }).catch(() => false)) {
+ await confirmUnpause.click();
+ }
+
+ // Validate updated state
+ await expect(checkbox).toBeChecked();
+ }
+
+ // Trigger the DAG (opens TriggerDAGModal)
+ const triggerButton = page.getByRole("button", { name: /trigger/i });
+
+ await expect(triggerButton).toBeVisible();
+ await triggerButton.click();
+
+ // Confirm trigger inside TriggerDAGModal
+ const triggerConfirm = page.getByRole("button", { name: /trigger|confirm/i });
+
+ await expect(triggerConfirm).toBeVisible({ timeout: 10_000 });
+ await triggerConfirm.click();
+
+ // Navigate to DAG Runs page
+ await page.goto(`/dags/${DAG_ID}/runs`);
+ await page.waitForLoadState("networkidle");
+
+ await expect
+ .poll(
+ async () => {
+ const row = page.locator("tbody tr").first();
+
+ if (!(await row.isVisible().catch(() => false))) {
+ return "loading";
+ }
+
+ const text = (await row.textContent())?.toLowerCase().trim() ?? "";
+
+ if (text.includes("success")) return "success";
+ if (text.includes("failed")) return "failed";
+ if (text.includes("running")) return "running";
+
+ return "loading";
+ },
+ {
+ intervals: [3000],
+ timeout: 30_000,
+ },
+ )
+ .toBe("success");
+
+ await page.close();
+ });
+
Review Comment:
```suggestion
test.beforeAll(async ({ browser }) => {
test.setTimeout(3 * 60 * 1000);
const context = await browser.newContext({ storageState: AUTH_FILE });
const page = await context.newPage();
const dagsPage = new DagsPage(page);
await dagsPage.triggerDag("asset_produces_1");
await expect
.poll(
async () => {
const response = await page.request.get(
`/api/v2/dags/asset_produces_1/dagRuns?order_by=-start_date&limit=1`
);
const data = await response.json();
return data.dag_runs?.[0]?.state ?? "pending";
},
{ intervals: [2000], timeout: 120_000 }
)
.toBe("success");
await context.close();
});
```
##########
airflow-core/src/airflow/ui/tests/e2e/pages/AssetListPage.ts:
##########
@@ -0,0 +1,79 @@
+/*!
+ * 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 "./BasePage";
+
+export class AssetListPage extends BasePage {
+ public readonly emptyState: Locator;
+ public readonly heading: Locator;
+ public readonly rows: Locator;
+ public readonly searchInput: Locator;
+ public readonly table: Locator;
+
+ public constructor(page: Page) {
+ super(page);
+
+ this.heading = page.getByRole("heading", {
+ name: /\d+\s+asset/i,
+ });
+ this.table = page.getByTestId("table-list");
+ this.rows = this.table.locator("tbody tr").filter({
+ has: page.locator("td"),
+ });
+
+ this.searchInput = page.getByTestId("search-dags");
+ this.emptyState = page.getByText(/no items/i);
+ }
+
+ public async assetCount(): Promise<number> {
+ return this.rows.count();
+ }
+
+ public async assetNames(): Promise<Array<string>> {
Review Comment:
looks duplicate of `getAssetNames`
##########
airflow-core/src/airflow/ui/tests/e2e/specs/asset.spec.ts:
##########
@@ -0,0 +1,169 @@
+/*!
+ * 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 { AssetListPage } from "../pages/AssetListPage";
+
+test.describe("Assets Page", () => {
+ let assets: AssetListPage;
+
+ test.beforeAll(async ({ browser }) => {
Review Comment:
beforeAll mising auth state and looks very complicating
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Test: Add E2E tests for Assets Page [airflow]
Posted by "vatsrahul1001 (via GitHub)" <gi...@apache.org>.
vatsrahul1001 commented on code in PR #59990:
URL: https://github.com/apache/airflow/pull/59990#discussion_r2675724450
##########
airflow-core/src/airflow/ui/tests/e2e/specs/asset.spec.ts:
##########
@@ -0,0 +1,169 @@
+/*!
+ * 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 { AssetListPage } from "../pages/AssetListPage";
+
+test.describe("Assets Page", () => {
+ let assets: AssetListPage;
+
+ test.beforeAll(async ({ browser }) => {
Review Comment:
beforeAll mising auth state and looks very complicated
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Test: Add E2E tests for Assets Page [airflow]
Posted by "vatsrahul1001 (via GitHub)" <gi...@apache.org>.
vatsrahul1001 commented on code in PR #59990:
URL: https://github.com/apache/airflow/pull/59990#discussion_r2657445464
##########
airflow-core/src/airflow/ui/tests/e2e/specs/asset.spec.ts:
##########
@@ -0,0 +1,44 @@
+import { test, expect } from "@playwright/test";
Review Comment:
Missing Apache License Headers
##########
airflow-core/src/airflow/ui/tests/e2e/pages/AssetListPage.ts:
##########
@@ -0,0 +1,53 @@
+import type { Locator, Page } from "@playwright/test";
Review Comment:
Missing Apache License Headers
##########
airflow-core/src/airflow/ui/tests/e2e/pages/AssetListPage.ts:
##########
@@ -0,0 +1,87 @@
+/*!
+ * 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 "./BasePage";
+
+export class AssetListPage extends BasePage {
+ public readonly emptyState: Locator;
+ public readonly heading: Locator;
+ public readonly nextButton: Locator;
+ public readonly prevButton: Locator;
+ public readonly rows: Locator;
+ public readonly searchInput: Locator;
+ public readonly table: Locator;
+
+ public constructor(page: Page) {
+ super(page);
+
+ this.heading = page.getByRole("heading", { level: 2 });
Review Comment:
This is Fragile
We can try
`this.heading = page.getByRole("heading", { name: /assets/i });`
##########
airflow-core/src/airflow/ui/tests/e2e/specs/asset.spec.ts:
##########
@@ -0,0 +1,78 @@
+/*!
+ * 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 { AssetListPage } from "../pages/AssetListPage";
+
+test.describe("Assets Page", () => {
+ let assets: AssetListPage;
+
+ test.beforeEach(async ({ page }) => {
+ assets = new AssetListPage(page);
+ await assets.navigate();
+ await assets.waitForLoad();
+ });
+
+ test("renders assets page heading", async () => {
+ await expect(assets.heading).toBeVisible();
+ });
+
+ test("renders assets table", async () => {
+ await expect(assets.table).toBeVisible();
+ });
+
+ test("shows asset rows when data exists", async () => {
+ const count = await assets.assetCount();
+
+ expect(count).toBeGreaterThanOrEqual(0);
+ });
+
+ test("each asset has a visible name link", async () => {
+ const names = await assets.assetNames();
+
+ for (const name of names) {
+ expect(name.trim().length).toBeGreaterThan(0);
+ }
+ });
+
+ test("filters assets using search", async () => {
Review Comment:
Current test is not testing filtering correctly. You can try below
```
test("verify search filters assets", async () => {
const initialCount = await assets.assetCount();
expect(initialCount).toBeGreaterThan(1);
await assets.search("dag1"); // dag1 is just example, use proper asset name which filters
await expect.poll(() => assets.assetCount(), { timeout: 20_000 }).toBeLessThan(initialCount);
const names = await assets.assetNames();
expect(names.length).toBeGreaterThan(0);
for (const name of names) {
expect(name.toLowerCase()).toContain("dag1");
}
});
```
##########
airflow-core/src/airflow/ui/tests/e2e/specs/asset.spec.ts:
##########
@@ -0,0 +1,78 @@
+/*!
+ * 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 { AssetListPage } from "../pages/AssetListPage";
+
+test.describe("Assets Page", () => {
+ let assets: AssetListPage;
+
+ test.beforeEach(async ({ page }) => {
+ assets = new AssetListPage(page);
+ await assets.navigate();
+ await assets.waitForLoad();
+ });
+
+ test("renders assets page heading", async () => {
+ await expect(assets.heading).toBeVisible();
+ });
+
+ test("renders assets table", async () => {
+ await expect(assets.table).toBeVisible();
+ });
+
+ test("shows asset rows when data exists", async () => {
+ const count = await assets.assetCount();
+
+ expect(count).toBeGreaterThanOrEqual(0);
+ });
+
+ test("each asset has a visible name link", async () => {
+ const names = await assets.assetNames();
+
+ for (const name of names) {
+ expect(name.trim().length).toBeGreaterThan(0);
+ }
+ });
+
+ test("filters assets using search", async () => {
+ const initialCount = await assets.assetCount();
+
+ await assets.search("dag");
+ await expect.poll(() => assets.assetCount(), { timeout: 20_000 }).toBeLessThanOrEqual(initialCount);
+
+ if ((await assets.assetCount()) === 0) {
+ await expect(assets.emptyState).toBeVisible();
+ }
+ });
+
+ test("supports pagination when multiple pages exist", async () => {
Review Comment:
You can try something like below
test("verify pagination controls navigate between pages", async () => {
// Force pagination with small limit
await assets.navigateTo("/assets?limit=5&offset=0");
await assets.waitForLoad();
const page1Assets = await assets.getAssetNames();
expect(page1Assets.length).toBeGreaterThan(0);
await assets.page.getByRole("button", { name: "2" }).click();
await assets.page.waitForURL(/offset=5/);
await assets.waitForLoad();
const page2Assets = await assets.getAssetNames();
expect(page2Assets.length).toBeGreaterThan(0);
expect(page2Assets).not.toEqual(page1Assets);
await assets.page.getByRole("button", { name: "1" }).click();
await assets.page.waitForURL(/offset=0|(?!.*offset)/);
await assets.waitForLoad();
const backToPage1Assets = await assets.getAssetNames();
expect(backToPage1Assets).toEqual(page1Assets);
});
##########
airflow-core/src/airflow/ui/tests/e2e/pages/AssetListPage.ts:
##########
@@ -0,0 +1,87 @@
+/*!
+ * 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 "./BasePage";
+
+export class AssetListPage extends BasePage {
+ public readonly emptyState: Locator;
+ public readonly heading: Locator;
+ public readonly nextButton: Locator;
+ public readonly prevButton: Locator;
+ public readonly rows: Locator;
+ public readonly searchInput: Locator;
+ public readonly table: Locator;
+
+ public constructor(page: Page) {
+ super(page);
+
+ this.heading = page.getByRole("heading", { level: 2 });
+ this.table = page.getByTestId("table-list");
+ this.rows = this.table.locator("tbody tr").filter({
+ has: page.locator("td"),
+ });
+
+ this.nextButton = page.getByTestId("next");
+ this.prevButton = page.getByTestId("prev");
+
+ this.searchInput = page.getByRole("textbox");
Review Comment:
We can use `this.searchInput = page.getByTestId("search-dags");`
##########
airflow-core/src/airflow/ui/tests/e2e/specs/asset.spec.ts:
##########
@@ -0,0 +1,78 @@
+/*!
+ * 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 { AssetListPage } from "../pages/AssetListPage";
+
+test.describe("Assets Page", () => {
+ let assets: AssetListPage;
+
+ test.beforeEach(async ({ page }) => {
+ assets = new AssetListPage(page);
+ await assets.navigate();
+ await assets.waitForLoad();
+ });
+
+ test("renders assets page heading", async () => {
+ await expect(assets.heading).toBeVisible();
+ });
+
+ test("renders assets table", async () => {
+ await expect(assets.table).toBeVisible();
+ });
+
+ test("shows asset rows when data exists", async () => {
+ const count = await assets.assetCount();
+
+ expect(count).toBeGreaterThanOrEqual(0);
+ });
+
+ test("each asset has a visible name link", async () => {
+ const names = await assets.assetNames();
+
+ for (const name of names) {
+ expect(name.trim().length).toBeGreaterThan(0);
+ }
+ });
+
+ test("filters assets using search", async () => {
+ const initialCount = await assets.assetCount();
+
+ await assets.search("dag");
+ await expect.poll(() => assets.assetCount(), { timeout: 20_000 }).toBeLessThanOrEqual(initialCount);
+
+ if ((await assets.assetCount()) === 0) {
+ await expect(assets.emptyState).toBeVisible();
+ }
+ });
+
+ test("supports pagination when multiple pages exist", async () => {
Review Comment:
Also we need to add below to AssetListPage
```
async getAssetNames(): Promise<string[]> {
const names: string[] = [];
const count = await this.rows.count();
for (let i = 0; i < count; i++) {
const text = await this.rows.nth(i).locator("a").first().textContent();
if (text) names.push(text.trim());
}
return names;
}
async clickPageNumber(pageNum: number): Promise<void> {
await this.page.getByRole("button", { name: String(pageNum) }).click();
}
```
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Test: Add E2E tests for Assets Page [airflow]
Posted by "sarth-akvaish (via GitHub)" <gi...@apache.org>.
sarth-akvaish commented on PR #59990:
URL: https://github.com/apache/airflow/pull/59990#issuecomment-3723623443
Hi @vatsrahul1001 , currently in beforeAll I'm triggering a DAG, and checking its status in the run tab (using polling), Should I have to check the status of Dag run ? If yes, then current one is giving flaky test, can I test using rest api endpoints?
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Test: Add E2E tests for Assets Page [airflow]
Posted by "vatsrahul1001 (via GitHub)" <gi...@apache.org>.
vatsrahul1001 commented on PR #59990:
URL: https://github.com/apache/airflow/pull/59990#issuecomment-3728409871
> Hi @vatsrahul1001 , currently in beforeAll I'm triggering a DAG, and checking its status in the run tab (using polling), Should I have to check the status of Dag run ? If yes, then current one is giving flaky test, can I test using rest api endpoints?
Yes we can use RESTAPI and have added this in suggested change
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org