PR 2: DI tests (Service, EspClient, runX, persistentPreRunE)#31
Merged
Conversation
The dispatcher checkSSMError has no GetMany case, so checkSSMByPathError was never reached at runtime — GetMany falls through to checkBaseSSMErrors in the default branch. Drop the function and its now-orphaned isolated test. The TestCheckSSMError dispatcher test continues to pin the fall-through behavior.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
PR 2 of 2 implementing the DI and tests spec at
docs/superpowers/specs/2026-05-08-di-and-tests-design.md.This PR is purely additive — new tests against the seams introduced in PR 1 (#30). No production code changes.
test(ssm): cover Service methods via fake ssmAPI) — 13TestService_*tests coveringSave,GetOne,GetMany,Delete,Copyhappy paths and AWS error mapping. LocalfakeSSMAPIimplements the unexportedssmAPIinterface; pagination is exercised via a multi-page output sequence.test(client): cover EspClient wrapper methods) — 11TestEspClient_*tests covering all six wrapper methods. The non-trivial cases pin the composition order inCopy(re-fetch with decrypt) andMove(Get → Save → Delete with short-circuit on each failure).test(cmd): cover runX functions via fake EspClient) — 21TestRun*tests across all seven subcommands.cmd/testutil_test.goprovidesfakeBackend,newTestEspClient,newCmdWithFlags,testConfigshared across files; thefakeBackendis a deliberate copy of the one ininternal/client/client_test.go(two callers, no shared package).test(cmd): cover persistentPreRunE env-var validation) — 3TestPersistentPreRunE_*tests covering missing-region, missing-profile, and unsupported-backend error paths. CustomunsetEnvhelper becauset.Setenvonly sets.Total: 48 new tests across 8 new/modified test files.
Coverage gaps explicitly accepted
Per the spec:
ssm.New()— callsconfig.LoadDefaultConfig, environment-coupled.app.InitQuestionsandcmd/init.go— interactivesurvey.Ask.cmd/get.go::display*— formatting only, no logic.cmd/root.go::configureLogging— sets slog default, no failure path.main.go— single line.Test plan
go build ./...cleango vet ./...cleango test ./...green; all 48 new tests passtestingonly)Notes for review
fakeBackendis duplicated betweeninternal/client/client_test.goandcmd/testutil_test.gorather than promoted to a sharedclienttestpackage — appropriate for two callers, avoids exporting test-only API.internal/ssm/errors.go::checkSSMByPathErroris dead code. ThecheckSSMErrorswitch has noGetManycase, so it falls through tocheckBaseSSMErrors. Caught becauseTestService_GetMany_ErrorMidIterationsucceeds viaInternalServerErrorbeing recognized by the base mapper, not the per-action one.