Repository navigation
build: make check runs the nested-module loop, via the script CI uses - #788
Merged
Merged
Conversation
`go test ./...` does not descend into a directory with its own go.mod, so the 12 modules under lambda/ and scripts/ were invisible to `make check` while CI checked them. The local gate was weaker than the remote one. Demonstrated rather than asserted — a nested module with a deliberate syntax error: go test -short ./... PASSES scripts/nested-modules.sh FAILS A module that does not compile passed the pre-commit check. That gap cost two round trips in one session: #770 LogRetentionDays added to three templates and wired into none. Caught by lambda/ttl-reaper's own test — which only CI ran. #771 pkg/aws gained six SDK imports; sweep-orchestrator needed re-tidying and failed on a missing go.sum entry. Both found after a push, where the fix is a second commit and another wait. ## One script, two callers Extracted to scripts/nested-modules.sh and called from both `make check` and ci.yml, rather than copied into the Makefile. Two copies of the loop would drift back apart, and the drift is precisely what this fixes. The workflow's inline loop is gone. The script discovers modules rather than listing them, and refuses to pass when it finds none — a broken discovery is not a clean repo. That is the same reasoning as lambda-deploy-census.sh (#654), whose first version hardcoded five function names in an account holding sixteen, and the three incomplete footprint inventories in #653. Scoping the find to lambda/* would also have silently excluded scripts/ the day that module appeared, which is the #136 bug one level up. ## A flag I removed before shipping The first version had --quiet, documented as "only report failures". It did not do that: go test writes its own output and the flag only suppressed this script's headers. A flag whose name overstates what it does is worse than no flag, so there is no --quiet and the comment says why. `make check` is correspondingly slower. That is the right trade against discovering the same class of failure in CI twice in one evening.
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
scttfrdmn
added a commit
that referenced
this pull request
Oct 10, 2026
Much of this repo's CI is gates — tests whose job is to fail when someone
removes a property, not to check that today's code works. `make check`
runs dozens. They are cheap to write and easy to write uselessly, and a
gate that cannot fail is worse than no gate because it is counted as
coverage.
Four rules, each from a gate in THIS repo that broke it:
1. Verify against a revert that COMPILES. Three gates were "verified"
against reverts that failed go vet first — unused variable, dropped
import — so the test never ran and the verification proved nothing.
2. Test the invariant where it lives. #787's !Determined guard survived
a compiling revert because every test built the struct through
DetectIdleness, which cannot produce the offending combination.
3. Match the assignment or invocation, never the bare name. Four gates
matched their own prose: spawn:version inside a parameter
Description and an echo line, put-retention-policy inside a comment
saying which call to make.
4. Discover the set, don't list it, and fail when discovery finds
nothing. A gate over a hand-written list reports a clean bill of
health for everything the list forgot.
Plus the corollary: a suspicious negative indicts the invocation more
often than the system. Four false negatives in one session came from the
probe — `strings` on a Go binary, awk $2 on tab-separated `gh pr checks`
output, --max-items reshaping a JMESPath result to null, and
FilterLogEvents --limit returning an empty FIRST page with a
continuation token. The last nearly produced a fix to working code.
Written from the gates as they stand, not from memory: the quoted match
forms are the ones in lambda-deploy-census.sh, and two examples were
corrected while drafting. The census does not hardcode function names
today — its first version did, which is the point — and the footprint
exact-name map is cited as the surviving unavoidable list, with the rule
that entries come from a live account and never from recollection.
Also fixes the `make check` description, which still said "fmt, vet,
lint, short tests" after #788 added the nested-module loop.
Docs only. make check green, including all 12 nested modules.
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.
go test ./...does not descend into a directory with its owngo.mod, so the 12modules under
lambda/andscripts/were invisible tomake checkwhile CIchecked them. The local gate was weaker than the remote one.
Demonstrated rather than asserted — a nested module with a deliberate syntax
error:
A module that doesn't compile passed the pre-commit check.
What the gap cost
LogRetentionDaysadded to three templates and wired into none — caught bylambda/ttl-reaper's own test, which only CI ranpkg/awsgained six SDK imports;sweep-orchestratorneeded re-tidying and failed on a missinggo.sumentryBoth found after a push, where the fix is a second commit and another wait. Twice
in one session.
One script, two callers
Extracted to
scripts/nested-modules.sh, called from bothmake checkandci.yml, rather than copied into the Makefile. Two copies of the loop would driftback apart — and the drift is exactly what this fixes. The workflow's inline loop
is gone.
The script discovers modules rather than listing them, and refuses to pass
when it finds none, because a broken discovery is not a clean repo. Same
reasoning as
lambda-deploy-census.sh(#654), whose first version hardcoded fivefunction names in an account holding sixteen. Scoping the
findtolambda/*would also have silently excluded
scripts/the day that module appeared — the#136 bug one level up.
A flag I removed before shipping
The first version had
--quiet, documented as "only report failures". It didn'tdo that:
go testwrites its own output and the flag only suppressed thisscript's headers. A flag whose name overstates what it does is worse than no flag,
so there isn't one, and the comment says why.
Trade-off
make checkis correspondingly slower — it now builds, vets and tests 12 extramodules. That's the right trade against discovering the same class of failure in
CI twice in one evening.