Skip to content

fix(ui): wait for schedule deletion redirect in live test #2879

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
Dragonzz27 wants to merge 1 commit into
base: main
Choose a base branch
from
+3 −3
Open

fix(ui): wait for schedule deletion redirect in live test #2879

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions ui/playwright/live/schedules.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,9 +63,9 @@ test("live: schedule configuration persists through the browser and controller",
await page.goto(detailURL);
await page.getByRole("button", { name: `Delete schedule ${name}`, exact: true }).click();
await page.getByRole("dialog").getByRole("button", { name: "Delete", exact: true }).click();
await expect(page.getByText("This schedule was deleted. Its execution history is retained.")).toBeVisible();
await expect(page.getByRole("button", { name: "Run", exact: true })).toBeDisabled();
await page.getByRole("link", { name: "Back", exact: true }).click();
// Deleting leaves the detail page; its retained-history controls are no

@Charlesthebird Charlesthebird Sep 21, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[nit]
Level: 🟡 Low · Not Blocking

Only the alert is about retained history — Run is the page's primary action, disabled because the schedule is gone (src/pages/ScheduledRunsPage.tsx:113,167) — and the second sentence restates the line under it.

Suggested change
// Deleting leaves the detail page; its retained-history controls are no
// Deleting navigates back to the list, so the detail page's own controls are gone.

🤖 written by Claude

// longer mounted. Wait for navigation before checking the list.
await expect(page).toHaveURL(/\/schedules(?:\?.*)?$/);
await expect(page.getByRole("link", { name, exact: true })).toHaveCount(0);

@Charlesthebird Charlesthebird Sep 21, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Level: 🔴 High · Blocking

This assertion cannot fail, and after the edit it is the only thing standing between a working delete and a leaked schedule. page.goto(detailURL) on line 63 starts a fresh document, and useInvalidateKeys "reaches what is on screen, and only that" (src/api/hooks/useInvalidateKeys.ts:19), so the list key is never cached: the freshly mounted list renders zero rows while it reads, and this passes on the first poll. It would pass just as happily against a controller that kept the schedule.

There is no DOM state that separates "loading" from "empty" here — against ?mock=slow the loading table renders one .ant-table-placeholder row with schedules-empty visible — so the read itself has to be waited for. Attach it before the click, since the response can land before the URL assertion resolves:

const listRead = page.waitForResponse((r) => r.url().includes("ScheduledRunService/ListScheduledRuns"));
await page.getByRole("dialog").getByRole("button", { name: "Delete", exact: true }).click();
await expect(page).toHaveURL(/\/schedules(?:\?.*)?$/);
await listRead;
await expect(page.getByTestId(`schedule-link-${name}`)).toHaveCount(0);

The list polls every 10s (src/pages/ScheduledRunsPage.tsx:32), so a missed first read cannot hang the wait. getByTestId rather than the link role, to match the list's own handle.

🤖 written by Claude

}
}
Expand Down
Loading