ci(examples): cover go-http-zip and standardize template filenames - #759
ci(examples): cover go-http-zip and standardize template filenames#759JamBalaya56562 wants to merge 2 commits into
Conversation
go-http-zip used template.yml, which the validate job's find (template.yaml) never matched, so the example was untested. Rename the two stragglers (go-http-zip, sinatra) to the conventional template.yaml -- SAM auto-detects either name, so there is no functional change -- and add go-http-zip to the test-zip matrix. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
| - { name: fasthtml-zip, path: /, expect_body: "Hello World", port: "8000" } | ||
| - { name: flask-zip, path: /, expect_body: "message", port: "8000" } | ||
| - { name: gin-zip, path: /, expect_body: "message", port: "8000" } | ||
| - { name: go-http-zip, path: /, expect_body: "Hello, Website!", port: "3000" } |
There was a problem hiding this comment.
[GENERAL] The matrix entry and the renames are consistent with the example (cmd/website/main.go hardcodes :3000 and the template sets PORT: 3000, so port: "3000" and expect_body: "Hello, Website!" both match), but the underlying cause of go-http-zip going untested is still present.
The validate job at line 40 discovers templates with:
for template in $(find examples -maxdepth 2 -name "template.yaml" | sort); doAny example whose template is not named exactly template.yaml is skipped silently and the job still passes green — which is precisely how go-http-zip stayed unvalidated. Standardizing the two existing filenames fixes today's gap but relies on convention to prevent recurrence; the next contributor who adds template.yml (a name SAM accepts) reintroduces the same invisible hole.
Making discovery match both names keeps the loop honest without depending on reviewers to catch the filename:
for template in $(find examples -maxdepth 2 \( -name "template.yaml" -o -name "template.yml" \) | sort); doIf you prefer to keep enforcing the single conventional name, an explicit failure is better than a silent skip — e.g. fail the job when an example directory contains a template.yml, so the mismatch surfaces in CI rather than as missing coverage.
There was a problem hiding this comment.
Thanks — the underlying point is right, so I've addressed it in a follow-up commit.
I went with the second suggestion rather than the first. Matching both spellings in find would fix the silent skip, but it also re-legitimises the template.yml name this PR just removed, so filename drift would stay permanently acceptable. Failing the job keeps a single conventional name and enforces it in CI instead of relying on reviewers to spot it.
The guard searches at any depth, so it also covers nested examples the -maxdepth 2 loop would miss. The repository has zero template.yml files today, so it passes as-is.
The validate job discovers templates with `find -name "template.yaml"`, so an example using the template.yml spelling is skipped silently and the job still passes green. Standardizing the two existing filenames closes today's gap but leaves the failure mode intact: the next example added as template.yml is unvalidated again, with no signal. Reject the other spelling before the validation loop runs, so a filename mismatch surfaces as a failed job instead of as missing coverage. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
| # coverage. | ||
| - name: Enforce template.yaml naming | ||
| run: | | ||
| stragglers=$(find examples -name "template.yml") |
There was a problem hiding this comment.
[GENERAL] The guard closes the exact case that bit go-http-zip, but it hard-codes the inverse of the discovery rule instead of deriving from it, so the two can still disagree:
# guard (new): any depth, only the .yml spelling
find examples -name "template.yml"
# discovery (line 51 loop): depth <= 2, only the .yaml spelling
find examples -maxdepth 2 -name "template.yaml"Anything that is a SAM template but doesn't land at exactly examples/<name>/template.yaml — e.g. examples/foo/infra/template.yaml — passes the guard and is still skipped silently by validation, which is the same green-but-uncovered outcome the step exists to prevent. Conversely, the guard has no -maxdepth, so a nested third-party template.yml that validation was never meant to lint would fail the build.
Expressing the invariant once ("every SAM template lives at examples/<name>/template.yaml") covers both directions:
stragglers=$(find examples -name 'template.y*ml' | grep -vE '^examples/[^/]+/template\.yaml$' || true)
if [ -n "$stragglers" ]; then
echo "These templates must be moved to examples/<name>/template.yaml:"
echo "$stragglers"
exit 1
fiWorth pairing with a zero-match assertion in the validation loop itself — it currently exits 0 when find returns nothing, so a future change to the directory layout would also read as a pass rather than a failure.
I confirmed the rest of the change: no references to template.yml remain outside the diff, the renamed templates match the shapes of examples the validate job already lints (go-http-zip mirrors gin-zip, sinatra mirrors fastapi-response-streaming), and the matrix entry's port: "3000" / expect_body: "Hello, Website!" line up with cmd/website/main.go and the template's PORT: 3000. No example currently has a template below depth 2, so the point above is about keeping the guard honest going forward, not a present-day break.
There was a problem hiding this comment.
Both directions are real, and I'd rather not encode the stricter rule here.
The proposed invariant enforces that every SAM template sits at examples/<name>/template.yaml, but the repo already nests examples one level deeper — examples/datadog/*/cdk/ and examples/sls/nestjs/ — they just don't use SAM today. A future examples/datadog/fastapi/template.yaml would follow the existing grouping and still fail the guard, and the right fix in that case is widening the validation loop's -maxdepth 2, not rejecting the layout.
Since nothing currently has a template below depth 2 (as you note), I'd rather keep this PR to the coverage gap it set out to close and leave the depth question to a change that can decide the nested-example policy properly.
Problem
The
validatejob discovers SAM templates with afindthat matches one exact filename:find examples -maxdepth 2 -name "template.yaml"Two examples spelled it
template.ymlinstead —go-http-zipandsinatra— sofindnever matched them and both were silently skipped by template validation.go-http-zipwas missing from thetest-zipmatrix as well, which left it with no CI coverage at all.Changes
Rename
template.yml→template.yamlingo-http-zipandsinatra.These were the last two using the short spelling, so every example now shares one filename and
findpicks them all up. SAM auto-detects either spelling, so this is a no-op for anyone deploying the examples — the rename exists purely so CI stops skipping them.Add
go-http-zipto thetest-zipmatrix, assertingHello, Website!on/.The entry sets
port: "3000"rather than the8000its neighbours use, because the app hardcodeshttp.ListenAndServe(":3000", nil); that matches thePORT: 3000its template already passes to the adapter.Fail the
validatejob when any example usestemplate.yml(second commit, added in response to review feedback).Renaming the two stragglers closes today's gap but leaves the failure mode intact — the next example added as
template.ymlwould be skipped just as silently. A guard ahead of the validation loop turns that filename mismatch into a red job instead of invisible missing coverage. It searches at any depth, so it also covers nested examples that the-maxdepth 2loop would not reach.Notes
CI coverage only — no adapter or runtime code is touched, and the renamed templates are byte-identical (100% similarity) to their originals. The repository currently has zero
template.ymlfiles, so the new guard passes as-is./cc @bnusunny — apologies for the ping. This has been open since June 5 without triage, so I wanted to surface it in case it slipped through. No rush on my end, and I'm happy to rebase or split it up if that makes review easier.