You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by "haseebmalik18 (via GitHub)" <gi...@apache.org> on 2026/04/28 22:26:41 UTC
[PR] Improve Playwright test patterns in connections e2e tests [airflow]
haseebmalik18 opened a new pull request, #66058:
URL: https://github.com/apache/airflow/pull/66058
Improve Playwright test patterns in connections e2e tests
- Remove redundant `waitForLoadState("domcontentloaded")` in `ConnectionsPage.ts`.
- Replace manual `expect(url).toContain()` with web-first `toHaveURL()` in `connections.spec.ts` for auto-retry support.
closes: #63088
--
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] Improve Playwright test patterns in connections e2e tests [airflow]
Posted by "choo121600 (via GitHub)" <gi...@apache.org>.
choo121600 merged PR #66058:
URL: https://github.com/apache/airflow/pull/66058
--
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] Improve Playwright test patterns in connections e2e tests [airflow]
Posted by "choo121600 (via GitHub)" <gi...@apache.org>.
choo121600 commented on code in PR #66058:
URL: https://github.com/apache/airflow/pull/66058#discussion_r3158620420
##########
airflow-core/src/airflow/ui/tests/e2e/specs/connections.spec.ts:
##########
@@ -289,18 +289,6 @@ test.describe("Connections Page - Search and Filter", () => {
await connectionsPage.searchConnections(searchTerm);
- await expect
Review Comment:
Could you clarify why these were removed?
--
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] Improve Playwright test patterns in connections e2e tests [airflow]
Posted by "choo121600 (via GitHub)" <gi...@apache.org>.
choo121600 commented on code in PR #66058:
URL: https://github.com/apache/airflow/pull/66058#discussion_r3186055385
##########
airflow-core/src/airflow/ui/tests/e2e/pages/ConnectionsPage.ts:
##########
@@ -394,7 +393,7 @@ export class ConnectionsPage extends BasePage {
}
private async waitForConnectionsListLoad(): Promise<void> {
- await expect(this.page).toHaveURL(/\/connections/);
+ await expect(this.page).toHaveURL(/\/connections/, { timeout: 10_000 });
Review Comment:
we can remove `{ timeout: 10_000 }` from expect assertions and Locator actions, as Playwright config already sets global defaults.
--
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] Improve Playwright test patterns in connections e2e tests [airflow]
Posted by "choo121600 (via GitHub)" <gi...@apache.org>.
choo121600 commented on code in PR #66058:
URL: https://github.com/apache/airflow/pull/66058#discussion_r3179331521
##########
airflow-core/src/airflow/ui/tests/e2e/pages/ConnectionsPage.ts:
##########
@@ -224,7 +225,10 @@ export class ConnectionsPage extends BasePage {
if (details.extra !== undefined && details.extra !== "") {
const extraAccordion = this.page.getByRole("button", { name: /extra fields json/i });
- const accordionVisible = await extraAccordion.isVisible({ timeout: 5000 }).catch(() => false);
+ const accordionVisible = await extraAccordion
Review Comment:
Is there a specific reason for choosing `waitFor({ state: "visible", timeout: 5000 })` over `isVisible({ timeout: 5000 })`?
--
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] Improve Playwright test patterns in connections e2e tests [airflow]
Posted by "haseebmalik18 (via GitHub)" <gi...@apache.org>.
haseebmalik18 commented on code in PR #66058:
URL: https://github.com/apache/airflow/pull/66058#discussion_r3158755586
##########
airflow-core/src/airflow/ui/tests/e2e/specs/connections.spec.ts:
##########
@@ -289,18 +289,6 @@ test.describe("Connections Page - Search and Filter", () => {
await connectionsPage.searchConnections(searchTerm);
- await expect
Review Comment:
`searchConnections()` has its own `expect.poll()` internally that already waits for the results to match the search term before it returns. So these were just duplicate checks
--
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] Improve Playwright test patterns in connections e2e tests [airflow]
Posted by "haseebmalik18 (via GitHub)" <gi...@apache.org>.
haseebmalik18 commented on code in PR #66058:
URL: https://github.com/apache/airflow/pull/66058#discussion_r3183383315
##########
airflow-core/src/airflow/ui/tests/e2e/pages/ConnectionsPage.ts:
##########
@@ -224,7 +225,10 @@ export class ConnectionsPage extends BasePage {
if (details.extra !== undefined && details.extra !== "") {
const extraAccordion = this.page.getByRole("button", { name: /extra fields json/i });
- const accordionVisible = await extraAccordion.isVisible({ timeout: 5000 }).catch(() => false);
+ const accordionVisible = await extraAccordion
Review Comment:
`isVisible()` does accept a timeout param but [Playwright ignores](https://playwright.dev/docs/api/class-locator#locator-is-visible) it it always returns immediately without waiting. `waitFor()` actually waits up to 5s for the UI element to render
##########
airflow-core/src/airflow/ui/tests/e2e/pages/ConnectionsPage.ts:
##########
@@ -224,7 +225,10 @@ export class ConnectionsPage extends BasePage {
if (details.extra !== undefined && details.extra !== "") {
const extraAccordion = this.page.getByRole("button", { name: /extra fields json/i });
- const accordionVisible = await extraAccordion.isVisible({ timeout: 5000 }).catch(() => false);
+ const accordionVisible = await extraAccordion
Review Comment:
`isVisible()` does accept a timeout param but [Playwright ignores](https://playwright.dev/docs/api/class-locator#locator-is-visible) it always returns immediately without waiting. `waitFor()` actually waits up to 5s for the UI element to render
--
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