test: add timeout isolation regression coverage#118
Open
hwbrzzl wants to merge 3 commits into
Open
Conversation
Pin goravel/gin and goravel/fiber to the timeout fix snapshots so the example app can verify 408 timeout responses and stale-response isolation under both drivers.
There was a problem hiding this comment.
Pull request overview
This PR updates the example app and its integration tests to cover a timeout “isolation” regression scenario across the Gin and Fiber HTTP drivers, while also bumping goravel/gin and goravel/fiber to snapshot versions that include the timeout fix.
Changes:
- Add new example routes to simulate a timed-out request and a subsequent “fresh” request.
- Add a new
TestTimeoutIsolationintegration test that asserts the timeout response and verifies the next response is not contaminated. - Update URL-route assertions to match the new closure ordering, and bump module dependencies to the referenced snapshots.
Reviewed changes
Copilot reviewed 3 out of 4 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
tests/feature/http_test.go |
Adds timeout-isolation regression test and updates URL route assertions. |
routes/api.go |
Adds /timeout-isolated and /timeout-after routes used by the new regression test. |
go.mod |
Bumps goravel/gin and goravel/fiber snapshots plus related indirect deps. |
go.sum |
Updates checksums for the dependency changes. |
Comments suppressed due to low confidence (1)
tests/feature/http_test.go:219
- This assertion hard-codes the Go compiler's synthetic function name (Api.func13.2), which changes whenever new closures are added earlier in routes/api.go. To make this test resilient, consider decoding the JSON and asserting stable fields only, or asserting the handler has a stable prefix instead of the exact func index.
content, err = resp.Content()
s.Require().NoError(err)
s.Equal(`{"full_url":"http://example.com/url/post/1?a=1\u0026b=2","info":{"handler":"goravel/routes.Api.func13.2","method":"POST","name":"url.post","path":"/url/post/{id}"},"info1":{"handler":"goravel/routes.Api.func13.2","method":"POST","name":"url.post","path":"/url/post/{id}"},"method":"POST","name":"url.post","origin_path":"/url/post/{id}","path":"/url/post/1","url":"/url/post/1?a=1\u0026b=2"}`, content)
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Derive timeout overlap timings from config, stop timers explicitly, and make route URL assertions resilient to compiler-generated handler names.
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
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
goravel/ginandgoravel/fiberto the timeout-fix snapshots that preserve408 Request Timeoutresponses without leaking stale writes into later requestsRequest Timeoutand the next request still returns its own fresh bodyWhy
The timeout fixes in
goravel/ginandgoravel/fiberare about preserving the existingTimeout(...)behavior while preventing pooled request state from being reused after a request has already timed out. This example repo is the right place to pin those driver snapshots and prove the integration still behaves correctly under both HTTP drivers.Before these driver fixes, the timeout middleware could return
408while the original handler kept running against pooled request and response wrappers. That created the bug class where a later write likelate:stalecould bleed into the next request. After the fixes, the timed-out request still yieldsRequest Timeout, but the follow-up request gets its own clean response body instead of leaked output from the abandoned handler.