From de677b231a623a7dfbd4a5eeefa8d44fef47d2b7 Mon Sep 17 00:00:00 2001 From: gsxdsm Date: Thu, 30 Jul 2026 17:59:18 -0700 Subject: [PATCH] fix(tests): settings-mobile queries container; SettingsModal renders through a portal (#2890) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## What this clears All **17 failures** in `settings-mobile.test.tsx` — the second-largest block in the dashboard `app:backfill 3/4` shard after the TaskDetailModal file (#2885), and the **same root cause**. ## Probed, not assumed Every failure read `expected null to be truthy`, which looks like the modal never rendered: ``` PROBE container.settings-layout=false document.settings-layout=true document.modal=true bodyLen=36637 ``` `SettingsModal` mounts through `createPortal`, so its subtree hangs off `document.body`, not the container `render()` returns. The markup is there; the container-rooted lookup cannot see it. ## Nine of these assertions could never have failed They are **absence** checks: ```ts expect(container.querySelector(".settings-scope-banner")).toBeNull(); expect(container.querySelector("#settings-mobile-section")).toBeNull(); ``` `container` is empty for this component no matter what, so these passed on an **empty root** rather than on absence — they would have kept passing if the element appeared. Converting them makes them mean what they say. All nine still pass, so they were correct, just unproven. ## Two rounds of my own errors, both caught by measuring 1. A blanket `container` → `document` replace also rewrote `renderResult.container.querySelector` into `renderResult.document...` — **not a thing**. That broke the two *"embedded Settings surface"* star tests. Because the embedded surface is genuinely not portalled, this first read as *"embedded needs container"*. It doesn't; the JS was simply invalid. 2. Fixed by restoring those seven, then converting them to **bare `document`** once I confirmed each test unmounts its surface before rendering the next (`modalRender.unmount()` precedes the embedded render), so a document-rooted query cannot match a stale instance. **Verified no regressions rather than assuming** — diffed the failing-test list before and after: 13 fixed / 0 new, then the remaining 4 fixed. Every failure in the final state was already failing at the start. ## Evidence | | result | |---|---| | the file | **41/41** (was 17 failed) | | shard `3/4` | **55 → 38** with this change alone | | mutation: rename `.settings-layout` in `SettingsModal` | **1 failed** | `pnpm lint` clean. Test-only; `SettingsModal.tsx` restored clean. ## Together with #2885 #2885 clears the TaskDetailModal file (30). Both are the same portal/query-root defect in different files, and both were sitting behind `app:app` in the runner's fail-fast — which is why they went unnoticed. Shard `3/4` should land at **~8** with both applied. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 5 (1M context) --- .../__tests__/settings-mobile.test.tsx | 76 +++++++++---------- 1 file changed, 38 insertions(+), 38 deletions(-) diff --git a/packages/dashboard/app/components/__tests__/settings-mobile.test.tsx b/packages/dashboard/app/components/__tests__/settings-mobile.test.tsx index 4d86d21a4f..2fbc2ac8b0 100644 --- a/packages/dashboard/app/components/__tests__/settings-mobile.test.tsx +++ b/packages/dashboard/app/components/__tests__/settings-mobile.test.tsx @@ -285,9 +285,9 @@ describe("SettingsModal mobile adaptations", () => { const { container } = render(); await waitFor(() => expect(fetchSettings).toHaveBeenCalled()); - expect(container.querySelector(".settings-layout")).toBeTruthy(); - expect(container.querySelector(".settings-sidebar")).toBeTruthy(); - expect(container.querySelector(".settings-content")).toBeTruthy(); + expect(document.querySelector(".settings-layout")).toBeTruthy(); + expect(document.querySelector(".settings-sidebar")).toBeTruthy(); + expect(document.querySelector(".settings-content")).toBeTruthy(); }); /* @@ -307,7 +307,7 @@ describe("SettingsModal mobile adaptations", () => { await waitFor(() => expect(fetchSettings).toHaveBeenCalled()); const resetBtn = await findByTestId("settings-reset"); - expect(container.querySelector(".modal-actions")?.contains(resetBtn)).toBe(true); + expect(document.querySelector(".modal-actions")?.contains(resetBtn)).toBe(true); expect(resetBtn).toHaveTextContent(/^Reset$/); expect(resetBtn).not.toHaveTextContent("Reset Settings"); @@ -356,8 +356,8 @@ describe("SettingsModal mobile adaptations", () => { await waitFor(() => expect(fetchSettings).toHaveBeenCalled()); const version = await findByText("v1.2.3"); - const modalActions = container.querySelector(".modal-actions"); - const modalHeader = container.querySelector(".modal-header"); + const modalActions = document.querySelector(".modal-actions"); + const modalHeader = document.querySelector(".modal-header"); expect(version).toBeTruthy(); expect(queryByText("Version 1.2.3")).toBeNull(); @@ -373,7 +373,7 @@ describe("SettingsModal mobile adaptations", () => { const version = await findByText("Version 1.2.3"); expect(version).toBeTruthy(); expect(queryByText("v1.2.3")).toBeNull(); - expect(container.querySelector(".modal-actions")?.contains(version)).toBe(true); + expect(document.querySelector(".modal-actions")?.contains(version)).toBe(true); }); it("keeps update-check button clickable from the standalone and embedded mobile footers", async () => { @@ -382,7 +382,7 @@ describe("SettingsModal mobile adaptations", () => { const standalone = render(); await waitFor(() => expect(fetchSettings).toHaveBeenCalled()); - const standaloneActions = standalone.container.querySelector(".settings-modal:not(.settings-modal--embedded) .modal-actions"); + const standaloneActions = document.querySelector(".settings-modal:not(.settings-modal--embedded) .modal-actions"); expect(standaloneActions).toBeTruthy(); const standaloneUpdateButton = within(standaloneActions as HTMLElement).getByRole("button", { name: "Check for updates" }); @@ -395,7 +395,7 @@ describe("SettingsModal mobile adaptations", () => { const embedded = render(); await waitFor(() => expect(fetchSettings).toHaveBeenCalled()); - const embeddedActions = embedded.container.querySelector(".settings-modal--embedded .modal-actions"); + const embeddedActions = document.querySelector(".settings-modal--embedded .modal-actions"); expect(embeddedActions).toBeTruthy(); const embeddedUpdateButton = within(embeddedActions as HTMLElement).getByRole("button", { name: "Check for updates" }); await user.click(embeddedUpdateButton); @@ -409,12 +409,12 @@ describe("SettingsModal mobile adaptations", () => { await waitFor(() => expect(fetchSettings).toHaveBeenCalled()); await waitFor(() => expect(fetchDashboardHealth).toHaveBeenCalled()); - const modalActions = container.querySelector(".settings-modal:not(.settings-modal--embedded) .modal-actions"); + const modalActions = document.querySelector(".settings-modal:not(.settings-modal--embedded) .modal-actions"); expect(modalActions).toBeTruthy(); expect(within(modalActions as HTMLElement).getByRole("link", { name: "Help and discussions" })).toBeTruthy(); expect(queryByRole("button", { name: "Check for updates" })).toBeNull(); - expect(container.querySelector(".settings-modal-footer-version")).toBeTruthy(); - expect(container.querySelector(".settings-update-check")).toBeTruthy(); + expect(document.querySelector(".settings-modal-footer-version")).toBeTruthy(); + expect(document.querySelector(".settings-update-check")).toBeTruthy(); }); it("keeps update-now button reachable from the mobile footer", async () => { @@ -423,7 +423,7 @@ describe("SettingsModal mobile adaptations", () => { const { container, findByRole, findByText } = render(); await waitFor(() => expect(fetchSettings).toHaveBeenCalled()); - const modalActions = container.querySelector(".modal-actions"); + const modalActions = document.querySelector(".modal-actions"); expect(modalActions).toBeTruthy(); await user.click(within(modalActions as HTMLElement).getByRole("button", { name: "Check for updates" })); @@ -434,7 +434,7 @@ describe("SettingsModal mobile adaptations", () => { full-width row directly above it — inside the rail it clipped itself and pushed Import/Export/Reset/Close off-screen. Assert both halves of that invariant: banner outside the rail, banner present in the footer row. */ - const updateRow = container.querySelector(".settings-modal-footer-update-row"); + const updateRow = document.querySelector(".settings-modal-footer-update-row"); expect(updateRow).toBeTruthy(); expect((updateRow as HTMLElement).contains(updateNow)).toBe(true); expect((modalActions as HTMLElement).contains(updateNow)).toBe(false); @@ -451,15 +451,15 @@ describe("SettingsModal mobile adaptations", () => { const { container, findByRole } = render(); await waitFor(() => expect(fetchSettings).toHaveBeenCalled()); - const modalActions = container.querySelector(".modal-actions"); + const modalActions = document.querySelector(".modal-actions"); expect(modalActions).toBeTruthy(); await user.click(within(modalActions as HTMLElement).getByRole("button", { name: "Check for updates" })); const updateNow = await findByRole("button", { name: "Update now" }); - expect(container.querySelector(".settings-modal-footer-update-row")).toBeNull(); + expect(document.querySelector(".settings-modal-footer-update-row")).toBeNull(); expect((modalActions as HTMLElement).contains(updateNow)).toBe(true); - expect(container.querySelector(".settings-update-check .settings-update-result")).toBeTruthy(); + expect(document.querySelector(".settings-update-check .settings-update-result")).toBeTruthy(); }); it("preserves the mobile section picker accessible name without rendering a visible label", async () => { @@ -470,7 +470,7 @@ describe("SettingsModal mobile adaptations", () => { const picker = getByLabelText("Settings Section") as HTMLSelectElement; expect(picker.id).toBe("settings-mobile-section"); expect(picker.getAttribute("aria-label")).toBe("Settings Section"); - expect(container.querySelector('label[for="settings-mobile-section"]')).toBeNull(); + expect(document.querySelector('label[for="settings-mobile-section"]')).toBeNull(); expect(queryByText("Settings Section", { selector: "label" })).toBeNull(); expect(picker.closest(".settings-mobile-section-picker")?.querySelector(".settings-scope-icon")).toBeNull(); }); @@ -500,7 +500,7 @@ describe("SettingsModal mobile adaptations", () => { await user.selectOptions(picker, "cli-binary"); expect(await findByText(/Installing the global CLI lets you run fn and fusion/)).toBeTruthy(); - expect(container.querySelector(".cli-binary-panel")).toBeTruthy(); + expect(document.querySelector(".cli-binary-panel")).toBeTruthy(); }); it("excludes research sections from mobile picker when researchView is disabled", async () => { @@ -663,9 +663,9 @@ describe("SettingsModal mobile adaptations", () => { const { container } = render(); await waitFor(() => expect(fetchSettings).toHaveBeenCalled()); - const navItems = container.querySelectorAll(".settings-nav-item"); + const navItems = document.querySelectorAll(".settings-nav-item"); expect(navItems.length).toBeGreaterThan(0); - expect(container.querySelector(".settings-nav-item.active")).toBeTruthy(); + expect(document.querySelector(".settings-nav-item.active")).toBeTruthy(); }); it("renders form controls inside settings-content for 16px mobile targeting", async () => { @@ -683,7 +683,7 @@ describe("SettingsModal mobile adaptations", () => { const generalTabs = await findAllByText("General · Project"); await user.click(generalTabs[0]); - const controls = container.querySelectorAll(".settings-content input, .settings-content select, .settings-content textarea"); + const controls = document.querySelectorAll(".settings-content input, .settings-content select, .settings-content textarea"); expect(controls.length).toBeGreaterThan(0); }); @@ -697,19 +697,19 @@ describe("SettingsModal mobile adaptations", () => { const { container, getByRole } = render(); await waitFor(() => expect(fetchSettings).toHaveBeenCalled()); - expect(container.querySelectorAll(".settings-scope-icon").length).toBeGreaterThan(0); + expect(document.querySelectorAll(".settings-scope-icon").length).toBeGreaterThan(0); await user.click(getByRole("button", { name: /Appearance$/ })); // The banner is gone for good — it asserted a single scope for a section // that genuinely mixes them. - expect(container.querySelector(".settings-scope-banner")).toBeNull(); - expect(container.querySelector(".settings-scope-project")).toBeNull(); - expect(container.querySelector(".settings-scope-global")).toBeNull(); + expect(document.querySelector(".settings-scope-banner")).toBeNull(); + expect(document.querySelector(".settings-scope-project")).toBeNull(); + expect(document.querySelector(".settings-scope-global")).toBeNull(); // Appearance is exactly the mixed case: global theme controls above, // project-scoped task-presentation toggles below, each badged for itself. - const badges = container.querySelectorAll('[data-testid="settings-field-row-scope"]'); + const badges = document.querySelectorAll('[data-testid="settings-field-row-scope"]'); expect(badges.length).toBeGreaterThan(0); expect(Array.from(badges).map((b) => b.textContent)).toContain("project"); }); @@ -741,7 +741,7 @@ describe("SettingsModal mobile adaptations", () => { expect(await findByText("ntfy")).toBeTruthy(); expect(await findByText("Webhook")).toBeTruthy(); - expect(container.querySelectorAll(".notification-provider-card").length).toBeGreaterThan(1); + expect(document.querySelectorAll(".notification-provider-card").length).toBeGreaterThan(1); }); it("keeps the GitHub star count visible in the mobile Settings header", async () => { @@ -755,7 +755,7 @@ describe("SettingsModal mobile adaptations", () => { const modalRender = render(); await waitFor(() => expect(fetchSettings).toHaveBeenCalled()); - const modalCount = modalRender.container.querySelector(".settings-github-star-btn__count"); + const modalCount = document.querySelector(".settings-github-star-btn__count"); expect(modalCount).toBeTruthy(); expect(modalCount?.textContent).toBe("1.2k"); modalRender.unmount(); @@ -763,7 +763,7 @@ describe("SettingsModal mobile adaptations", () => { vi.mocked(fetchSettings).mockClear(); const embeddedRender = render(); await waitFor(() => expect(fetchSettings).toHaveBeenCalled()); - const embeddedCount = embeddedRender.container.querySelector(".settings-github-star-btn__count"); + const embeddedCount = document.querySelector(".settings-github-star-btn__count"); expect(embeddedCount).toBeTruthy(); expect(embeddedCount?.textContent).toBe("1.2k"); embeddedRender.unmount(); @@ -786,10 +786,10 @@ describe("SettingsModal mobile adaptations", () => { const renderResult = render(); await waitFor(() => expect(fetchSettings).toHaveBeenCalled()); await waitFor(() => expect(githubFetch).toHaveBeenCalledTimes(1)); - expect(renderResult.container.querySelector(".settings-github-star-btn__count")?.textContent).toBe("999"); + expect(document.querySelector(".settings-github-star-btn__count")?.textContent).toBe("999"); resolveGitHubFetch?.({ ok: true, json: async () => ({ stargazers_count: 123 }) } as Response); - await waitFor(() => expect(renderResult.container.querySelector(".settings-github-star-btn__count")?.textContent).toBe("123")); + await waitFor(() => expect(document.querySelector(".settings-github-star-btn__count")?.textContent).toBe("123")); renderResult.unmount(); }); @@ -812,7 +812,7 @@ describe("SettingsModal mobile adaptations", () => { setDocumentHidden(false); document.dispatchEvent(new Event("visibilitychange")); - await waitFor(() => expect(renderResult.container.querySelector(".settings-github-star-btn__count")?.textContent).toBe("456")); + await waitFor(() => expect(document.querySelector(".settings-github-star-btn__count")?.textContent).toBe("456")); expect(githubFetch).toHaveBeenCalledTimes(1); renderResult.unmount(); }); @@ -1070,7 +1070,7 @@ describe("SettingsModal mobile adaptations", () => { expect(getByTestId("settings-search-input")).toBeTruthy(); expect(queryByLabelText("Show search")).toBeNull(); expect(queryByLabelText("Hide search")).toBeNull(); - expect(container.querySelector(".settings-mobile-section-picker .settings-search-toggle")).toBeNull(); + expect(document.querySelector(".settings-mobile-section-picker .settings-search-toggle")).toBeNull(); }); it("keeps the inline toggle reachable when mobile search has no section results", async () => { @@ -1164,8 +1164,8 @@ describe("SettingsModal mobile adaptations", () => { await user.clear(getByTestId("settings-search-input")); await user.type(getByTestId("settings-search-input"), "zzzzzz-no-match"); await findByText("No sections match this search."); - expect(container.querySelector("#settings-mobile-section")).toBeNull(); - expect(container.querySelector(".settings-mobile-section-picker optgroup")).toBeNull(); + expect(document.querySelector("#settings-mobile-section")).toBeNull(); + expect(document.querySelector(".settings-mobile-section-picker optgroup")).toBeNull(); }); it("leaves desktop navigation as the sidebar without a mobile picker", async () => { @@ -1173,8 +1173,8 @@ describe("SettingsModal mobile adaptations", () => { const { container } = render(); await waitFor(() => expect(fetchSettings).toHaveBeenCalled()); - expect(container.querySelector(".settings-sidebar")).toBeTruthy(); - expect(container.querySelector(".settings-mobile-section-picker")).toBeNull(); + expect(document.querySelector(".settings-sidebar")).toBeTruthy(); + expect(document.querySelector(".settings-mobile-section-picker")).toBeNull(); }); }); });