test: cover pkg/deploy against Substrate (4.1% -> 75.5%) - #125
Merged
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
pkg/deploy (lagotto's `deploy`/`teardown` stack manager) was the least- covered non-trivial package in the repo (#1): only its 4 pure helper functions (LambdaArtifactURL, DefaultBucketName, LambdaObjectKey, failedCreateStates) had tests. Deploy/Teardown/StackOutputs/ensureBucket/ uploadArtifact — the actual AWS-calling code — had none, despite Deployer already taking an injectable aws.Config (deploy.New(cfg)), which is exactly what a Substrate-backed test needs. Adds pkg/deploy/deploy_substrate_test.go using the same pkg/testutil.SubstrateServer + fake-httpGet pattern pkg/watcher's own substrate tests already use, covering: - Deploy end-to-end (bucket create + artifact upload + CreateStack, verified against the real SAM-transformed stack's outputs) - Redeploy (the UpdateStack branch of createOrUpdate) - Teardown (DeleteStack + wait, then stackState confirms it's gone) - StackOutputs for both a deployed and an undeployed stack - ensureBucket's idempotent-already-exists path - uploadArtifact's httpGet-error and non-200-status error paths pkg/deploy: 4.1% -> 75.5% coverage. Aggregate repo coverage: 51.2% -> 54.4%. Not attempted: the failed-create-state (ROLLBACK_COMPLETE etc.) delete-and-recreate branch in createOrUpdate, which would need Substrate's fault-injection to force a stack into that state — left uncovered rather than adding a speculative/brittle test for it. Progresses #1 (broad coverage-debt issue; scoped here to the single weakest package rather than a blanket aggregate-percentage chase).
scttfrdmn
force-pushed
the
test/1-deploy-coverage
branch
from
August 19, 2026 02:40
5837fbf to
adf2106
Compare
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
pkg/deploy(deploy_substrate_test.go), coveringDeploy,Teardown,StackOutputs,ensureBucket, anduploadArtifact— previously untested; only the package's 4 pure helper functions (LambdaArtifactURL,DefaultBucketName,LambdaObjectKey,failedCreateStates) had coverage.pkg/deploycoverage: 4.1% → 75.5%. Aggregate repo coverage: 51.2% → 54.4% (measured on currentmain; note the issue's original 20.9% figure is stale — a lot of test debt has already been paid down elsewhere since Raise test coverage — only pkg/watcher tested, aggregate 20.9% #1 was filed).Progresses #1.
Why this package, and why not a blanket coverage push
Per the task's own framing, I looked at where aggregate coverage sits today rather than trusting the issue's number, and picked the single weakest non-trivial package instead of chasing an aggregate percentage target:
pkg/deploycmd/pkg/watcherpkg/snipepkg/runtimeiampkg/failure,pkg/awscfgpkg/deploywas the clear outlier and the best-scoped target:Deployeralready takes an injectableaws.Config(deploy.New(cfg)), so it was straightforward to point atpkg/testutil.SubstrateServer— the same substrate-backed patternpkg/watcher's own tests already use — with no source changes needed, just missing tests.cmd/is comparatively lower-value to chase: most of its 0%-covered functions arerun*cobra handlers that are thin CLI-argument-to-library-call wiring, better suited to a follow-up that specifically covers flag parsing/wiring rather than folded into this PR.Not attempted here: the
createOrUpdatefailed-create-state (ROLLBACK_COMPLETEetc.) delete-and-recreate branch — forcing a stack into that state would need Substrate's fault-injection wired up, which felt like more machinery than this PR's scope justified. Left as a known gap rather than adding something speculative/brittle.Verification
go build ./...,go vet ./...,go test ./...clean from repo root.go build ./...,go vet ./...,go test ./...clean fromlambda/capacity-poller(untouched, confirmed still green).golangci-lint run ./pkg/deploy/...— 0 issues.Test plan