Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
251 changes: 251 additions & 0 deletions api/internal/handlers/secrets_managed_test.go
Original file line number Diff line number Diff line change
@@ -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)
}
})
}
10 changes: 5 additions & 5 deletions specs/013-expand-test-coverage/coverage-gaps.md
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
# Coverage Gap Record

**Summary**: 5/24 closed · 0 closed unproven · 2 in progress · 16 open · 1 unit-only by design
**Summary**: 8/24 closed · 0 closed unproven · 1 in progress · 14 open · 1 unit-only by design
**Last updated**: 2026-10-12

| ID | Category | Gap | Story | FR | Priority | Status | Evidence | Notes |
Expand All @@ -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 | | |
10 changes: 6 additions & 4 deletions specs/013-expand-test-coverage/tasks.md
Original file line number Diff line number Diff line change
Expand Up @@ -311,7 +311,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
Expand All @@ -321,7 +321,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
Expand All @@ -330,13 +330,15 @@ 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.

**Checkpoint**: SC-004 is met. Module thresholds stay unchanged.
*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.
Comment thread
ValgulNecron marked this conversation as resolved.

**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.

---

Expand Down
5 changes: 3 additions & 2 deletions web/src/components/CaptureWidget.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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();
});
Expand Down
Loading
Loading