Skip to content

ci(examples): cover go-http-zip and standardize template filenames #759

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
JamBalaya56562 wants to merge 2 commits into
base: main
Choose a base branch
from
Open

ci(examples): cover go-http-zip and standardize template filenames #759

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
78f18a8
ci(examples): cover go-http-zip and standardize template filenames
JamBalaya56562 Aug 8, 2026
e8e49bd
ci(examples): fail when an example uses the template.yml spelling
JamBalaya56562 Aug 8, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 15 additions & 0 deletions .github/workflows/examples.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,20 @@ jobs:
use-installer: true
token: ${{ secrets.GITHUB_TOKEN }}

# The loop below matches template.yaml exactly, so an example that uses
# the template.yml spelling is skipped silently and this job still passes
# green -- which is how go-http-zip stayed unvalidated. Reject the other
# spelling up front so the gap surfaces as a failure, not as missing
# coverage.
- name: Enforce template.yaml naming
run: |
stragglers=$(find examples -name "template.yml")

@aws-sam-tooling-bot aws-sam-tooling-bot Bot Aug 8, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[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
fi

Worth 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.

@JamBalaya56562 JamBalaya56562 Aug 9, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

if [ -n "$stragglers" ]; then
echo "These examples must be renamed to template.yaml:"
echo "$stragglers"
exit 1
fi

- name: Validate all SAM templates
run: |
failed=0
Expand Down Expand Up @@ -190,6 +204,7 @@ jobs:
- { 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" }

@aws-sam-tooling-bot aws-sam-tooling-bot Bot Aug 8, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[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); do

Any 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); do

If 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.

@JamBalaya56562 JamBalaya56562 Aug 8, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

- { name: remix-zip, path: /, expect_body: "Welcome to", port: "8000" }
- { name: springboot-zip, path: /v1/, expect_body: "Hello, world!", port: "8000" }
steps:
Expand Down
0 examples/go-http-zip/template.yml → examples/go-http-zip/template.yaml
View file
Open in desktop
File renamed without changes.
0 examples/sinatra/template.yml → examples/sinatra/template.yaml
View file
Open in desktop
File renamed without changes.