fix(tests): three more portal query-root failures (pr-tab, worktree-terminal, milestone-slice) (#2959)
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. <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## 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. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -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");
|
||||
|
||||
@@ -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();
|
||||
});
|
||||
});
|
||||
|
||||
@@ -165,7 +165,7 @@ describe("TaskDetailModal worktree terminal tab", () => {
|
||||
const { container } = renderDetail();
|
||||
|
||||
await screen.findByRole("button", { name: "Terminal" });
|
||||
const tabLabels = Array.from(container.querySelectorAll<HTMLButtonElement>(".detail-tabs .detail-tab"))
|
||||
const tabLabels = Array.from(document.querySelectorAll<HTMLButtonElement>(".detail-tabs .detail-tab"))
|
||||
.map((tab) => tab.textContent?.trim());
|
||||
|
||||
expect(tabLabels.indexOf("Comments")).toBeGreaterThanOrEqual(0);
|
||||
|
||||
@@ -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);
|
||||
|
||||
@@ -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,
|
||||
|
||||
Reference in New Issue
Block a user