Pure-logic unit tests (PR 2 of 2)#27
Merged
Merged
Conversation
Add table-driven tests for the pure logic in internal/app: - config_test.go: TestGetAppPath and TestGetAppParamPath cover the /OrgName/Env/AppName/[param] convention plus edge cases (empty fields, leading slashes in inputs, slash-containing param names). - init_test.go: TestUpdateWithInput pins the comma-list parsing rule for Envs (regexp ", *" splits correctly with zero, one, or multiple trailing spaces). TestCreateEspFile asserts field-by-field that createEspFile copies the espFile-shaped fields and ignores Config-only fields like IsEspProject, Env, Path, Filename. Stdlib testing only; no testify.
Fill the empty stubs and extend the existing list test to cover the pure helpers in the cmd package. - cmd/put_test.go (was an empty stub): TestEspName pins formatParamName's actual rule. Already-prefixed inputs pass through (HasPrefix, not exact match — so "ACMEISH" with prefix "ACME" is unchanged); other inputs become OrgPrefix + "_" + ToUpper(n). Hyphens are preserved as-is, not converted to underscores; the test makes that explicit. - cmd/get_test.go (was an empty stub): TestGetParamPath pins the leading-"/" → literal path / short-name → GetAppParamPath rule. - cmd/list_test.go: TestGetPathRelative covers the empty-args branch (getPath returns esp.GetAppPath() — the project's base path). Helpers withOrgPrefix and withAppConfig save/restore the package-level esp global so each test runs in isolation.
Two new test files (no production code change):
- internal/ssm/utils_test.go:
- TestSelectType — true → SecureString, false → String.
- TestConvertToEspParam — table covers SecureString (Secure
flips to true), plain String, StringList (Secure stays
false — only SecureString flips it), and nil-pointer
fields rendering as zero values via aws.ToString /
aws.ToTime.
- internal/ssm/errors_test.go:
- One test per checkSSM*Error function, table-driven against
each typed AWS error the function maps. Each table also
covers a generic smithy.APIError fallthrough case and a
plain non-API error to assert the no-match-returns-nil
path.
- TestCheckSSMByPathError pins the asymmetry that this
function lacks the smithy.APIError fallback that the
others have, so a generic API error returns nil.
- TestCheckSSMError exercises the dispatcher: each known
action routes to its mapper; unknown actions (e.g.
GetMany — no case) fall through to checkBaseSSMErrors.
Stdlib testing only.
Fill the empty cmd/client/client_test.go stub with TestNewUnsupportedBackend: passing a Backend value other than "ssm" must return a non-nil error, a nil *EspClient, and an error message that names the offending backend. A comment in the test file explains why the "ssm" happy path is intentionally not covered here: it would require AWS credentials (LoadDefaultConfig) or a dependency-injection seam, both deferred per the cleanup-and-tests design's Non-Goals.
The previous note said the four files that had been empty stubs (cmd/get_test.go, cmd/put_test.go, internal/app/config_test.go, internal/client/client_test.go) were stubs to satisfy the test runner. They are all populated now, so replace that paragraph with the actual test conventions: stdlib testing only, table- driven for multi-case functions, AWS-side-effect functions intentionally unit-test-free, and the cmd-package global swap helpers (withOrgPrefix, withAppConfig). This replaces the conditional "delete unused stub files" commit the design doc carved out — the audit found no stubs left to delete, so the slot is repurposed for the docs refresh.
3 tasks
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
1 participant
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
Implements PR 2 of the cleanup-and-tests design (
docs/superpowers/specs/2026-05-05-cleanup-and-tests-design.md). All test files use stdlibtestingonly, table-driven where the function has multiple distinct cases. No production code changes.Builds on the cleaner shape from PR 1 (#26).
Commits (in order)
test: cover internal/app path builders and init—TestGetAppPathandTestGetAppParamPathlock down the/OrgName/Env/AppName/[param]convention and edge cases (empty fields, leading slashes in inputs, slash-containing param names).TestUpdateWithInputpins the comma-listregexp.MustCompile(", *").Splitrule.TestCreateEspFileasserts which fields cross fromConfigtoespFileand which don't.test: cover cmd path/name helpers—TestEspName(the spec name; testsformatParamName) pins the actual rule:HasPrefix → unchanged, elseOrgPrefix + "_" + ToUpper(n). Hyphens are preserved, not converted to underscores — this surfaces a behavior the design doc described differently; the test pins what the code actually does.TestGetParamPathcovers the slash-prefix vs. short-name routing incmd/get.go.TestGetPathRelativecovers the empty-args branch ofgetPathincmd/list.go. AddswithOrgPrefix/withAppConfighelpers to swap the package-level globals in and out per test.test: cover internal/ssm utils and errors—TestSelectTypeandTestConvertToEspParam(includingSecure: trueflip onSecureString). One test percheckSSM*Errorfunction plus the dispatcher; each covers typed-error pass-through, thesmithy.APIErrorfallback, and thenilno-match path.TestCheckSSMByPathErrorpins the asymmetry that this function has nosmithy.APIErrorfallback — generic API errors returnnil, unlike its siblings.test: cover internal/client.New unsupported-backend path—TestNewUnsupportedBackendconfirms a non-"ssm"backend returns(nil, error)with the offending backend named in the message. The"ssm"happy path is intentionally not covered (would require AWS credentials or a DI seam — explicitly deferred per the spec's Non-Goals).chore: refresh CLAUDE.md testing notes— replaces the stale "these four files are empty stubs" paragraph (all four are now populated) with the actual test conventions used in this PR.Behavior pinned by the new tests (worth a second look)
formatParamNamedoes not convert hyphens to underscores; onlyToUpper. The spec asked for "hyphens to underscores" but the code preserves hyphens —TestEspName/hyphenated_input...makes this explicit.getPath(cmd/list.go) returns the positional arg verbatim for any non-empty input; only the empty-args branch routes throughGetAppPath().TestGetPathEnvVarNamealready documented this;TestGetPathRelativeadds the empty-args side.checkSSMByPathErrorhas nosmithy.APIErrorfallback. A generic API error from aGetParametersByPathcall would returnnilfrom this mapper, which is then handled bymapErr's raw-error fallback — so callers still see the failure. Worth knowing if the dispatch incheckSSMErrorever wiresGetManyto use it.Spec deviations
Test Plan
go build ./...cleango vet ./...cleango test ./...— every package now reportsok(was several[no test files])