Skip to content

test(cli): cover runClusterInfo/runClusterDoctor post-discovery - #258

Merged
saadqbal merged 3 commits into
developfrom
test/cluster-info-doctor
Jul 14, 2026
Merged

test(cli): cover runClusterInfo/runClusterDoctor post-discovery#258
saadqbal merged 3 commits into
developfrom
test/cluster-info-doctor

Conversation

@LukasWodka

@LukasWodkaLukasWodka commented Jul 13, 2026

Copy link
Copy Markdown
Contributor

Summary

Routes the last two commands that reached a cluster directlyrunClusterInfo (cluster info) and runClusterDoctor (doctor) — through the existing loadClusterFn/newClientsetFn seams, and adds a doctorRunFn seam over doctor.Run. This makes their post-discovery logic testable without a real kubeconfig or apiserver.

Pure seam routing — the package vars default to the real functions, so there is no production behavior change (5 direct calls → seam vars). This closes structural root-cause #1 from the coverage audit (runClusterInfo/runClusterDoctor bypassing the seam that resolveClusterTarget and the data commands already use; realProbeEnv was routed in an earlier PR).

Coverage

cli package own-coverage 77% → 79.3%; the two functions:

funcbeforeafter
runClusterInfoexit-3 branch only95.6%
runClusterDoctorauth half + kubeconfig-error only100%

runClusterInfo — exit-4 no-client, exit-5 release-found-but-no-token (proving the install-print block renders before token minting), all three arms of the token-expiry switch (server ExpiresAt / requested-only / static-secret "never"), and the image-digest configured/not-configured branches. Pins the security contract: the raw token is never printed, only sha256(token)[:8].

runClusterDoctor — the clientset-build error arm, the render loop (all three status arms + remedy hints), and the overall-verdict switch (Fail→exit 2 silent / Warn→exit 0 / OK→exit 0), with the auth half stubbed healthy so the verdict reflects the cluster checks.

Test plan

  • make ci green (build, vet, gofmt -s, go test -race -cover, errcheck/ineffassign/misspell, schema-check).
  • New tests use the established withClusterSeams + fake.NewSimpleClientset + TokenRequest PrependReactor (mirrors internal/cluster/token_test.go) and the existing stubBackend seam for doctor's auth half — no real cluster required.

🤖 Generated with Claude Code


Note

Low Risk
Test-only seams and new unit tests; production call sites still invoke the same underlying cluster helpers via package-level defaults.

Overview
cluster info and doctor now load kubeconfig through the same loadClusterFn / newClientsetFn hooks that data commands already use, replacing direct cluster.Load / cluster.NewClientset calls. Production still defaults those vars to the real implementations, so runtime behavior is unchanged.

doctor adds a doctorRunFn seam over internal/doctor.Run so tests can drive the Checks render loop and overall verdict without a live cluster.

New tests exercise runClusterInfo post-discovery: exit codes 4/5, install block before token mint, all three token-expiry display arms, static-secret fallback, digest configured vs placeholder, and the rule that only sha256(token)[:8] is printed. runClusterDoctor tests cover OK/Warn/Fail rendering, remedy hints, exit 2 vs 0 verdicts, and clientset-build failures (exit 3).

Reviewed by Cursor Bugbot for commit b76dbd7. Bugbot is set up for automated code reviews on this repo. Configure here.

LukasWodkaand others added 2 commits July 13, 2026 21:40
Route the last two commands that reached a cluster directly — runClusterInfo
(`cluster info`) and runClusterDoctor (`doctor`) — through the existing
loadClusterFn/newClientsetFn seams, and add a doctorRunFn seam over doctor.Run,
so their post-discovery logic is testable without a real kubeconfig or
apiserver. Pure seam routing: the package vars default to the real functions,
so there is no production behavior change.
New coverage (cli package own-coverage 77% -> 79.3%):
- runClusterInfo 95.6%: exit-4 no-client, exit-5 release-found-but-no-token
(proving the install-print block renders before token minting), and all three
arms of the token-expiry switch (server ExpiresAt / requested-only / static-
secret "never"), plus the image-digest configured/not-configured branches.
Pins the security contract: the raw token is never printed, only sha256[:8].
- runClusterDoctor 100%: the clientset-build error arm, the check render loop
(all three status arms + remedy hints), and the overall-verdict switch
(Fail->exit 2 silent / Warn->exit 0 / OK->exit 0), with the auth half stubbed
healthy so the verdict reflects the cluster checks.
make ci green.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Both arms of the token-expiry switch print the "expires in" label, so the
happy-path assertion didn't prove the server-timestamp arm was taken vs the
requested-lifetime fallback. Add a negative assertion that the arm-2 message is
absent when ExpiresAt is set — makes the test resistant to an arm-1/arm-2 swap.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@LukasWodka

Copy link
Copy Markdown
ContributorAuthor

bugbot run

@cursorcursorBot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Bugbot reviewed your changes and found no new issues!

Comment @cursor review or bugbot run to trigger another review on this PR

Reviewed by Cursor Bugbot for commit 1329867. Configure here.

@LukasWodkaLukasWodka self-assigned this Jul 14, 2026
Gremlins found the arm-2 test asserted only the 'requested; server may cap
shorter' note, not the rendered duration — so a `*`->`/` mutant on
`time.Duration(ExpirationSeconds) * time.Second` (which would display ~0s)
survived. Assert 600s renders as 10m0s. Mutation-proven: the `/` mutant now
fails the test.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@LukasWodka

Copy link
Copy Markdown
ContributorAuthor

bugbot run

@cursorcursorBot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Bugbot reviewed your changes and found no new issues!

Comment @cursor review or bugbot run to trigger another review on this PR

Reviewed by Cursor Bugbot for commit b76dbd7. Configure here.

@saadqbalsaadqbal left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 👍 Production side is just the behavior-preserving seam routing (loadClusterFn/newClientsetFn/doctorRunFn, all defaulting to the real fns), and the tests finally reach the post-discovery paths that only a live cluster could hit before — nice to see the token-expiry switch arms and the sha256[:8] never-print-the-raw-token contract pinned. Same stacked-rebase note as the sibling seam PRs.

@saadqbal
saadqbal merged commit 187a4bf into developJul 14, 2026
18 checks passed
@saadqbal
saadqbal deleted the test/cluster-info-doctor branch July 14, 2026 08:55
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@LukasWodka@saadqbal