From b0d6d6965e7b25edcabc1252f4ca8bd1a27ea3e4 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 11 Oct 2026 23:39:16 +0000 Subject: [PATCH 1/4] test: unit coverage for managed secrets and console shell (spec 013 US4) secrets_managed_test.go covers upsertLabelledSecret and deleteManagedSecret on a fake clientset: create, merge-patch of a labelled Secret, refusal of an unlabelled one, label-gated delete, and propagation of Create/Get/Patch/Delete errors. ConsoleShell.test.tsx covers every status label, send/trim/empty input, history recall with draft restore, the 100-entry cap and the toolbar buttons. T051-T052. Co-Authored-By: Claude Opus 5.5 (1M context) Signed-off-by: Claude --- api/internal/handlers/secrets_managed_test.go | 251 ++++++++++++++++++ specs/013-expand-test-coverage/tasks.md | 4 +- web/src/routes/tabs/ConsoleShell.test.tsx | 210 +++++++++++++++ 3 files changed, 463 insertions(+), 2 deletions(-) create mode 100644 api/internal/handlers/secrets_managed_test.go create mode 100644 web/src/routes/tabs/ConsoleShell.test.tsx diff --git a/api/internal/handlers/secrets_managed_test.go b/api/internal/handlers/secrets_managed_test.go new file mode 100644 index 000000000..6ea930914 --- /dev/null +++ b/api/internal/handlers/secrets_managed_test.go @@ -0,0 +1,251 @@ +package handlers + +import ( + "errors" + "reflect" + "testing" + + corev1 "k8s.io/api/core/v1" + apierrors "k8s.io/apimachinery/pkg/api/errors" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" + kubefake "k8s.io/client-go/kubernetes/fake" + k8stesting "k8s.io/client-go/testing" + + "github.com/GameplanePanel/gameplane/api/internal/kube" +) + +const ( + smtFeatureLabel = "gameplane.local/test-feature" + smtNamespace = "gp-system" +) + +// smtClient builds a kube.Client backed by a fake clientset seeded with objs. +// Returning the concrete *kubefake.Clientset lets callers prepend reactors. +func smtClient(objs ...runtime.Object) (*kubefake.Clientset, *kube.Client) { + cs := kubefake.NewClientset(objs...) + return cs, &kube.Client{Typed: cs} +} + +func smtSecret(name string, labels, data map[string]string) *corev1.Secret { + return &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{Name: name, Namespace: smtNamespace, Labels: labels}, + Type: corev1.SecretTypeOpaque, + StringData: data, + } +} + +func smtFailVerb(cs *kubefake.Clientset, verb string, injected error) { + cs.PrependReactor(verb, "secrets", func(k8stesting.Action) (bool, runtime.Object, error) { + return true, nil, injected + }) +} + +func TestUpsertLabelledSecret_Create(t *testing.T) { + t.Parallel() + + _, k := smtClient() + input := map[string]string{"a": "1", "b": "2"} + if err := upsertLabelledSecret(t.Context(), k, smtNamespace, "creds", smtFeatureLabel, input); err != nil { + t.Fatalf("upsert: %v", err) + } + got, err := k.Typed.CoreV1().Secrets(smtNamespace).Get(t.Context(), "creds", metav1.GetOptions{}) + if err != nil { + t.Fatalf("get: %v", err) + } + if got.Labels[smtFeatureLabel] != "true" || got.Labels[ManagedByLabel] != managedByValue { + t.Fatalf("labels = %v", got.Labels) + } + if got.Type != corev1.SecretTypeOpaque { + t.Fatalf("type = %q", got.Type) + } + if !reflect.DeepEqual(got.StringData, input) { + t.Fatalf("stringData = %v, want %v", got.StringData, input) + } +} + +func TestUpsertLabelledSecret_UpdatesLabelledSecret(t *testing.T) { + t.Parallel() + + existing := smtSecret("creds", + map[string]string{smtFeatureLabel: "true"}, + map[string]string{"a": "old", "b": "keep"}) + _, k := smtClient(existing) + + if err := upsertLabelledSecret(t.Context(), k, smtNamespace, "creds", smtFeatureLabel, map[string]string{"a": "new"}); err != nil { + t.Fatalf("upsert: %v", err) + } + got, err := k.Typed.CoreV1().Secrets(smtNamespace).Get(t.Context(), "creds", metav1.GetOptions{}) + if err != nil { + t.Fatalf("get: %v", err) + } + if got.Labels[ManagedByLabel] != managedByValue || got.Labels[smtFeatureLabel] != "true" { + t.Fatalf("labels = %v", got.Labels) + } + if got.StringData["a"] != "new" { + t.Fatalf("a = %q, want new", got.StringData["a"]) + } + if got.StringData["b"] != "keep" { + t.Fatalf("b = %q, want keep (merge patch must not drop it)", got.StringData["b"]) + } +} + +func TestUpsertLabelledSecret_RefusesUnlabelledSecret(t *testing.T) { + t.Parallel() + + existing := smtSecret("creds", + map[string]string{"someone": "else"}, + map[string]string{"a": "orig"}) + _, k := smtClient(existing) + before, err := k.Typed.CoreV1().Secrets(smtNamespace).Get(t.Context(), "creds", metav1.GetOptions{}) + if err != nil { + t.Fatalf("get before: %v", err) + } + + err = upsertLabelledSecret(t.Context(), k, smtNamespace, "creds", smtFeatureLabel, map[string]string{"a": "new"}) + if !errors.Is(err, errNotManagedSecret) { + t.Fatalf("err = %v, want errNotManagedSecret", err) + } + after, err := k.Typed.CoreV1().Secrets(smtNamespace).Get(t.Context(), "creds", metav1.GetOptions{}) + if err != nil { + t.Fatalf("get after: %v", err) + } + if !reflect.DeepEqual(before.Labels, after.Labels) { + t.Fatalf("labels changed: before=%v after=%v", before.Labels, after.Labels) + } + if !reflect.DeepEqual(before.StringData, after.StringData) { + t.Fatalf("data changed: before=%v after=%v", before.StringData, after.StringData) + } +} + +func TestUpsertLabelledSecret_ErrorPropagation(t *testing.T) { + t.Parallel() + + injected := errors.New("injected") + + tests := []struct { + name string + objs []runtime.Object + verb string + injected error + }{ + { + name: "create error other than AlreadyExists is returned", + objs: nil, + verb: "create", + injected: injected, + }, + { + name: "get error after AlreadyExists is returned", + objs: []runtime.Object{smtSecret("creds", map[string]string{smtFeatureLabel: "true"}, nil)}, + verb: "get", + injected: injected, + }, + { + name: "patch error is returned", + objs: []runtime.Object{smtSecret("creds", map[string]string{smtFeatureLabel: "true"}, nil)}, + verb: "patch", + injected: injected, + }, + } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + + cs, k := smtClient(tc.objs...) + smtFailVerb(cs, tc.verb, tc.injected) + + err := upsertLabelledSecret(t.Context(), k, smtNamespace, "creds", smtFeatureLabel, map[string]string{"a": "new"}) + if !errors.Is(err, tc.injected) { + t.Fatalf("err = %v, want injected", err) + } + }) + } +} + +func TestDeleteManagedSecret(t *testing.T) { + t.Parallel() + + managedLabels := func() map[string]string { + return map[string]string{smtFeatureLabel: "true", ManagedByLabel: managedByValue} + } + + t.Run("both labels deletes the secret", func(t *testing.T) { + t.Parallel() + + _, k := smtClient(smtSecret("creds", managedLabels(), nil)) + if err := deleteManagedSecret(t.Context(), k, smtNamespace, "creds", smtFeatureLabel); err != nil { + t.Fatalf("delete: %v", err) + } + _, err := k.Typed.CoreV1().Secrets(smtNamespace).Get(t.Context(), "creds", metav1.GetOptions{}) + if !apierrors.IsNotFound(err) { + t.Fatalf("after delete, get err = %v, want NotFound", err) + } + }) + + refusedCases := []struct { + name string + labels map[string]string + }{ + { + name: "feature label missing", + labels: map[string]string{ManagedByLabel: managedByValue}, + }, + { + name: "managed-by label missing", + labels: map[string]string{smtFeatureLabel: "true"}, + }, + { + name: "managed-by has another value", + labels: map[string]string{smtFeatureLabel: "true", ManagedByLabel: "kubectl"}, + }, + } + for _, tc := range refusedCases { + t.Run(tc.name+" reads as not found and keeps the secret", func(t *testing.T) { + t.Parallel() + + _, k := smtClient(smtSecret("creds", tc.labels, nil)) + err := deleteManagedSecret(t.Context(), k, smtNamespace, "creds", smtFeatureLabel) + if !apierrors.IsNotFound(err) { + t.Fatalf("err = %v, want NotFound", err) + } + if _, err := k.Typed.CoreV1().Secrets(smtNamespace).Get(t.Context(), "creds", metav1.GetOptions{}); err != nil { + t.Fatalf("secret should still exist: %v", err) + } + }) + } + + t.Run("get error is returned", func(t *testing.T) { + t.Parallel() + + injected := errors.New("injected") + cs, k := smtClient(smtSecret("creds", managedLabels(), nil)) + smtFailVerb(cs, "get", injected) + err := deleteManagedSecret(t.Context(), k, smtNamespace, "creds", smtFeatureLabel) + if !errors.Is(err, injected) { + t.Fatalf("err = %v, want injected", err) + } + }) + + t.Run("secret not found at all", func(t *testing.T) { + t.Parallel() + + _, k := smtClient() + err := deleteManagedSecret(t.Context(), k, smtNamespace, "creds", smtFeatureLabel) + if !apierrors.IsNotFound(err) { + t.Fatalf("err = %v, want NotFound", err) + } + }) + + t.Run("delete error is returned", func(t *testing.T) { + t.Parallel() + + injected := errors.New("injected") + cs, k := smtClient(smtSecret("creds", managedLabels(), nil)) + smtFailVerb(cs, "delete", injected) + err := deleteManagedSecret(t.Context(), k, smtNamespace, "creds", smtFeatureLabel) + if !errors.Is(err, injected) { + t.Fatalf("err = %v, want injected", err) + } + }) +} diff --git a/specs/013-expand-test-coverage/tasks.md b/specs/013-expand-test-coverage/tasks.md index 4b8252f39..3a10ec95c 100644 --- a/specs/013-expand-test-coverage/tasks.md +++ b/specs/013-expand-test-coverage/tasks.md @@ -309,7 +309,7 @@ description: "Task list for 013-expand-test-coverage" **PR**: `test: unit coverage for managed secrets and console shell`. Independent of every other phase. -- [ ] T051 [P] [US4] Write `api/internal/handlers/secrets_managed_test.go`. Build a `kube.Client` on `k8s.io/client-go/kubernetes/fake`, following `registry_secret_test.go`. Inject errors with `PrependReactor`. Cover every case in research R8: +- [X] T051 [P] [US4] Write `api/internal/handlers/secrets_managed_test.go`. Build a `kube.Client` on `k8s.io/client-go/kubernetes/fake`, following `registry_secret_test.go`. Inject errors with `PrependReactor`. Cover every case in research R8: - create carries both labels - `AlreadyExists` with the feature label → merge patch and `managed-by` re-asserted - `AlreadyExists` without the label → `errNotManagedSecret`, Secret untouched @@ -319,7 +319,7 @@ description: "Task list for 013-expand-test-coverage" - Get and Delete errors propagate Use table-driven tests with `t.Parallel()`. -- [ ] T052 [P] [US4] Write `web/src/routes/tabs/ConsoleShell.test.tsx`. Render `ConsoleShell` with a stub `ConsoleHandle` (`vi.fn()` for `clear`, `download`, `toggleFullscreen` and `sendCommand`, and a `hostRef`). Cover: +- [X] T052 [P] [US4] Write `web/src/routes/tabs/ConsoleShell.test.tsx`. Render `ConsoleShell` with a stub `ConsoleHandle` (`vi.fn()` for `clear`, `download`, `toggleFullscreen` and `sendCommand`, and a `hostRef`). Cover: - each `WSStatus` label: LIVE, connecting…, reconnecting…, offline - Enter sends the trimmed command and clears the input; an empty input sends nothing - ArrowUp/ArrowDown recall, including restoring the unsent draft diff --git a/web/src/routes/tabs/ConsoleShell.test.tsx b/web/src/routes/tabs/ConsoleShell.test.tsx new file mode 100644 index 000000000..d5e3d8a58 --- /dev/null +++ b/web/src/routes/tabs/ConsoleShell.test.tsx @@ -0,0 +1,210 @@ +import { createRef } from "react"; +import { render, screen, fireEvent } from "@testing-library/react"; +import userEvent from "@testing-library/user-event"; +import { describe, it, expect, vi } from "vitest"; + +import type { WSStatus } from "@/lib/ws"; +import type { ConsoleHandle } from "./useConsoleTerminal"; +import { ConsoleShell } from "./ConsoleShell"; + +function makeHandle(status: WSStatus = "open"): ConsoleHandle { + return { + hostRef: createRef(), + status, + clear: vi.fn(), + download: vi.fn(), + toggleFullscreen: vi.fn(), + sendCommand: vi.fn(), + }; +} + +function getInput(): HTMLInputElement { + return screen.getByPlaceholderText(/Type a command/) as HTMLInputElement; +} + +function getForm(input: HTMLInputElement): HTMLFormElement { + return input.closest("form") as HTMLFormElement; +} + +describe("ConsoleShell", () => { + it.each([ + ["open", "LIVE"], + ["reconnecting", "reconnecting…"], + ["connecting", "connecting…"], + ["closed", "offline"], + ] as const)("renders the %s status as %s", (status, label) => { + render(); + expect(screen.getByText(label)).toBeInTheDocument(); + }); + + it("submits the trimmed command on Enter and clears the input", async () => { + const user = userEvent.setup(); + const handle = makeHandle(); + render(); + const input = getInput(); + + await user.type(input, " say hi {Enter}"); + + expect(handle.sendCommand).toHaveBeenCalledTimes(1); + expect(handle.sendCommand).toHaveBeenCalledWith("say hi"); + expect(input.value).toBe(""); + }); + + it("does not send an empty or whitespace-only command and disables Send", async () => { + const user = userEvent.setup(); + const handle = makeHandle(); + render(); + const input = getInput(); + const sendButton = screen.getByRole("button", { name: "Send" }); + + expect(sendButton).toBeDisabled(); + + await user.type(input, " {Enter}"); + + expect(handle.sendCommand).not.toHaveBeenCalled(); + expect(sendButton).toBeDisabled(); + }); + + it("submits the same command when the Send button is clicked", async () => { + const user = userEvent.setup(); + const handle = makeHandle(); + render(); + const input = getInput(); + const sendButton = screen.getByRole("button", { name: "Send" }); + + await user.type(input, "ping"); + expect(sendButton).toBeEnabled(); + await user.click(sendButton); + + expect(handle.sendCommand).toHaveBeenCalledTimes(1); + expect(handle.sendCommand).toHaveBeenCalledWith("ping"); + expect(input.value).toBe(""); + }); + + it("recalls history with ArrowUp/ArrowDown and restores the draft past the newest entry", async () => { + const user = userEvent.setup(); + const handle = makeHandle(); + render(); + const input = getInput(); + + await user.type(input, "a{Enter}"); + await user.type(input, "b{Enter}"); + await user.type(input, "c{Enter}"); + expect(handle.sendCommand).toHaveBeenCalledTimes(3); + + await user.type(input, "dr"); + expect(input.value).toBe("dr"); + + await user.keyboard("{ArrowUp}"); + expect(input.value).toBe("c"); + await user.keyboard("{ArrowUp}"); + expect(input.value).toBe("b"); + await user.keyboard("{ArrowUp}"); + expect(input.value).toBe("a"); + await user.keyboard("{ArrowUp}"); + expect(input.value).toBe("a"); + + await user.keyboard("{ArrowDown}"); + expect(input.value).toBe("b"); + await user.keyboard("{ArrowDown}"); + expect(input.value).toBe("c"); + await user.keyboard("{ArrowDown}"); + expect(input.value).toBe("dr"); + await user.keyboard("{ArrowDown}"); + expect(input.value).toBe("dr"); + }); + + it("leaves the input unchanged for ArrowUp with empty history and ArrowDown when not browsing", async () => { + const user = userEvent.setup(); + const handle = makeHandle(); + render(); + const input = getInput(); + + await user.type(input, "keep"); + await user.keyboard("{ArrowUp}"); + expect(input.value).toBe("keep"); + await user.keyboard("{ArrowDown}"); + expect(input.value).toBe("keep"); + expect(handle.sendCommand).not.toHaveBeenCalled(); + }); + + it("stores a duplicate consecutive command only once", async () => { + const user = userEvent.setup(); + const handle = makeHandle(); + render(); + const input = getInput(); + + await user.type(input, "x{Enter}"); + await user.type(input, "x{Enter}"); + expect(handle.sendCommand).toHaveBeenCalledTimes(2); + + await user.keyboard("{ArrowUp}"); + expect(input.value).toBe("x"); + await user.keyboard("{ArrowUp}"); + expect(input.value).toBe("x"); + // With a single stored entry, ArrowDown from the oldest entry restores + // the (empty) draft; a duplicate entry would show "x" here instead. + await user.keyboard("{ArrowDown}"); + expect(input.value).toBe(""); + }); + + it("caps history at 100 entries, dropping the oldest", () => { + const handle = makeHandle(); + render(); + const input = getInput(); + const form = getForm(input); + expect(form).not.toBeNull(); + if (!form) return; + + for (let i = 0; i <= 100; i++) { + fireEvent.change(input, { target: { value: `c${i}` } }); + fireEvent.submit(form); + } + expect(handle.sendCommand).toHaveBeenCalledTimes(101); + + for (let i = 0; i < 101; i++) { + fireEvent.keyDown(input, { key: "ArrowUp" }); + } + // 101 commands were sent but only the newest 100 are kept, so the + // oldest ("c0") was dropped and the recall floor is "c1". + expect(input.value).toBe("c1"); + }); + + it("calls each toolbar handle exactly once and nothing else", async () => { + const user = userEvent.setup(); + const handle = makeHandle(); + render(); + + await user.click(screen.getByRole("button", { name: "Clear terminal" })); + expect(handle.clear).toHaveBeenCalledTimes(1); + expect(handle.download).not.toHaveBeenCalled(); + expect(handle.toggleFullscreen).not.toHaveBeenCalled(); + expect(handle.sendCommand).not.toHaveBeenCalled(); + + await user.click(screen.getByRole("button", { name: "Download terminal buffer" })); + expect(handle.download).toHaveBeenCalledTimes(1); + expect(handle.clear).toHaveBeenCalledTimes(1); + expect(handle.toggleFullscreen).not.toHaveBeenCalled(); + expect(handle.sendCommand).not.toHaveBeenCalled(); + + await user.click(screen.getByRole("button", { name: "Toggle fullscreen" })); + expect(handle.toggleFullscreen).toHaveBeenCalledTimes(1); + expect(handle.clear).toHaveBeenCalledTimes(1); + expect(handle.download).toHaveBeenCalledTimes(1); + expect(handle.sendCommand).not.toHaveBeenCalled(); + }); + + it("stays usable and still sends commands when the status is closed", async () => { + const user = userEvent.setup(); + const handle = makeHandle("closed"); + render(); + + expect(screen.getByText("offline")).toBeInTheDocument(); + const input = getInput(); + expect(input).toBeEnabled(); + + await user.type(input, "status{Enter}"); + expect(handle.sendCommand).toHaveBeenCalledTimes(1); + expect(handle.sendCommand).toHaveBeenCalledWith("status"); + }); +}); From 78fd49dd3c110718c886847a6966705d3e4d2c57 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 12 Oct 2026 03:32:35 +0000 Subject: [PATCH 2/4] test(web): cover CloneServerDialog API errors and cancel reset (T053) lcov from the first green run showed CloneServerDialog at 89 % lines but 69 % branches, with no API-error case. Add cases for 409, 403, other status (empty and non-empty body), non-API errors, and a cancel that discards an edited name on reopen. Flip G-20 (ConsoleShell 100 % lines), G-21 and G-22 (ServerActionsMenu 100 %; it makes no API calls, so cancel/error live in its dialogs). G-19 stays in progress: CI publishes only Go package totals. Co-Authored-By: Claude Opus 5.5 (1M context) Signed-off-by: Claude --- .../013-expand-test-coverage/coverage-gaps.md | 12 +-- specs/013-expand-test-coverage/tasks.md | 4 +- .../server/CloneServerDialog.test.tsx | 98 +++++++++++++++++++ 3 files changed, 107 insertions(+), 7 deletions(-) diff --git a/specs/013-expand-test-coverage/coverage-gaps.md b/specs/013-expand-test-coverage/coverage-gaps.md index fbf685c0d..28f440acc 100644 --- a/specs/013-expand-test-coverage/coverage-gaps.md +++ b/specs/013-expand-test-coverage/coverage-gaps.md @@ -1,7 +1,7 @@ # Coverage Gap Record -**Summary**: 2/24 closed · 0 closed unproven · 2 in progress · 19 open · 1 unit-only by design -**Last updated**: 2026-10-11 +**Summary**: 5/24 closed · 0 closed unproven · 1 in progress · 17 open · 1 unit-only by design +**Last updated**: 2026-10-12 | ID | Category | Gap | Story | FR | Priority | Status | Evidence | Notes | |---|---|---|---|---|---|---|---|---| @@ -23,9 +23,9 @@ | G-16 | e2e-optional | tunnel (tailscale, playit) | US3 | FR-003 | P2 | unit-only-by-design | | Tailscale and playit need external accounts and network reachability that CI cannot provide; covered by unit tests only (research R7). | | G-17 | e2e-optional | telemetry receiver | US3 | FR-003 | P2 | closed | `TestTelemetryLifecycle` in bucket `telemetry` (job `e2e telemetry`), spec 022: https://github.com/GameplanePanel/Gameplane/blob/master/test/e2e/telemetry_e2e_test.go | | | G-18 | e2e-optional | disabled state + no interference | US3 | FR-003 | P2 | open | | | -| G-19 | unit | `secrets_managed.go` 100 % | US4 | FR-004 | P2 | open | | | -| G-20 | unit | `ConsoleShell.test.tsx` ≥ 80 % lines | US4 | FR-004 | P2 | open | | | -| G-21 | unit | `CloneServerDialog` ≥ 80 % lines + cancel/error | US4 | FR-004 | P2 | in-progress | | test exists (`CloneServerDialog.test.tsx`); coverage to verify in T053. | -| G-22 | unit | `ServerActionsMenu` ≥ 80 % lines + cancel/error | US4 | FR-004 | P2 | in-progress | | test exists (`ServerActionsMenu.test.tsx`); coverage to verify in T053. | +| G-19 | unit | `secrets_managed.go` 100 % | US4 | FR-004 | P2 | in-progress | | `secrets_managed_test.go` added in https://github.com/GameplanePanel/Gameplane/pull/677; CI publishes only package totals, so the per-file figure is not yet measured. The `json.Marshal` error branch in `upsertLabelledSecret` is unreachable. | +| G-20 | unit | `ConsoleShell.test.tsx` ≥ 80 % lines | US4 | FR-004 | P2 | closed | `web` https://github.com/GameplanePanel/Gameplane/actions/runs/38189418314 | `ConsoleShell.tsx` 100 % lines, 95 % branches (lcov from https://github.com/GameplanePanel/Gameplane/pull/677). | +| G-21 | unit | `CloneServerDialog` ≥ 80 % lines + cancel/error | US4 | FR-004 | P2 | closed | `web` https://github.com/GameplanePanel/Gameplane/actions/runs/38189418314 | 89.29 % lines before T053; T053 adds API-error (409, 403, other status, non-API) and cancel-discards-changes cases in https://github.com/GameplanePanel/Gameplane/pull/677. | +| G-22 | unit | `ServerActionsMenu` ≥ 80 % lines + cancel/error | US4 | FR-004 | P2 | closed | `web` https://github.com/GameplanePanel/Gameplane/actions/runs/38189418314 | 100 % lines, branches and functions. The menu makes no API calls; cancel and API errors live in its dialogs, covered by their own tests (G-21 for clone). | | G-23 | e2e-postgres | kind E2E on Postgres-backed API | US5 | FR-005 | P3 | open | | | | G-24 | e2e-postgres | upgrade from frozen 001–012 baseline | US5 | FR-005 | P3 | open | | | diff --git a/specs/013-expand-test-coverage/tasks.md b/specs/013-expand-test-coverage/tasks.md index 3a10ec95c..ed782a7f8 100644 --- a/specs/013-expand-test-coverage/tasks.md +++ b/specs/013-expand-test-coverage/tasks.md @@ -328,12 +328,14 @@ description: "Task list for 013-expand-test-coverage" - the `closed` status state Use `@testing-library/user-event` the same way `Console.test.tsx` does. -- [ ] T053 [US4] After T051 and T052 are green in CI, read the `web` job's lcov for `src/components/server/CloneServerDialog.tsx` and `src/components/server/ServerActionsMenu.tsx`. +- [X] T053 [US4] After T051 and T052 are green in CI, read the `web` job's lcov for `src/components/server/CloneServerDialog.tsx` and `src/components/server/ServerActionsMenu.tsx`. - Any file under 80 % lines, or missing a "cancel discards changes" case or an API-error case: add only the missing cases to `CloneServerDialog.test.tsx` or `ServerActionsMenu.test.tsx`. - Otherwise: record the measured percentages as evidence. Flip G-19 to G-22. + *Implementation note (2026-10-12)*: lcov from run 38189418314: `CloneServerDialog.tsx` 89.29 % lines / 69.23 % branches, with a Cancel case but no API-error case, so T053 adds the API-error and cancel-discards-changes cases. `ServerActionsMenu.tsx` 100 % lines / branches / functions; it makes no API calls, so cancel and API errors are dialog-level (G-22 note). `ConsoleShell.tsx` 100 % lines. G-19 stays in progress: CI reports only package coverage for Go. + **Checkpoint**: SC-004 is met. Module thresholds stay unchanged. --- diff --git a/web/src/components/server/CloneServerDialog.test.tsx b/web/src/components/server/CloneServerDialog.test.tsx index 092eb3622..aec9658c3 100644 --- a/web/src/components/server/CloneServerDialog.test.tsx +++ b/web/src/components/server/CloneServerDialog.test.tsx @@ -4,6 +4,7 @@ import { render, screen } from "@testing-library/react"; import userEvent from "@testing-library/user-event"; import { QueryClient, QueryClientProvider } from "@tanstack/react-query"; import { CloneServerDialog } from "./CloneServerDialog"; +import { APIError } from "@/lib/api"; // Mock the endpoints vi.mock("@/lib/endpoints", async (importOriginal) => { @@ -231,4 +232,101 @@ describe("CloneServerDialog", () => { search: { cluster: "local", ns: "team-a" }, }); }); + + it("keeps the dialog open and shows the conflict message when the name already exists", async () => { + const { Servers } = await import("@/lib/endpoints"); + vi.mocked(Servers.clone).mockRejectedValue(new APIError(409, "conflict")); + const onOpenChange = vi.fn(); + const user = userEvent.setup(); + render( + , + { wrapper: Wrapper }, + ); + + await user.click(screen.getByRole("button", { name: "Clone server" })); + + expect(await screen.findByText("A server named source-copy already exists.")).toBeInTheDocument(); + expect(onOpenChange).not.toHaveBeenCalled(); + expect(mockNavigate).not.toHaveBeenCalled(); + }); + + it.each([ + { status: 403, body: "", message: "Your role does not allow cloning servers." }, + { status: 500, body: "", message: "Clone failed (500)." }, + { status: 500, body: "disk full", message: "disk full" }, + ])("shows '$message' when the API answers $status", async ({ status, body, message }) => { + const { Servers } = await import("@/lib/endpoints"); + vi.mocked(Servers.clone).mockRejectedValue(new APIError(status, body)); + const user = userEvent.setup(); + render( + {}} + sourceName="source" + />, + { wrapper: Wrapper }, + ); + + await user.click(screen.getByRole("button", { name: "Clone server" })); + + expect(await screen.findByText(message)).toBeInTheDocument(); + expect(mockNavigate).not.toHaveBeenCalled(); + }); + + it("shows the error text when the request fails without an API response", async () => { + const { Servers } = await import("@/lib/endpoints"); + vi.mocked(Servers.clone).mockRejectedValue(new Error("network down")); + const user = userEvent.setup(); + render( + {}} + sourceName="source" + />, + { wrapper: Wrapper }, + ); + + await user.click(screen.getByRole("button", { name: "Clone server" })); + + expect(await screen.findByText("network down")).toBeInTheDocument(); + expect(mockNavigate).not.toHaveBeenCalled(); + }); + + it("discards an edited name when the dialog is cancelled and reopened", async () => { + const user = userEvent.setup(); + const { rerender } = render( + {}} + sourceName="source" + />, + { wrapper: Wrapper }, + ); + const input = screen.getByDisplayValue("source-copy") as HTMLInputElement; + await user.clear(input); + await user.type(input, "edited-name"); + expect(input.value).toBe("edited-name"); + + rerender( + {}} + sourceName="source" + />, + ); + rerender( + {}} + sourceName="source" + />, + ); + + expect(await screen.findByDisplayValue("source-copy")).toBeInTheDocument(); + expect(screen.queryByDisplayValue("edited-name")).not.toBeInTheDocument(); + }); }); From a8ffe535601a58aecf32357ef18a179f69c53cea Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 12 Oct 2026 20:55:46 +0000 Subject: [PATCH 3/4] test(web): make Share polling and CaptureWidget download tests deterministic Two T184 Share polling tests mixed shouldAdvanceTime fake timers with a synchronous advanceTimersByTime and waitFor, so they timed out on busy runners (#677, #678). They now use pure fake timers advanced with advanceTimersByTimeAsync inside act, like the newer T184 test in the same file. The CaptureWidget download-button test asserted the button was enabled before the capabilities query had resolved (#672, #684); it now waits for that. Assertions are unchanged. valgul chose "Fix in #677". Co-Authored-By: Claude Opus 5.5 (1M context) Signed-off-by: Claude --- web/src/components/CaptureWidget.test.tsx | 5 ++-- web/src/routes/Share.test.tsx | 30 ++++++++++------------- 2 files changed, 16 insertions(+), 19 deletions(-) diff --git a/web/src/components/CaptureWidget.test.tsx b/web/src/components/CaptureWidget.test.tsx index 3bd3b425f..b753cd47f 100644 --- a/web/src/components/CaptureWidget.test.tsx +++ b/web/src/components/CaptureWidget.test.tsx @@ -794,8 +794,9 @@ describe("CaptureWidget", () => { const buttons = await screen.findAllByLabelText(/Download capture/i); expect(buttons.length).toBe(2); - // First button (Completed) should be enabled - expect(buttons[0]).not.toBeDisabled(); + // First button (Completed) should be enabled once the capabilities + // query resolves; the list can render before it does. + await waitFor(() => expect(buttons[0]).not.toBeDisabled()); // Second button (Failed) should be disabled expect(buttons[1]).toBeDisabled(); }); diff --git a/web/src/routes/Share.test.tsx b/web/src/routes/Share.test.tsx index d336fbe91..6f8e50f27 100644 --- a/web/src/routes/Share.test.tsx +++ b/web/src/routes/Share.test.tsx @@ -460,7 +460,9 @@ describe("SharePage", () => { }); it("T184: waits five seconds between successful resolve requests", async () => { - vi.useFakeTimers({ shouldAdvanceTime: true }); + // Pure fake timers, advanced with the async API: shouldAdvanceTime + // plus waitFor raced real time and timed out on busy CI runners. + vi.useFakeTimers(); vi.mocked(Shares.resolve).mockResolvedValue({ serverName: "mc-survival", @@ -469,25 +471,22 @@ describe("SharePage", () => { renderWithRouter("test-token"); - await waitFor(() => { - expect(screen.getByText(/The server is starting up/)).toBeInTheDocument(); - }); + await act(async () => { await vi.advanceTimersByTimeAsync(0); }); + expect(screen.getByText(/The server is starting up/)).toBeInTheDocument(); // First call on mount expect(vi.mocked(Shares.resolve)).toHaveBeenCalledTimes(1); // Advance time - vi.advanceTimersByTime(5000); + await act(async () => { await vi.advanceTimersByTimeAsync(5000); }); - await waitFor(() => { - expect(vi.mocked(Shares.resolve)).toHaveBeenCalledTimes(2); - }); + expect(vi.mocked(Shares.resolve)).toHaveBeenCalledTimes(2); vi.useRealTimers(); }); it("T184: transitions to Up when server is Running", async () => { - vi.useFakeTimers({ shouldAdvanceTime: true }); + vi.useFakeTimers(); vi.mocked(Shares.resolve) .mockResolvedValueOnce({ @@ -502,16 +501,13 @@ describe("SharePage", () => { renderWithRouter("test-token"); - await waitFor(() => { - expect(screen.getByText(/The server is starting up/)).toBeInTheDocument(); - }); + await act(async () => { await vi.advanceTimersByTimeAsync(0); }); + expect(screen.getByText(/The server is starting up/)).toBeInTheDocument(); - vi.advanceTimersByTime(5000); + await act(async () => { await vi.advanceTimersByTimeAsync(5000); }); - await waitFor(() => { - expect(screen.getByText("Online")).toBeInTheDocument(); - expect(screen.getByText("play.gameplane.example:25565")).toBeInTheDocument(); - }); + expect(screen.getByText("Online")).toBeInTheDocument(); + expect(screen.getByText("play.gameplane.example:25565")).toBeInTheDocument(); vi.useRealTimers(); }); From b386b7db082174d0fb95faa9fde8fcf692c2de7d Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 12 Oct 2026 22:42:32 +0000 Subject: [PATCH 4/4] docs(specs): keep the 013 SC-004 checkpoint pending until G-19 is proven Co-Authored-By: Claude Opus 5.5 (1M context) Signed-off-by: Claude --- specs/013-expand-test-coverage/tasks.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/specs/013-expand-test-coverage/tasks.md b/specs/013-expand-test-coverage/tasks.md index 64d7e441b..28eed7068 100644 --- a/specs/013-expand-test-coverage/tasks.md +++ b/specs/013-expand-test-coverage/tasks.md @@ -338,7 +338,7 @@ description: "Task list for 013-expand-test-coverage" *Implementation note (2026-10-12)*: lcov from run 38189418314: `CloneServerDialog.tsx` 89.29 % lines / 69.23 % branches, with a Cancel case but no API-error case, so T053 adds the API-error and cancel-discards-changes cases. `ServerActionsMenu.tsx` 100 % lines / branches / functions; it makes no API calls, so cancel and API errors are dialog-level (G-22 note). `ConsoleShell.tsx` 100 % lines. G-19 stays in progress: CI reports only package coverage for Go. -**Checkpoint**: SC-004 is met. Module thresholds stay unchanged. +**Checkpoint**: SC-004 stays pending until G-19 has per-file proof for `secrets_managed.go` (G-20 to G-22 are proven). Module thresholds stay unchanged. ---