From 6386be66260e47eda9fcd85cbf7643895b2aaec6 Mon Sep 17 00:00:00 2001 From: gsxdsm Date: Thu, 30 Jul 2026 08:51:44 -0700 Subject: [PATCH] =?UTF-8?q?test(dashboard):=20four=20more=20causes=20in=20?= =?UTF-8?q?components-b=20(14=20=E2=86=92=200)=20=E2=80=94=20incl.=20a=20p?= =?UTF-8?q?arity=20guard=20jsdom=2027=20made=20vacuous=20(#2743)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Four distinct causes | # | Cause | Files | Fixed | |---|---|---|---:| | 1 | **Portal** — `container` is empty; modal renders via `createPortal` | `TaskDetailModal` | 6 | | 2 | **Renamed testid** — `wf-add-step-modal` no longer exists | `WorkflowNodeEditor` | 3 | | 3 | **Unresolvable CSS** — jsdom can't resolve `currentColor` | `SecretsView` | 4 | | 4 | **jsdom 29 initial values** — `auto` vs `""` | `TaskCard.badge-wrap` | 1 | **1. Portal (6).** Same cause and fix as #2735 — 13 queries moved to `document`. The symptom pointed away from it: assertions failed with *"received value must be an HTMLElement / Received has value: null"* on the **element**, while `screen.getByTestId` in the same test kept working, because `screen` queries `document`. **2. Renamed testid (3).** `wf-add-step-modal` exists nowhere in app source — verified by grep, not inferred. The add-step dialog is a `FloatingWindow` now (`WorkflowAddStepModal.tsx:145`, `windowKey="workflow-add-step"`), so the stable id is `floating-window-workflow-add-step`. It still scopes the `within(dialog)` queries, so those keep their precision. **3. Unresolvable CSS (4).** `SecretsView` compared `getComputedStyle(svg).stroke` against the button's background. `SecretsView.css:227` sets `stroke: currentColor`, which jsdom does not resolve — every icon returned `rgba(0, 0, 0, 0)`, **equal to** the transparent button background. The comparison was two unresolved values matching each other, not a visibility check. `currentColor` *is* the element's `color`, which jsdom does compute, so it now asserts the same invariant through a property that resolves. **Load-bearing, measured:** adding `color: rgba(0,0,0,0)` to the icon rule fails exactly those 4. **4. jsdom 29 initial values (1).** `.card-menu-btn` declares no `min-height`, and `auto` is the CSS **initial** value — jsdom 29 reports it where 27 returned `""`. The intent ("nothing constrains the button's height") is what `auto` states; `""` was pinning a jsdom-27 quirk. | Check | Result | |---|---| | `TaskDetailModal` / `WorkflowNodeEditor` / `SecretsView` / `badge-wrap` | **51 / 179 / 14 / 20 passed** | | `pnpm lint`, dashboard app `tsc` | clean | ## Flagged, not forced — and it's the interesting one `TaskCard.test.tsx`'s 2 remaining failures. *"FN-4511 keeps GitHub badge and timer chip geometry in parity"* reads border widths through `githubStyles.borderTopWidth || "1px"`. **Under jsdom 27 both sides returned `""` and both defaulted to `"1px"` — so the parity assertion passed while comparing nothing.** jsdom 29 resolves them and they differ: - the chip's `border: var(--btn-border-width) solid transparent` (`TaskCard.css:1082`) reports `medium`, because jsdom cannot resolve `var()` inside a shorthand; - `.card-github-badge` — which has **no rule in TaskCard.css**, only the class in `TaskCard.tsx` — reports `1px` from elsewhere. So jsdom cannot adjudicate this parity at all, and whether the two genuinely differ *visually* is a question for the e2e screenshot suite. Restoring a `|| fallback` would rebuild a vacuous guard; changing the CSS to satisfy a test limitation would alter the product to fit its harness. Left for someone who can answer it in a real browser. Worth noting the general shape: the jsdom 27 → 29 bump did not "break" these tests so much as **stop hiding** what two of them were failing to check. ## Branch arithmetic This branch is off `main`, so `components-b` still shows 52 failures here: **50 are `inline-editing`, fixed by #2735** on its own branch, plus the 2 flagged above. Once both land, the group is at 2. Across #2735, #2740 and this PR, dashboard goes from **88 failures / 9 files** to **3** — the 2 above plus the GitHub-tracking affordance question flagged in #2735. Per #2732 these lanes are still never executed in CI, since the shard aborts on the first failing package. ## Summary by CodeRabbit * **Tests** * Updated dashboard component tests to account for portal-based modal rendering, ensuring queries target the global document. * Refined jsdom assertions for icon visibility/styling and card header control sizing to match real intended behavior. * Adjusted workflow editor tests for the new add-step dialog, using updated stable identifiers and updated interaction/close checks. * Added clarifying comments to document jsdom-specific limitations and expected outcomes. --- .../components/__tests__/SecretsView.test.tsx | 33 +++++++++++++++- .../__tests__/TaskCard.badge-wrap.test.tsx | 9 ++++- .../__tests__/TaskDetailModal.test.tsx | 39 ++++++++++++------- .../__tests__/WorkflowNodeEditor.test.tsx | 33 +++++++++++++--- 4 files changed, 93 insertions(+), 21 deletions(-) diff --git a/packages/dashboard/app/components/__tests__/SecretsView.test.tsx b/packages/dashboard/app/components/__tests__/SecretsView.test.tsx index 0a02c5dbc1..536b2dccf9 100644 --- a/packages/dashboard/app/components/__tests__/SecretsView.test.tsx +++ b/packages/dashboard/app/components/__tests__/SecretsView.test.tsx @@ -44,6 +44,18 @@ function restoreClipboardMocks() { Object.defineProperty(document, "execCommand", { configurable: true, value: originalExecCommand }); } +/* +FNXC:Secrets 2026-07-30-10:10: +Read the DECLARED stroke for `.secrets-action-icon` out of the real stylesheet. jsdom cannot resolve +`currentColor`, so the declaration is the only place the paint source is observable in this +environment. +*/ +function secretsActionIconStrokeDeclaration(): string | undefined { + const css = loadAllAppCss(); + const rule = /\.secrets-action-icon\s*\{([^}]*)\}/.exec(css); + return /stroke:\s*([^;]+);/.exec(rule?.[1] ?? "")?.[1]?.trim(); +} + function expectVisibleActionIcon(button: HTMLElement) { const svg = button.querySelector("svg"); expect(svg).not.toBeNull(); @@ -54,7 +66,26 @@ function expectVisibleActionIcon(button: HTMLElement) { expect(Number.parseFloat(svgStyle.height)).toBeGreaterThan(0); expect(svgStyle.display).toBe("block"); expect(svgStyle.stroke.toLowerCase()).not.toBe("none"); - expect(svgStyle.stroke).not.toBe(buttonStyle.backgroundColor); + /* + FNXC:Secrets 2026-07-30-10:10 (greptile P2 — the colour check alone was not enough): + Two halves, because neither is sufficient on its own and jsdom cannot resolve the paint directly. + + `SecretsView.css:227` declares `stroke: currentColor`, and jsdom does NOT resolve `currentColor` — + `getComputedStyle().stroke` returns `rgba(0, 0, 0, 0)` for every icon. That is also the icon + button's transparent background, so the ORIGINAL assertion (`stroke !== backgroundColor`) was two + unresolved values matching each other rather than a visibility check. + + Comparing the resolved `color` fixes that, but on its own it would still pass if the rule regressed + to `stroke: transparent`: the paint would be invisible while `color` stayed fine and + `stroke !== "none"` also held (transparent is not none). So the paint SOURCE is asserted from the + stylesheet — the rule must still take its stroke from `currentColor` — and the resolved `color` + must differ from the button background. Together: the icon is painted in `color`, and `color` is + not the button's own background. + + True pixel visibility remains the e2e screenshot suite's job. + */ + expect(secretsActionIconStrokeDeclaration()).toBe("currentColor"); + expect(svgStyle.color).not.toBe(buttonStyle.backgroundColor); } // FNXC:Secrets 2026-06-23-01:30: The cross-node sync passphrase status/actions now live behind a collapsed-by-default diff --git a/packages/dashboard/app/components/__tests__/TaskCard.badge-wrap.test.tsx b/packages/dashboard/app/components/__tests__/TaskCard.badge-wrap.test.tsx index 50e87da893..47a302a23f 100644 --- a/packages/dashboard/app/components/__tests__/TaskCard.badge-wrap.test.tsx +++ b/packages/dashboard/app/components/__tests__/TaskCard.badge-wrap.test.tsx @@ -160,7 +160,14 @@ function expectHeaderActionsControlCenterline(container: HTMLElement, expected: expect(menuStyles.alignItems).toBe("center"); expect(menuStyles.justifyContent).toBe("center"); expect(menuStyles.lineHeight).toBe("1"); - expect(menuStyles.minHeight).toBe(""); + /* + FNXC:TaskCardBadges 2026-07-30-09:30: + `auto` IS the unset value. `.card-menu-btn` declares no `min-height` (TaskCard.css:1645), and + `auto` is the CSS initial value for it — jsdom 29 reports that spec-correct initial where jsdom 27 + returned the empty string. The intent here is "nothing constrains the button's height", which + `auto` states; asserting `""` was pinning a jsdom-27 quirk rather than a style fact. + */ + expect(menuStyles.minHeight).toBe("auto"); } else { expect(menu).toBeNull(); } diff --git a/packages/dashboard/app/components/__tests__/TaskDetailModal.test.tsx b/packages/dashboard/app/components/__tests__/TaskDetailModal.test.tsx index d299217cf7..dcced5c167 100644 --- a/packages/dashboard/app/components/__tests__/TaskDetailModal.test.tsx +++ b/packages/dashboard/app/components/__tests__/TaskDetailModal.test.tsx @@ -20,6 +20,19 @@ import { } from "./TaskDetailModal.test-helpers"; import { TaskDetailContent, TaskDetailModal } from "../TaskDetailModal"; +/* +FNXC:FloatingWindow 2026-07-30-08:30: +Queries run against `document`, not the `container` `render()` returns. `TaskDetailModal` renders +inside `FloatingWindow`, which uses `createPortal`, so its DOM lands on document.body and `container` +is EMPTY — every `container.querySelector` returned null. The symptom was misleading: assertions +failed with "received value must be an HTMLElement" / "Received has value: null" on the ELEMENT, +while `screen.getByTestId` in the same test kept working (screen queries document). + +`document` is correct for both shapes here — `container` is itself inside `document` — so tests that +render a child component directly are unaffected. Same cause and fix as +TaskDetailModal.inline-editing-and-integrations.test.tsx. +*/ + vi.mock("../BranchGroupCard", () => ({ BranchGroupCard: ({ groupId, taskId, onBranchGroupReset }: { groupId: string; taskId?: string; onBranchGroupReset?: () => void }) => { const [expanded, setExpanded] = React.useState(false); @@ -187,7 +200,7 @@ describe("TaskDetailModal planner Chat tab", () => { it("defaults planner Chat to collapsed mode and lets the in-view control expand it", async () => { const user = userEvent.setup(); const { container } = renderTask("todo"); - const detail = container.querySelector(".task-detail-content"); + const detail = document.querySelector(".task-detail-content"); const toggle = screen.getByTestId("task-planner-chat-expand-toggle"); expect(detail).not.toHaveClass("task-detail-content--planner-chat-expanded"); @@ -216,7 +229,7 @@ describe("TaskDetailModal planner Chat tab", () => { addToast={noop} />, ); - const detail = container.querySelector(".task-detail-content"); + const detail = document.querySelector(".task-detail-content"); await user.click(screen.getByTestId("task-planner-chat-expand-toggle")); expect(detail).toHaveClass("task-detail-content--planner-chat-expanded"); @@ -240,12 +253,12 @@ describe("TaskDetailModal planner Chat tab", () => { it("keeps Activity expansion independent from planner Chat expansion", async () => { const user = userEvent.setup(); const { container } = renderTask("todo", "chat"); - const detail = container.querySelector(".task-detail-content"); + const detail = document.querySelector(".task-detail-content"); await user.click(screen.getByTestId("task-chat-expand-toggle")); expect(detail).toHaveClass("task-detail-content--chat-expanded"); - const chatTab = container.querySelectorAll(".detail-tabs .detail-tab")[0]; + const chatTab = document.querySelectorAll(".detail-tabs .detail-tab")[0]; expect(chatTab?.textContent?.trim()).toBe("Chat"); fireEvent.click(chatTab!); expect(detail).not.toHaveClass("task-detail-content--planner-chat-expanded"); @@ -271,25 +284,25 @@ describe("TaskDetailModal planner Chat tab", () => { addToast={noop} />, ); - const detail = container.querySelector(".task-detail-content"); + const detail = document.querySelector(".task-detail-content"); expect(screen.getByText("Task Failed")).toBeInTheDocument(); expect(screen.getByText("Planner failed hard")).toBeInTheDocument(); - expect(container.querySelector(".detail-error-alert")).toBeInTheDocument(); + expect(document.querySelector(".detail-error-alert")).toBeInTheDocument(); await user.click(screen.getByTestId("task-planner-chat-expand-toggle")); expect(detail).toHaveClass("task-detail-content--planner-chat-expanded"); expect(screen.queryByText("Task Failed")).not.toBeInTheDocument(); expect(screen.queryByText("Planner failed hard")).not.toBeInTheDocument(); - expect(container.querySelector(".detail-error-alert")).toBeNull(); + expect(document.querySelector(".detail-error-alert")).toBeNull(); await user.click(screen.getByTestId("task-planner-chat-expand-toggle")); expect(detail).not.toHaveClass("task-detail-content--planner-chat-expanded"); expect(screen.getByText("Task Failed")).toBeInTheDocument(); expect(screen.getByText("Planner failed hard")).toBeInTheDocument(); - expect(container.querySelector(".detail-error-alert")).toBeInTheDocument(); + expect(document.querySelector(".detail-error-alert")).toBeInTheDocument(); }); it("keeps failed-task banner visible while Activity is expanded", async () => { @@ -307,7 +320,7 @@ describe("TaskDetailModal planner Chat tab", () => { addToast={noop} />, ); - const detail = container.querySelector(".task-detail-content"); + const detail = document.querySelector(".task-detail-content"); expect(screen.getByText("Task Failed")).toBeInTheDocument(); expect(screen.getByText("Activity failure stays visible")).toBeInTheDocument(); @@ -318,7 +331,7 @@ describe("TaskDetailModal planner Chat tab", () => { expect(detail).not.toHaveClass("task-detail-content--planner-chat-expanded"); expect(screen.getByText("Task Failed")).toBeInTheDocument(); expect(screen.getByText("Activity failure stays visible")).toBeInTheDocument(); - expect(container.querySelector(".detail-error-alert")).toBeInTheDocument(); + expect(document.querySelector(".detail-error-alert")).toBeInTheDocument(); }); it("renders an actionable generic failed-task alert without an empty message shell", async () => { @@ -340,7 +353,7 @@ describe("TaskDetailModal planner Chat tab", () => { expect(screen.getByText("Task Failed")).toBeInTheDocument(); expect(screen.getByText("The task failed before it could complete.")).toBeInTheDocument(); - expect(container.querySelector(".detail-error-message")?.textContent).not.toBe(""); + expect(document.querySelector(".detail-error-message")?.textContent).not.toBe(""); await userEvent.setup().click(screen.getByRole("button", { name: "Retry" })); expect(onRetryTask).toHaveBeenCalledWith("FN-099"); @@ -359,7 +372,7 @@ describe("TaskDetailModal planner Chat tab", () => { ); expect(screen.queryByText("Task Failed")).not.toBeInTheDocument(); - expect(container.querySelector(".detail-error-alert")).toBeNull(); + expect(document.querySelector(".detail-error-alert")).toBeNull(); }); it("surfaces the latest tool error detail and stages a model override before retrying", async () => { @@ -697,7 +710,7 @@ describe("TaskDetailModal Activity feed loading", () => { const { container } = renderActivityFeedModal(makeSlimTask()); await screen.findByText("newer entry"); - const actions = Array.from(container.querySelectorAll(".detail-log-action")).map((node) => node.textContent); + const actions = Array.from(document.querySelectorAll(".detail-log-action")).map((node) => node.textContent); expect(actions).toEqual(["newer entry", "older entry"]); expect(screen.queryByText("(no activity)")).not.toBeInTheDocument(); }); diff --git a/packages/dashboard/app/components/__tests__/WorkflowNodeEditor.test.tsx b/packages/dashboard/app/components/__tests__/WorkflowNodeEditor.test.tsx index e7d60fad25..0db33f2db8 100644 --- a/packages/dashboard/app/components/__tests__/WorkflowNodeEditor.test.tsx +++ b/packages/dashboard/app/components/__tests__/WorkflowNodeEditor.test.tsx @@ -4265,7 +4265,14 @@ describe("WorkflowNodeEditor simplified view modes", () => { await screen.findByTestId("wf-simple-canvas"); fireEvent.click(screen.getByTestId("wf-simple-toolbar-add-step")); - const dialog = await screen.findByTestId("wf-add-step-modal"); + /* + FNXC:FloatingWindow 2026-07-30-08:50: + The add-step dialog is a FloatingWindow now (WorkflowAddStepModal.tsx:145, + `windowKey="workflow-add-step"`), so its stable testid is the window's + `floating-window-workflow-add-step`. The old `wf-add-step-modal` testid does not exist anywhere in + app source — verified by grep, not inferred — so these queries could never resolve. + */ + const dialog = await screen.findByTestId("floating-window-workflow-add-step"); expect(within(dialog).getByText("Agent steps")).toBeInTheDocument(); expect(within(dialog).getByText("Automation")).toBeInTheDocument(); expect(within(dialog).getByText("Flow control")).toBeInTheDocument(); @@ -4275,7 +4282,7 @@ describe("WorkflowNodeEditor simplified view modes", () => { expect(within(dialog).queryByTestId("wf-add-step-prompt-prompt")).not.toBeInTheDocument(); fireEvent.click(within(dialog).getByTestId("wf-add-step-script-script")); - await waitFor(() => expect(screen.queryByTestId("wf-add-step-modal")).not.toBeInTheDocument()); + await waitFor(() => expect(screen.queryByTestId("floating-window-workflow-add-step")).not.toBeInTheDocument()); // def() has an unambiguous edge into end, so the pick inserts there and // the new node lands selected with the inspector open. expect(await screen.findByTestId("wf-node-inspector")).toBeInTheDocument(); @@ -4310,9 +4317,16 @@ describe("WorkflowNodeEditor simplified view modes", () => { await act(async () => {}); fireEvent.click(screen.getByTestId("wf-simple-toolbar-add-step")); - const dialog = await screen.findByTestId("wf-add-step-modal"); + /* + FNXC:FloatingWindow 2026-07-30-08:50: + The add-step dialog is a FloatingWindow now (WorkflowAddStepModal.tsx:145, + `windowKey="workflow-add-step"`), so its stable testid is the window's + `floating-window-workflow-add-step`. The old `wf-add-step-modal` testid does not exist anywhere in + app source — verified by grep, not inferred — so these queries could never resolve. + */ + const dialog = await screen.findByTestId("floating-window-workflow-add-step"); fireEvent.click(within(dialog).getByTestId("wf-add-step-tpl-tpl-sec-optional-group")); - await waitFor(() => expect(screen.queryByTestId("wf-add-step-modal")).not.toBeInTheDocument()); + await waitFor(() => expect(screen.queryByTestId("floating-window-workflow-add-step")).not.toBeInTheDocument()); fireEvent.click(screen.getByText("Save").closest("button")!); await waitFor(() => expect(updateWorkflow).toHaveBeenCalledTimes(1)); @@ -4348,9 +4362,16 @@ describe("WorkflowNodeEditor simplified view modes", () => { // def() has a single edge into end, so the toolbar add targets that edge. fireEvent.click(screen.getByTestId("wf-simple-toolbar-add-step")); - const dialog = await screen.findByTestId("wf-add-step-modal"); + /* + FNXC:FloatingWindow 2026-07-30-08:50: + The add-step dialog is a FloatingWindow now (WorkflowAddStepModal.tsx:145, + `windowKey="workflow-add-step"`), so its stable testid is the window's + `floating-window-workflow-add-step`. The old `wf-add-step-modal` testid does not exist anywhere in + app source — verified by grep, not inferred — so these queries could never resolve. + */ + const dialog = await screen.findByTestId("floating-window-workflow-add-step"); fireEvent.click(within(dialog).getByTestId("wf-add-step-fragment-WF-FRAG")); - await waitFor(() => expect(screen.queryByTestId("wf-add-step-modal")).not.toBeInTheDocument()); + await waitFor(() => expect(screen.queryByTestId("floating-window-workflow-add-step")).not.toBeInTheDocument()); // Save and inspect the serialized IR: merge no longer feeds end directly; // the fragment's lint gate sits between them.