From dd3a0f0189c6ff2295ad96d8f002a1b37b3de6b2 Mon Sep 17 00:00:00 2001 From: Xiangzhe Date: Tue, 25 Aug 2026 09:48:12 -0300 Subject: [PATCH] test(vitest): realign five sibling-test contracts unmasked by the green mcp shard MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit None of these are cycle regressions. The Vitest job runs test:vitest (mcp shard) then test:vitest:ui; the mcp shard was failing on a missing glm-5.3-max and aborted the job before the ui shard ever ran. Fixing that shard this cycle unmasked 34 ui failures that had been broken since 18-23 Aug — four separate PRs that moved a contract and updated their own tests but not their siblings. - ProviderCard gained useRouter() in #10448; four test files render it without mocking next/navigation and died on 'invariant expected app router to be mounted'. The sibling created alongside #10448 already had the mock — it just was not applied to the other four consumers. - SkillCoverage gained a required config category. The four fixtures in agent-skills-page still described only api/cli, so the component read config.have off undefined. Values were chosen per scenario rather than pasted: full coverage gets 2/2 so its bar stays emerald, the amber fixture gets 3/4 so it stays amber. CoverageBar renders api -> config -> cli, so the new bar lands in the MIDDLE and the cli assertions moved from index [1] to [2]; without that the cli checks would have passed while measuring the config bar. The aria test now pins all three bars. - CliAgentsPage hardcoded AGENT_IDS, which had already drifted once (6 -> 8 with omp/letta, per its own comment) and drifted again with prime-agent (#11166). It is now derived from CLI_TOOLS. This is why an agent missing from that list is not cosmetic: it never enters the status map, defaults to not_installed, and adds a phantom card to the filter and count tests. Deriving keeps the fixture in sync by construction instead of waiting for the next agent. - claudeTlsClient asserted proxyUrl was undefined inside a test literally named 'falls back to env var when per-call proxyUrl not provided' — it pinned the old behaviour where testOverride bypassed proxy resolution. #10910 moved resolution ahead of the override on purpose ('so test overrides and the real path both see it'), so the assertion now checks the fallback the test name promises. test:vitest:ui goes from 34 failures to 14. The remaining 14 sit in six files none of this commit touches (AutoComboCatalog, CoolingConnectionsPanel, ProxyRegistryManager x2, connectionsSearchFilter) plus one claudeTlsClient case that passes in isolation and only fails in the full run — i.e. cross-file pollution. They need a clean environment to judge: this devbox resolves part of its tree through a stray pnpm store and has already produced one phantom failure count this cycle. Refs #10692 --- .../__tests__/claudeTlsClient.test.ts | 15 +++++---- .../__tests__/providerCardAudioBadge.test.tsx | 1 + tests/unit/agent-skills-page.test.tsx | 30 ++++++++++++----- tests/unit/ui/CliAgentsPage.test.tsx | 32 ++++++++----------- .../ui/highlightableProviderCard.test.tsx | 1 + tests/unit/ui/providerCardHandle.test.tsx | 1 + .../ui/providerCardKimiPartnerAccent.test.tsx | 1 + 7 files changed, 48 insertions(+), 33 deletions(-) diff --git a/open-sse/services/__tests__/claudeTlsClient.test.ts b/open-sse/services/__tests__/claudeTlsClient.test.ts index 7eb2479b1a..f4bd3b5f3f 100644 --- a/open-sse/services/__tests__/claudeTlsClient.test.ts +++ b/open-sse/services/__tests__/claudeTlsClient.test.ts @@ -273,15 +273,16 @@ describe("claudeTlsClient", () => { await tlsFetchClaude("https://claude.ai/test", {}); - // The testOverride is called with the raw options object BEFORE proxy - // resolution occurs (see claudeTlsClient.ts line 258: - // `if (testOverride) return testOverride(url, options)`). - // Proxy resolution (env var → proxyUrl) only runs inside the real - // tls-client path, which is bypassed when an override is active. - // So callOptions here is exactly the {} we passed — no proxyUrl injected. + // #10910 passou a resolver proxyUrl ANTES de chamar o testOverride + // (tlsClientBase.ts: "Resolve proxyUrl early so test overrides and the real + // path both see it"), justamente para que o override enxergue o mesmo proxy + // que o caminho real usaria. A assercao anterior travava o comportamento + // antigo — override recebia o {} cru — e contradizia o proprio nome deste + // teste, que diz verificar o fallback para a env var. Agora ela confere o + // fallback de fato. expect(mockFn).toHaveBeenCalledOnce(); const callOptions = mockFn.mock.calls[0][1]; - expect(callOptions.proxyUrl).toBeUndefined(); + expect(callOptions.proxyUrl).toBe("http://env-proxy:8080"); __setTlsFetchOverrideForTesting(null); delete process.env.HTTPS_PROXY; diff --git a/src/app/(dashboard)/dashboard/providers/components/__tests__/providerCardAudioBadge.test.tsx b/src/app/(dashboard)/dashboard/providers/components/__tests__/providerCardAudioBadge.test.tsx index 71850594aa..dbaa48cea8 100644 --- a/src/app/(dashboard)/dashboard/providers/components/__tests__/providerCardAudioBadge.test.tsx +++ b/src/app/(dashboard)/dashboard/providers/components/__tests__/providerCardAudioBadge.test.tsx @@ -7,6 +7,7 @@ import ProviderCard from "../ProviderCard"; vi.mock("next-intl", () => ({ useTranslations: () => (k: string) => k })); vi.mock("@/shared/components/ProviderTestSlideOver", () => ({ default: () => null })); vi.mock("@/shared/components/ProviderIcon", () => ({ default: () => null })); +vi.mock("next/navigation", () => ({ useRouter: () => ({ push: () => {} }) })); describe("ProviderCard — #6936 audio-transcriptions provider badge", () => { let container: HTMLDivElement | null = null; diff --git a/tests/unit/agent-skills-page.test.tsx b/tests/unit/agent-skills-page.test.tsx index b315e71806..1b241c4480 100644 --- a/tests/unit/agent-skills-page.test.tsx +++ b/tests/unit/agent-skills-page.test.tsx @@ -92,14 +92,16 @@ function make42Skills(): AgentSkill[] { const FULL_COVERAGE: SkillCoverage = { api: { have: 22, total: 22 }, cli: { have: 20, total: 20 }, - totalSkills: 42, + config: { have: 2, total: 2 }, + totalSkills: 44, generatedAt: new Date().toISOString(), }; const PARTIAL_COVERAGE: SkillCoverage = { api: { have: 10, total: 22 }, cli: { have: 8, total: 20 }, - totalSkills: 18, + config: { have: 1, total: 2 }, + totalSkills: 19, generatedAt: new Date().toISOString(), }; @@ -328,14 +330,17 @@ describe("AgentSkillsPageClient", () => { expect(coverageBar).not.toBeNull(); const progressBars = container.querySelectorAll("[role='progressbar']"); - expect(progressBars.length).toBe(2); + // A ordem de render em CoverageBar e api -> config -> cli, entao o indice do CLI + // acompanha a barra de config; sem isso [1] passaria a apontar para config e a + // assercao do CLI ficaria verde medindo a barra errada. + expect(progressBars.length).toBe(3); // API bar — 22/22 = 100%, should have emerald color class const apiBar = progressBars[0] as HTMLElement; expect(apiBar.className).toContain("bg-emerald-500"); // CLI bar — 20/20 = 100%, should have emerald color class - const cliBar = progressBars[1] as HTMLElement; + const cliBar = progressBars[2] as HTMLElement; expect(cliBar.className).toContain("bg-emerald-500"); }); @@ -425,7 +430,7 @@ describe("AgentSkillsPageClient", () => { // ── CoverageBar isolated tests ─────────────────────────────────────────────── describe("CoverageBar", () => { - it("renders two progressbars with correct aria attributes", async () => { + it("renders three progressbars with correct aria attributes", async () => { const { CoverageBar } = await import("../../src/app/(dashboard)/dashboard/agent-skills/components/CoverageBar"); const container = makeContainer(); @@ -435,13 +440,20 @@ describe("CoverageBar", () => { }); const bars = container.querySelectorAll("[role='progressbar']"); - expect(bars.length).toBe(2); + // api -> config -> cli. A barra de config entrou no MEIO, entao o indice do CLI + // desloca junto; conferir as tres aqui e o que impede um indice errado de passar + // despercebido medindo a barra vizinha. + expect(bars.length).toBe(3); const apiBar = bars[0] as HTMLElement; expect(apiBar.getAttribute("aria-valuenow")).toBe("22"); expect(apiBar.getAttribute("aria-valuemax")).toBe("22"); - const cliBar = bars[1] as HTMLElement; + const configBar = bars[1] as HTMLElement; + expect(configBar.getAttribute("aria-valuenow")).toBe("2"); + expect(configBar.getAttribute("aria-valuemax")).toBe("2"); + + const cliBar = bars[2] as HTMLElement; expect(cliBar.getAttribute("aria-valuenow")).toBe("20"); expect(cliBar.getAttribute("aria-valuemax")).toBe("20"); @@ -454,6 +466,7 @@ describe("CoverageBar", () => { const lowCoverage: SkillCoverage = { api: { have: 5, total: 22 }, cli: { have: 0, total: 20 }, + config: { have: 0, total: 2 }, totalSkills: 5, generatedAt: new Date().toISOString(), }; @@ -477,7 +490,8 @@ describe("CoverageBar", () => { const partialCoverage: SkillCoverage = { api: { have: 18, total: 22 }, // ~81.8% = amber cli: { have: 15, total: 20 }, // 75% = amber - totalSkills: 33, + config: { have: 3, total: 4 }, // 75% = amber + totalSkills: 36, generatedAt: new Date().toISOString(), }; diff --git a/tests/unit/ui/CliAgentsPage.test.tsx b/tests/unit/ui/CliAgentsPage.test.tsx index 257f5e4149..fd1cb1a6d6 100644 --- a/tests/unit/ui/CliAgentsPage.test.tsx +++ b/tests/unit/ui/CliAgentsPage.test.tsx @@ -4,6 +4,7 @@ import { act } from "react"; import { createRoot } from "react-dom/client"; import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; import type { ToolBatchStatusMap } from "@/shared/types/cliBatchStatus"; +import { CLI_TOOLS } from "@/shared/constants/cliTools"; // ── Mocks (declared before any imports that depend on them) ─────────────────── @@ -48,9 +49,8 @@ vi.mock("@/app/(dashboard)/dashboard/cli-code/components/CliStatusBadge", () => // ── Static imports after mocks ──────────────────────────────────────────────── -const { default: CliAgentsPageClient } = await import( - "@/app/(dashboard)/dashboard/cli-agents/CliAgentsPageClient" -); +const { default: CliAgentsPageClient } = + await import("@/app/(dashboard)/dashboard/cli-agents/CliAgentsPageClient"); // ── Fixtures ────────────────────────────────────────────────────────────────── @@ -59,16 +59,14 @@ const { default: CliAgentsPageClient } = await import( * src/shared/constants/cliTools.ts). "omp" and "letta" were added to the * catalog after plan-14 shipped, bringing the count from 6 to 8. */ -const AGENT_IDS = [ - "openclaw", - "hermes-agent", - "goose", - "interpreter", - "omp", - "letta", - "warp", - "agent-deck", -] as const; +// Derivado do catalogo, NAO escrito a mao. A lista hardcoded anterior ja tinha +// derivado duas vezes (6 -> 8 com omp/letta, depois 8 -> 9 com prime-agent), e a +// consequencia nao e obvia: um agente ausente daqui nao entra no mapa de status, +// cai no default not_installed e contamina os testes de filtro/contagem com um +// card extra. Derivar mantem o fixture em sincronia com a fonte por construcao. +const AGENT_IDS = Object.values(CLI_TOOLS) + .filter((tool) => tool.category === "agent") + .map((tool) => tool.id); function makeBatchStatusMap(overrides: Partial = {}): ToolBatchStatusMap { const base: ToolBatchStatusMap = {}; @@ -150,9 +148,9 @@ describe("CliAgentsPageClient", () => { expect(container.textContent).toContain("pageTitle"); }, 15000); - it("2. renders exactly 8 agent tool cards", async () => { + it("2. renders exactly one card per agent in the catalog", async () => { const container = await renderPage(); - expect(countAgentCards(container)).toBe(8); + expect(countAgentCards(container)).toBe(AGENT_IDS.length); }, 15000); it("3. search filter — 'hermes' shows 1 card (hermes-agent)", async () => { @@ -175,9 +173,7 @@ describe("CliAgentsPageClient", () => { const visibleCards = countAgentCards(container); expect(visibleCards).toBe(1); - const remainingHrefs = Array.from( - container.querySelectorAll("a[href]") - ) + const remainingHrefs = Array.from(container.querySelectorAll("a[href]")) .filter((a) => a.getAttribute("href")?.startsWith("/dashboard/cli-agents/")) .map((a) => a.getAttribute("href") ?? ""); diff --git a/tests/unit/ui/highlightableProviderCard.test.tsx b/tests/unit/ui/highlightableProviderCard.test.tsx index cefafdce64..22abb02782 100644 --- a/tests/unit/ui/highlightableProviderCard.test.tsx +++ b/tests/unit/ui/highlightableProviderCard.test.tsx @@ -18,6 +18,7 @@ import HighlightableProviderCard from "@/app/(dashboard)/dashboard/providers/com vi.mock("next-intl", () => ({ useTranslations: () => (k: string) => k })); vi.mock("@/shared/components/ProviderTestSlideOver", () => ({ default: () => null })); vi.mock("@/shared/components/ProviderIcon", () => ({ default: () => null })); +vi.mock("next/navigation", () => ({ useRouter: () => ({ push: () => {} }) })); // Deterministic anchor so the click path does not depend on next/link's router. vi.mock("next/link", () => ({ __esModule: true, diff --git a/tests/unit/ui/providerCardHandle.test.tsx b/tests/unit/ui/providerCardHandle.test.tsx index ce8f4b7fd7..bd4860b13e 100644 --- a/tests/unit/ui/providerCardHandle.test.tsx +++ b/tests/unit/ui/providerCardHandle.test.tsx @@ -16,6 +16,7 @@ import ProviderCard, { vi.mock("next-intl", () => ({ useTranslations: () => (k: string) => k })); vi.mock("@/shared/components/ProviderTestSlideOver", () => ({ default: () => null })); vi.mock("@/shared/components/ProviderIcon", () => ({ default: () => null })); +vi.mock("next/navigation", () => ({ useRouter: () => ({ push: () => {} }) })); // jsdom does not implement scrollIntoView or animate if (typeof Element.prototype.scrollIntoView === "undefined") { diff --git a/tests/unit/ui/providerCardKimiPartnerAccent.test.tsx b/tests/unit/ui/providerCardKimiPartnerAccent.test.tsx index 07aceb699a..85fa7e7b30 100644 --- a/tests/unit/ui/providerCardKimiPartnerAccent.test.tsx +++ b/tests/unit/ui/providerCardKimiPartnerAccent.test.tsx @@ -26,6 +26,7 @@ import ProviderCard from "@/app/(dashboard)/dashboard/providers/components/Provi vi.mock("next-intl", () => ({ useTranslations: () => (k: string) => k })); vi.mock("@/shared/components/ProviderTestSlideOver", () => ({ default: () => null })); vi.mock("@/shared/components/ProviderIcon", () => ({ default: () => null })); +vi.mock("next/navigation", () => ({ useRouter: () => ({ push: () => {} }) })); describe("ProviderCard — Kimi (Moonshot AI) founding-friend accent", () => { let container: HTMLDivElement | null = null;