Copilot commented on code in PR #64189:
URL: https://github.com/apache/airflow/pull/64189#discussion_r3025333552
##########
airflow-core/src/airflow/ui/tests/e2e/pages/DagCalendarTab.ts:
##########
@@ -50,9 +50,22 @@ export class DagCalendarTab extends BasePage {
for (let i = 0; i < count; i++) {
const cell = this.activeCells.nth(i);
- const bg = await cell.evaluate((el) =>
window.getComputedStyle(el).backgroundColor);
+ const computedStyle = await cell.getAttribute("style");
- colors.push(bg);
+ if (computedStyle) {
+ const bgColorMatch =
computedStyle.match(/background-color\s*:\s*([^;]+)/);
+
+ if (bgColorMatch) {
+ colors.push(bgColorMatch[1].trim());
+ continue;
+ }
+ }
+
+ const dataColor = await cell.getAttribute("data-bg-color");
+
+ if (dataColor) {
+ colors.push(dataColor);
+ }
}
Review Comment:
`getActiveCellColors()` now reads `background-color` from the element's
`style` attribute or a `data-bg-color` attribute. In the current UI
implementation, `calendar-cell` backgrounds are applied via Chakra/emotion
classes and (in daily mode) via child `<Box>` elements, and there is no
`data-bg-color` attribute. This will typically result in `colors` being
empty/shorter than the number of active cells. Consider reverting to
`getComputedStyle(...)` (via `evaluate`) or updating the rendered calendar
cells to expose the intended colors via explicit `data-*` attributes that the
tests can read.
##########
airflow-core/src/airflow/ui/tests/e2e/pages/DagCalendarTab.ts:
##########
@@ -112,22 +125,13 @@ export class DagCalendarTab extends BasePage {
}
private async waitForCalendarReady(): Promise<void> {
- await this.page.getByTestId("dag-calendar-root").waitFor({ state:
"visible", timeout: 120_000 });
-
- await this.page.getByTestId("calendar-current-period").waitFor({ state:
"visible", timeout: 120_000 });
-
- const overlay = this.page.getByTestId("calendar-loading-overlay");
-
- if (await overlay.isVisible().catch(() => false)) {
- await overlay.waitFor({ state: "hidden", timeout: 120_000 });
- }
+ await expect(this.page.getByTestId("dag-calendar-root")).toBeVisible({
timeout: 120_000 });
- await this.page.getByTestId("calendar-grid").waitFor({ state: "visible",
timeout: 120_000 });
+ await
expect(this.page.getByTestId("calendar-current-period")).toBeVisible({ timeout:
120_000 });
+ await expect(this.page.getByTestId("calendar-grid")).toBeVisible({
timeout: 120_000 });
Review Comment:
`waitForCalendarReady()` removed the loading overlay handling entirely.
Since the overlay element is conditionally rendered, you can use a web-first
assertion like `expect(overlay).toBeHidden(...)` (which passes when the element
is absent) to ensure the calendar is actually ready for interactions, without
the previous try/catch visibility check.
```suggestion
const overlay = this.page.getByTestId("calendar-loading-overlay");
await expect(overlay).toBeHidden({ timeout: 120_000 });
```
##########
airflow-core/src/airflow/ui/tests/e2e/pages/DagCalendarTab.ts:
##########
@@ -88,7 +101,7 @@ export class DagCalendarTab extends BasePage {
public async navigateToCalendar(dagId: string) {
await expect(async () => {
await this.safeGoto(`/dags/${dagId}/calendar`);
- await this.page.getByTestId("dag-calendar-root").waitFor({ state:
"visible", timeout: 5000 });
+ await expect(this.page.getByTestId("dag-calendar-root")).toBeVisible({
timeout: 5000 });
Review Comment:
The indentation inside `navigateToCalendar()`'s `expect(async () => { ...
}).toPass(...)` block is inconsistent (the inner `await
expect(...).toBeVisible(...)` line is not indented like similar patterns in
other page objects, e.g. `BackfillPage.navigateToBackfillsTab` in
`airflow-core/src/airflow/ui/tests/e2e/pages/BackfillPage.ts:378-381`). This
will likely be reformatted by Prettier/ESLint; please run the formatter so the
block matches the established style.
```suggestion
await expect(this.page.getByTestId("dag-calendar-root")).toBeVisible({
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
