Add gateway auth middleware integration tests#38
Conversation
📝 WalkthroughWalkthroughAdds a new Go test file for the gateway package containing integration-style tests covering AuthMiddleware, RateLimitMiddleware, RequestIDMiddleware, CORSMiddleware, RecoveryMiddleware, and TimeoutMiddleware, including verification of middleware chain ordering between authentication and rate limiting. ChangesGateway middleware tests
Estimated code review effort: 2 (Simple) | ~15 minutes Sequence Diagram(s)sequenceDiagram
participant Client
participant AuthMiddleware
participant RateLimitMiddleware
participant Handler
Client->>AuthMiddleware: Request with/without Bearer token
alt token missing/invalid
AuthMiddleware-->>Client: 401 Unauthorized
else token valid
AuthMiddleware->>AuthMiddleware: Set ContextKeyUserID, ContextKeySessionID
AuthMiddleware->>RateLimitMiddleware: Forward request with context
alt API key over limit
RateLimitMiddleware-->>Client: 429 Too Many Requests
else within limit
RateLimitMiddleware->>Handler: Forward request
Handler-->>Client: 200 OK
end
end
Related issues: Suggested labels: tests, gateway, go Suggested reviewers: jackjin1997 🐰 Middleware guards stand in a row, 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 golangci-lint (2.12.2)level=error msg="[linters_context] typechecking error: pattern ./...: directory prefix . does not contain main module or its selected dependencies" Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (3)
market/gateway/middleware_test.go (3)
156-174: 🩺 Stability & Availability | 🔵 Trivial | 💤 Low valueTiming-based assertions may be flaky under load.
10ms timeout vs. 50ms handler wait vs. 100ms outer wait leaves a fairly tight margin on a loaded CI runner. Consider widening the ratios (e.g., 50ms/250ms/500ms) to reduce flakiness risk without materially slowing the suite.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@market/gateway/middleware_test.go` around lines 156 - 174, The timeout test in TestTimeoutMiddleware_CancelsContext uses very tight timing windows that can flake on slower CI machines. Update the TimeoutMiddleware test to use wider, more forgiving durations for the middleware timeout, the simulated handler wait, and the outer select wait while keeping the same cancellation behavior and references to TimeoutMiddleware, http.HandlerFunc, and the done channel.
12-13: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider removing the bounty attribution comment from test source.
Referencing the bounty/issue link directly in code comments is unusual for committed source; this is typically better placed in the PR description/commit message rather than persisted in the file.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@market/gateway/middleware_test.go` around lines 12 - 13, Remove the bounty attribution note from the test source comment in middleware_test.go and keep the test file comment focused only on the middleware ordering coverage. Update the top-of-file comment near the integration test description so it no longer references the bounty/issue link, while preserving any useful context about auth, rate limiting, and context propagation.
174-174: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueDrop the artificial
contextimport usage.
context.Background()at line 174 exists only to keep the"context"import satisfied; the file otherwise only usesr.Context()method calls, which don't require importing"context"directly. Removing the import removes the need for this workaround line.♻️ Suggested cleanup
import ( - "context" "net/http" "net/http/httptest" "strings" "testing" "time" ) @@ - _ = context.Background() // ensure context import used }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@market/gateway/middleware_test.go` at line 174, Remove the artificial context usage in middleware_test.go by deleting the standalone context.Background() workaround and dropping the direct "context" import, since the test code only relies on r.Context() and does not need the package imported. Check the test setup around the middleware tests and keep only the imports actually referenced by the test helpers and assertions.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@market/gateway/middleware_test.go`:
- Around line 156-174: The timeout test in TestTimeoutMiddleware_CancelsContext
uses very tight timing windows that can flake on slower CI machines. Update the
TimeoutMiddleware test to use wider, more forgiving durations for the middleware
timeout, the simulated handler wait, and the outer select wait while keeping the
same cancellation behavior and references to TimeoutMiddleware,
http.HandlerFunc, and the done channel.
- Around line 12-13: Remove the bounty attribution note from the test source
comment in middleware_test.go and keep the test file comment focused only on the
middleware ordering coverage. Update the top-of-file comment near the
integration test description so it no longer references the bounty/issue link,
while preserving any useful context about auth, rate limiting, and context
propagation.
- Line 174: Remove the artificial context usage in middleware_test.go by
deleting the standalone context.Background() workaround and dropping the direct
"context" import, since the test code only relies on r.Context() and does not
need the package imported. Check the test setup around the middleware tests and
keep only the imports actually referenced by the test helpers and assertions.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: fae328a0-0817-4e03-92d8-8dfc58dfac7d
📒 Files selected for processing (1)
market/gateway/middleware_test.go
1 participant
Fixes #2
Integration tests for gateway middleware (auth, rate limit ordering, CORS, recovery).
Bounty delivery for issue #2.
Summary by CodeRabbit