Copilot commented on code in PR #63993:
URL: https://github.com/apache/airflow/pull/63993#discussion_r3066474302
##########
airflow-core/src/airflow/ui/tests/e2e/pages/XComsPage.ts:
##########
@@ -157,21 +151,22 @@ export class XComsPage extends BasePage {
const dataLinks = this.xcomsTable.locator("a[href*='/dags/']");
await expect(dataLinks.first()).toBeVisible({ timeout: 30_000 });
- expect(await dataLinks.count()).toBeGreaterThan(0);
+ await expect(dataLinks).not.toHaveCount(0);
}
public async verifyXComValuesDisplayed(): Promise<void> {
- const firstRow = this.xcomsTable.locator("tbody tr").first();
+ const firstRow = this.tableRows.first();
await expect(firstRow).toBeVisible({ timeout: 10_000 });
- const valueCell = firstRow.locator("td").last();
+ const valueCell = firstRow.getByRole("cell").last();
await expect(valueCell).toBeVisible();
await expect(async () => {
- const textContent = await valueCell.textContent();
- const hasTextContent = (textContent?.trim().length ?? 0) > 0;
+ const hasTextContent = await valueCell.evaluate(
+ (el) => (el.textContent?.trim().length ?? 0) > 0,
+ );
Review Comment:
This reintroduces `evaluate()` into the page object, which the PR
description calls out as an anti-pattern to avoid when possible. Consider using
locator/web-first primitives instead (e.g., `await valueCell.textContent()` in
Node context, or an `expect.poll` over `textContent()`/`innerText()`), keeping
DOM execution in the browser to a minimum for maintainability and debuggability.
```suggestion
const textContent = (await valueCell.textContent())?.trim() ?? "";
const hasTextContent = textContent.length > 0;
```
--
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]
