diff --git a/apps/web/e2e/helpers/dockview-resize.ts b/apps/web/e2e/helpers/dockview-resize.ts index 66f7f519d3..480ac7d3a9 100644 --- a/apps/web/e2e/helpers/dockview-resize.ts +++ b/apps/web/e2e/helpers/dockview-resize.ts @@ -49,6 +49,71 @@ export async function getDockviewGroupWidth(page: Page, panelId: string): Promis }, panelId); } +/** Read the dockview container's live pixel width. */ +export async function getDockviewContainerWidth(page: Page): Promise { + return page.evaluate(() => + Math.round(document.querySelector(".dv-dockview")?.getBoundingClientRect().width ?? -1), + ); +} + +/** + * Wait for a `setViewportSize` to have reached dockview's layout. + * + * Two conditions, both required, evaluated together in one `page.evaluate` so + * they describe a single instant: + * + * 1. the container's CSS box has moved off the width it had before, and + * 2. dockview's own `api.width` agrees with that box. + * + * (1) alone is not enough, and the reason is subtle. The app syncs a container + * resize into `api.layout` from `setupContainerResizeSync`'s ResizeObserver; + * measured under load (12 saturated cores, 6x CDP CPU throttling, five + * scenarios: auto shrink and grow, manual-width shrink and grow, over-cap + * re-clamp), that sync is already complete at the first ResizeObserver + * delivery reporting the new box, right column included. But a poll does not + * observe via ResizeObserver: `getBoundingClientRect()` forces a synchronous + * reflow, so it can read the post-resize box in a task that runs *before* that + * delivery, and therefore before `api.layout`. Condition (2) closes that + * window by reading the value the sync actually produces. + * + * (2) alone would be the already-true trap: `api.width` matches the box before + * the viewport changes as well, so it is satisfied instantly. Only the pair is + * causal, which is why this is not simply a poll on the expected group width. + * That matters most for the callers asserting a width must **not** change + * across the resize: they have no event of their own, and polling the value + * they expect would pass against the width that was already there. + * + * A 1px tolerance absorbs fractional-vs-rounded differences between the two + * readings; it is far below any real column-width assertion's slack. + */ +export async function waitForDockviewViewportResize( + page: Page, + previousContainerWidth: number, + timeout = 5_000, +): Promise { + await expect + .poll( + () => + page.evaluate((previous) => { + const element = document.querySelector(".dv-dockview"); + const api = (window as unknown as { __dockviewApi__?: { width: number } }) + .__dockviewApi__; + if (!element) return "no dockview container"; + if (!api) return "dockview api not exposed"; + const css = Math.round(element.getBoundingClientRect().width); + if (css === previous) return `container still ${css}px`; + const layout = Math.round(api.width); + if (Math.abs(layout - css) > 1) return `container ${css}px but api.width ${layout}px`; + return "settled"; + }, previousContainerWidth), + { + timeout, + message: `dockview never took the new viewport (was ${previousContainerWidth}px)`, + }, + ) + .toBe("settled"); +} + /** Read the live pixel width of a dockview group by group ID. */ export async function getDockviewGroupWidthById(page: Page, groupId: string): Promise { return page.evaluate((id) => { diff --git a/apps/web/e2e/tests/layout/changes-panel-focus.spec.ts b/apps/web/e2e/tests/layout/changes-panel-focus.spec.ts index 004c7ca5d1..c6add1dae3 100644 --- a/apps/web/e2e/tests/layout/changes-panel-focus.spec.ts +++ b/apps/web/e2e/tests/layout/changes-panel-focus.spec.ts @@ -6,6 +6,7 @@ import fs from "node:fs"; import path from "node:path"; import { execSync } from "node:child_process"; import { waitForSessionDone } from "../../helpers/session"; +import { dwell } from "../../helpers/causal-waits"; /** Minimal git helper for E2E tests - runs git commands in the test repository. */ class GitHelper { @@ -284,8 +285,12 @@ test.describe("Changes panel focus behavior", () => { timeout: 15_000, }); - // Wait a bit for any async auto-activate to fire - await testPage.waitForTimeout(2_000); + await dwell( + testPage, + 2_000, + "negative-assertion", + "the assertion below is that the changes tab does not steal activation; an auto-activate that must never happen publishes nothing, so the only way to give a regression room to fire is to let its window elapse", + ); await expect(changesTab(testPage)).not.toHaveClass(/dv-active-tab/, { timeout: 5_000 }); diff --git a/apps/web/e2e/tests/layout/pane-resize-edge-cases.spec.ts b/apps/web/e2e/tests/layout/pane-resize-edge-cases.spec.ts index 0dccbc8ebd..2642913917 100644 --- a/apps/web/e2e/tests/layout/pane-resize-edge-cases.spec.ts +++ b/apps/web/e2e/tests/layout/pane-resize-edge-cases.spec.ts @@ -33,7 +33,18 @@ test.describe("Pane resize edge cases", () => { if (!matching) throw new Error("files group not found"); matching.api.maximize(); }); - await testPage.waitForTimeout(150); + // Wait for the maximized layout to be applied rather than for a number. + // `hasMaximizedGroup()` flips synchronously inside `maximize()`, so polling + // it would guard nothing; the group growing is the consequence that has to + // land before exiting is a real round trip. Measured, maximize takes this + // group from the 600px set above to ~1000px -- it does not fill the + // container, so this asserts growth rather than a proportion. + await expect + .poll(() => getDockviewGroupWidth(testPage, "files"), { + timeout: 5_000, + message: `files group never grew past its pre-maximize ${before}px`, + }) + .toBeGreaterThan(before + 50); await testPage.evaluate(() => { type GroupApi = { exitMaximized: () => void }; type Group = { id: string; panels: { id: string }[]; api: GroupApi }; @@ -67,10 +78,31 @@ test.describe("Pane resize edge cases", () => { const errors: string[] = []; testPage.on("pageerror", (err) => errors.push(err.message)); - await testPage.locator("body").click({ position: { x: 5, y: 5 } }); - const mod = process.platform === "darwin" ? "Meta" : "Control"; - await testPage.keyboard.press(`${mod}+b`); - await testPage.waitForTimeout(250); + // This used to press Ctrl+B and sleep 250ms. Ctrl+B does nothing: + // `CONFIGURABLE_SHORTCUTS.TOGGLE_SIDEBAR` defaults to `UNBOUND_SHORTCUT`, + // so the binding exists only for users who set one. Nothing downstream + // asserted on the sidebar, so the test has been resizing with the sidebar + // still open -- a duplicate of the case above rather than the edge case it + // is named for. Collapse it through the store instead; the subject here is + // the resize, not how the collapse was triggered. + await testPage.evaluate(() => { + type StoreWindow = Window & { + __KANDEV_E2E_STORE__?: { + getState: () => { setAppSidebarCollapsed: (collapsed: boolean) => void }; + }; + }; + const store = (window as StoreWindow).__KANDEV_E2E_STORE__; + if (!store) throw new Error("E2E store bridge missing"); + store.getState().setAppSidebarCollapsed(true); + }); + // Assert the collapse instead of budgeting for it: this fails if the flag + // stops reaching the DOM, where the old sleep could not. The 300ms width + // animation that follows is deliberately not waited out -- the assertions + // below are "no page error" and "layout still healthy", neither of which + // reads geometry. + await expect(testPage.getByTestId("app-sidebar")).toHaveAttribute("data-collapsed", "true", { + timeout: 5_000, + }); await resizeColumnViaSplitview(testPage, "right", 600); await session.expectLayoutHealthy(); diff --git a/apps/web/e2e/tests/layout/pane-resize-right.spec.ts b/apps/web/e2e/tests/layout/pane-resize-right.spec.ts index f1b6da6ba2..515b5fd454 100644 --- a/apps/web/e2e/tests/layout/pane-resize-right.spec.ts +++ b/apps/web/e2e/tests/layout/pane-resize-right.spec.ts @@ -6,7 +6,9 @@ import { openWideTask, expectApproxWidth, getDockviewGroupWidth, + getDockviewContainerWidth, resizeColumnViaSplitview, + waitForDockviewViewportResize, } from "../../helpers/dockview-resize"; import { KanbanPage } from "../../pages/kanban-page"; import { SessionPage } from "../../pages/session-page"; @@ -42,13 +44,15 @@ test.describe("Right pane resize — container-proportional cap", () => { const largeWidth = await getDockviewGroupWidth(testPage, "files"); expectApproxWidth(largeWidth, 450, 12); + const wideContainer = await getDockviewContainerWidth(testPage); await testPage.setViewportSize({ width: 1280, height: 900 }); - await testPage.waitForTimeout(500); + await waitForDockviewViewportResize(testPage, wideContainer); const laptopWidth = await getDockviewGroupWidth(testPage, "files"); expectApproxWidth(laptopWidth, 288, 12); + const laptopContainer = await getDockviewContainerWidth(testPage); await testPage.setViewportSize({ width: 2200, height: 900 }); - await testPage.waitForTimeout(500); + await waitForDockviewViewportResize(testPage, laptopContainer); expectApproxWidth(await getDockviewGroupWidth(testPage, "files"), largeWidth, 12); }); @@ -64,12 +68,19 @@ test.describe("Right pane resize — container-proportional cap", () => { const manualWidth = await resizeColumnViaSplitview(testPage, "right", 360); expectApproxWidth(manualWidth, 360, 12); + // The assertion is that this width does *not* move, so there is nothing to + // poll for: the expected value is already there. Waiting for the container + // to take the new viewport is what gives a regression somewhere to happen, + // since the recompute that could clobber the manual width runs in that same + // layout pass. + const wideContainer = await getDockviewContainerWidth(testPage); await testPage.setViewportSize({ width: 1280, height: 900 }); - await testPage.waitForTimeout(500); + await waitForDockviewViewportResize(testPage, wideContainer); expectApproxWidth(await getDockviewGroupWidth(testPage, "files"), manualWidth, 12); + const laptopContainer = await getDockviewContainerWidth(testPage); await testPage.setViewportSize({ width: 2200, height: 900 }); - await testPage.waitForTimeout(500); + await waitForDockviewViewportResize(testPage, laptopContainer); expectApproxWidth(await getDockviewGroupWidth(testPage, "files"), manualWidth, 12); }); @@ -186,11 +197,12 @@ test.describe("Right pane resize — container-proportional cap", () => { const wideWidth = await resizeColumnViaSplitview(testPage, "right", 900); expect(wideWidth).toBeGreaterThan(700); + // Do not call the resize helper here: it reapplies constraints and would + // mask a failure to update them automatically on viewport changes. Wait for + // the container instead, which is where the pinned-target enforcement runs. + const wideContainer = await getDockviewContainerWidth(testPage); await testPage.setViewportSize({ width: 1100, height: 800 }); - // Allow the container ResizeObserver and pinned-target enforcement to - // settle. Do not call the resize helper here: it reapplies constraints and - // would mask a failure to update them automatically on viewport changes. - await testPage.waitForTimeout(500); + await waitForDockviewViewportResize(testPage, wideContainer); const dockviewBox = await testPage.locator(".dv-dockview").boundingBox(); expect(dockviewBox).not.toBeNull(); @@ -259,13 +271,18 @@ test.describe("Right pane width — per-task isolation", () => { timeout: 15_000, }); await expect(testPage.locator(".dv-dockview")).toBeVisible({ timeout: 15_000 }); - // Allow fromJSON + fixups-capture + enforcePinnedTargets to settle. - await testPage.waitForTimeout(500); - const widthB = await getDockviewGroupWidth(testPage, "files"); - expect( - widthB, - `Task B right width ${widthB} should be the default (>350), not Task A's narrow ${narrowedA}`, - ).toBeGreaterThan(350); + // Poll rather than sleep-then-read: the width is Task A's narrow value + // until fromJSON + fixups-capture + enforcePinnedTargets have run, so this + // still fails if the leak comes back. Sampled every frame through a real + // switch, the width makes exactly one move (240 -> 384 at ~180ms) and then + // holds for the next 5.8s, so there is no transient correct value for the + // poll to latch onto early. + await expect + .poll(() => getDockviewGroupWidth(testPage, "files"), { + timeout: 10_000, + message: `Task B right width should be the default (>350), not Task A's narrow ${narrowedA}`, + }) + .toBeGreaterThan(350); }); }); diff --git a/apps/web/e2e/tests/layout/preview-tab-session-switch.spec.ts b/apps/web/e2e/tests/layout/preview-tab-session-switch.spec.ts index f9be168897..50c493bc70 100644 --- a/apps/web/e2e/tests/layout/preview-tab-session-switch.spec.ts +++ b/apps/web/e2e/tests/layout/preview-tab-session-switch.spec.ts @@ -6,6 +6,7 @@ import type { ApiClient } from "../../helpers/api-client"; import { SessionPage } from "../../pages/session-page"; import { GitHelper, makeGitEnv } from "../../helpers/git-helper"; import { KanbanPage } from "../../pages/kanban-page"; +import { dwell } from "../../helpers/causal-waits"; const FILE_A = "alpha.ts"; const DONE_STATES = ["COMPLETED", "WAITING_FOR_INPUT"]; @@ -180,9 +181,13 @@ test.describe("Preview tab survives session switch", () => { await expect(testPage).toHaveURL((url) => url.pathname.includes(taskA.id), { timeout: 15_000, }); - // Wait for layout to stabilize after fromJSON restore await expect(testPage.locator(".dv-dockview")).toBeVisible({ timeout: 15_000 }); - await testPage.waitForTimeout(1_000); + await dwell( + testPage, + 1_000, + "negative-assertion", + "the count assertion below auto-retries, so it already covers a slow restore; what it cannot cover is a duplicate tab appearing after it first sees one, and a tab that must never be added publishes nothing to wait on", + ); // File tab should be restored (preview or pinned — the file was open) const fileTab = testPage.locator(".dv-default-tab").filter({ hasText: FILE_A });