test(dashboard): four more causes in components-b (14 → 0) — incl. a parity guard jsdom 27 made vacuous (#2743)
## 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. <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## 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. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
This commit is contained in:
@@ -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
|
||||
|
||||
@@ -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();
|
||||
}
|
||||
|
||||
@@ -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<HTMLButtonElement>(".detail-tabs .detail-tab")[0];
|
||||
const chatTab = document.querySelectorAll<HTMLButtonElement>(".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();
|
||||
});
|
||||
|
||||
@@ -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.
|
||||
|
||||
Reference in New Issue
Block a user