Skip to content

ci(examples): assert exact status/body instead of liveness-only check#755

Open
JamBalaya56562 wants to merge 2 commits into
aws:mainfrom
JamBalaya56562:ci/examples-verify-exact-response
Open

ci(examples): assert exact status/body instead of liveness-only check#755
JamBalaya56562 wants to merge 2 commits into
aws:mainfrom
JamBalaya56562:ci/examples-verify-exact-response

Conversation

@JamBalaya56562
Copy link
Copy Markdown
Contributor

@JamBalaya56562 JamBalaya56562 commented Jun 4, 2026

Summary

Before this PR, the verify step in test-image / test-zip / test-stream jobs treated any 2xx-4xx response as success (a plain liveness check). That means a broken route returning 404, or an app that booted but served empty/wrong content, would still pass example CI silently.

This PR adds exact response assertions: every example now declares the expected HTTP status (and optionally a body substring), and the verify step fails unless the real response matches.

Changes

  • Adds .github/scripts/verify-http.sh — polls an endpoint until both conditions hold:

    • the status code matches one of the expected values (e.g. 200, or 200,307 for redirect-then-page)
    • the response body contains an optional expected substring

    Uses a wall-clock deadline (90s) plus a short per-request timeout (curl -m 5) so a hanging endpoint can't multiply the wait window. Failure prints the last status, the expected substring, and the first 30 lines of the last body as diagnostics.

  • Updates .github/workflows/examples.yaml — converts the matrix in each verify job from plain example names to objects carrying {path, expect_status, expect_body}, so every example asserts its real response.

  • examples/expressjs/ — switches the local listen port from 8080 to 8000 (ENV PORT=8000 + EXPOSE 8000) so the example doesn't collide with SAM's Runtime Interface Emulator on 8080 during local testing. README updated to match.

  • examples/expressjs-zip/template.yaml — same PORT: 8000 adjustment for the zip-style variant, with an inline comment explaining the reason.

  • examples/fasthtml-response-streaming/ — the app hardcoded serve(port=8080) and ignored the PORT environment variable that every other example (e.g. fastapi-response-streaming) already respects. Reads PORT with an 8080 fallback so the listening port can be overridden. With the app now honoring PORT, the appport workaround in the test-stream job is dropped: the image case maps 8000:8000 like the rest instead of routing host 8000 to the hardcoded 8080.

Test plan

  • All three matrix jobs (test-image, test-zip, test-stream) pass on this PR.
  • Hanging-endpoint case stays bounded: a non-responsive example fails within ~95s instead of multiplying out to ~16 minutes.
  • Failure diagnostic is useful: when an example returns the wrong status/body, the log shows the actual status, the expected substring, and a body excerpt.
  • fasthtml-response-streaming honors PORT=8000 in the test-stream image job and the verify step passes against 8000:8000.

🤖 Generated with Claude Code

The verify step in test-image/test-zip/test-stream treated any 2xx-4xx
response as success, so a broken route (404) or an app that booted but
served the wrong/empty content still passed.

Add .github/scripts/verify-http.sh which waits for an endpoint to return
an exact status code and (optionally) a known body substring, and convert
each job's matrix to objects carrying {path, expect_status, expect_body}
so every example asserts its real response.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@JamBalaya56562 JamBalaya56562 requested a review from a team as a code owner June 4, 2026 01:29
@bnusunny
Copy link
Copy Markdown
Contributor

bnusunny commented Jun 4, 2026

Thanks for improving the testing for examples. One more example needs a small change:

The port is hard coded here. Should be able to overwrite with PORT.

@JamBalaya56562
Copy link
Copy Markdown
Contributor Author

Good catch, thanks! Fixed in 0408b33serve() now reads PORT (defaulting to 8080), matching the other examples. Also dropped the now-unnecessary appport workaround in the test-stream job.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants