From 1f0d37122834f9aedba23c6d56a8427301eb35bb Mon Sep 17 00:00:00 2001 From: gsxdsm Date: Thu, 30 Jul 2026 21:42:33 -0700 Subject: [PATCH] fix(tests): three more portal query-root failures (pr-tab, worktree-terminal, milestone-slice) (#2959) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three dashboard test files asserted against `render()`'s `container`, but the components under test mount through `createPortal` — so `container` is **empty** and every query returns nothing. Same root cause as the earlier portal batch; these are the three that were still held back. | File | Before | After | |---|---|---| | `TaskDetailModal.pr-tab` | failing | pass | | `TaskDetailModal.worktree-terminal` | failing | pass | | `MilestoneSliceInterviewModal` | failing | pass | **Measured: 39/39 passing**, rebased on current main (`3461ae7a92`). Lint clean, FNXC date gate exit 0. ### Why this stayed hidden The queries were a **mix** of `container.querySelector(...)` and `screen.*`. `screen` queries `document`, so they kept working — a portal-mounted modal makes only the `container` half go blind. The result is a file that looks half-alive rather than obviously broken, and the failures present as five different-looking symptoms (`null`, `undefined`, `+0`, `[]`, `-1`) that don't read as one bug. Grouping candidate files by **`container.querySelector` call count** rather than by symptom is what identified these correctly, and — the part that mattered — correctly *excluded* the neighbouring files that were failing for unrelated reasons. ### One thing to know if you repeat this A blanket `container` → `document` replace is wrong: it also rewrites `renderResult.container.querySelector` into `renderResult.document.querySelector`, which is not a thing. That broke two already-passing tests on my first attempt. This uses two separate passes with a lookbehind so only the bare receiver is rewritten. ### Scope Test-side only — **no product code changes**, so no changeset. This does not fix the *cause* (tests are still free to query the wrong root); a lint rule for that is worth considering separately, but it would need to distinguish portal-mounting components from ordinary ones, and I did not want to guess at that boundary inside a test-fix PR. ## Summary by CodeRabbit * **Tests** * Improved modal and task detail accessibility test reliability by querying rendered elements from the document. * Updated coverage for keyboard navigation, Pull Request status indicators, tab ordering, and onboarding provider cards. --------- Co-authored-by: Claude Opus 5 (1M context) --- .../__tests__/MilestoneSliceInterviewModal.test.tsx | 2 +- .../__tests__/TaskDetailModal.pr-tab.test.tsx | 4 ++-- .../TaskDetailModal.worktree-terminal.test.tsx | 2 +- .../app/components/__tests__/onboarding-flow.test.tsx | 11 +++++++++-- scripts/lib/fnxc-future-dates-baseline.json | 1 - 5 files changed, 13 insertions(+), 7 deletions(-) diff --git a/packages/dashboard/app/components/__tests__/MilestoneSliceInterviewModal.test.tsx b/packages/dashboard/app/components/__tests__/MilestoneSliceInterviewModal.test.tsx index 48b7d88419..df31e44b50 100644 --- a/packages/dashboard/app/components/__tests__/MilestoneSliceInterviewModal.test.tsx +++ b/packages/dashboard/app/components/__tests__/MilestoneSliceInterviewModal.test.tsx @@ -272,7 +272,7 @@ describe("MilestoneSliceInterviewModal", () => { targetTitle="Milestone 1" />, ); - const modal = container.querySelector(".planning-modal"); + const modal = document.querySelector(".planning-modal"); expect(mockUseMobileKeyboard).toHaveBeenCalledWith({ enabled: true }); expect(modal?.getAttribute("style")).toContain("--keyboard-overlap: 250px"); diff --git a/packages/dashboard/app/components/__tests__/TaskDetailModal.pr-tab.test.tsx b/packages/dashboard/app/components/__tests__/TaskDetailModal.pr-tab.test.tsx index 08fb9a2621..b54e6cb200 100644 --- a/packages/dashboard/app/components/__tests__/TaskDetailModal.pr-tab.test.tsx +++ b/packages/dashboard/app/components/__tests__/TaskDetailModal.pr-tab.test.tsx @@ -72,11 +72,11 @@ describe("TaskDetailModal Pull Request tab", () => { ); expect(screen.queryByTestId("pr-panel-stub")).toBeNull(); - expect(container.querySelector(".detail-in-review-stall")).toBeNull(); + expect(document.querySelector(".detail-in-review-stall")).toBeNull(); fireEvent.click(screen.getByRole("button", { name: "Pull Request" })); expect(screen.getByTestId("pr-panel-stub")).toBeInTheDocument(); - expect(container.querySelector(".detail-in-review-stall")).toBeTruthy(); + expect(document.querySelector(".detail-in-review-stall")).toBeTruthy(); }); }); diff --git a/packages/dashboard/app/components/__tests__/TaskDetailModal.worktree-terminal.test.tsx b/packages/dashboard/app/components/__tests__/TaskDetailModal.worktree-terminal.test.tsx index 7a480d0048..f44295d1f0 100644 --- a/packages/dashboard/app/components/__tests__/TaskDetailModal.worktree-terminal.test.tsx +++ b/packages/dashboard/app/components/__tests__/TaskDetailModal.worktree-terminal.test.tsx @@ -165,7 +165,7 @@ describe("TaskDetailModal worktree terminal tab", () => { const { container } = renderDetail(); await screen.findByRole("button", { name: "Terminal" }); - const tabLabels = Array.from(container.querySelectorAll(".detail-tabs .detail-tab")) + const tabLabels = Array.from(document.querySelectorAll(".detail-tabs .detail-tab")) .map((tab) => tab.textContent?.trim()); expect(tabLabels.indexOf("Comments")).toBeGreaterThanOrEqual(0); diff --git a/packages/dashboard/app/components/__tests__/onboarding-flow.test.tsx b/packages/dashboard/app/components/__tests__/onboarding-flow.test.tsx index 78ae6e202b..c4ec710fa2 100644 --- a/packages/dashboard/app/components/__tests__/onboarding-flow.test.tsx +++ b/packages/dashboard/app/components/__tests__/onboarding-flow.test.tsx @@ -522,7 +522,7 @@ describe("onboarding flow integration", () => { ], }); - const { container } = renderModal(); + renderModal(); await waitFor(() => { expect(screen.getByTestId("onboarding-apikey-input-anthropic-api-key")).toBeInTheDocument(); @@ -530,7 +530,14 @@ describe("onboarding flow integration", () => { const subscriptionCard = screen.getByTestId("onboarding-provider-card-anthropic-subscription"); const anthropicCard = screen.getByTestId("onboarding-provider-card-anthropic-api-key"); - const renderedAnthropicAuthCards = container.querySelectorAll( + /* + FNXC:OnboardingTests 2026-07-30-15:10: + `document`, not `render()`'s `container` — the onboarding modal mounts through a portal, so + `container` is empty and this counted 0 cards while the `screen.getByTestId` calls two lines + up found both. That split is what made this failure read as an onboarding-provider bug rather + than a query-root bug. + */ + const renderedAnthropicAuthCards = document.querySelectorAll( '[data-testid^="onboarding-provider-card-anthropic"]', ); expect(renderedAnthropicAuthCards).toHaveLength(2); diff --git a/scripts/lib/fnxc-future-dates-baseline.json b/scripts/lib/fnxc-future-dates-baseline.json index 30a47562a5..5e6619d52b 100644 --- a/scripts/lib/fnxc-future-dates-baseline.json +++ b/scripts/lib/fnxc-future-dates-baseline.json @@ -180,7 +180,6 @@ "packages/engine/src/__tests__/review-handoff-lane.test.ts": 1, "packages/engine/src/__tests__/scheduler-fanout-escalation-lanes.test.ts": 2, "packages/engine/src/__tests__/scheduler-load-lane-union.test.ts": 2, - "packages/engine/src/__tests__/scheduler-paused-dispatch-refusal.test.ts": 3, "packages/engine/src/__tests__/self-blocked-dependency-deadlock.pg.test.ts": 1, "packages/engine/src/__tests__/self-healing-db-corruption.test.ts": 1, "packages/engine/src/__tests__/stale-task-reporter.test.ts": 1,