[test-improver] Improve tests for server http_helpers package#3212
Merged
[test-improver] Improve tests for server http_helpers package#3212
Conversation
These two functions in http_helpers.go were untested: - writeErrorResponse: called by all HTTP error paths in the server - rejectRequest: combines structured error logging with HTTP rejection New tests verify: - Correct HTTP status codes are written - Content-Type is application/json for all responses - Response body is valid JSON with 'error' and 'message' fields - No panics on any valid input combination Co-authored-by: Copilot <[email protected]>
Contributor
There was a problem hiding this comment.
Pull request overview
Improves unit test coverage in internal/server by adding direct, table-driven tests for core HTTP error response helpers to ensure consistent JSON error shapes and headers across server error paths.
Changes:
- Added direct tests for
writeErrorResponsecovering common HTTP status codes and validating JSON shape (error,message), status, andContent-Type. - Added direct tests for
rejectRequestcovering realistic rejection scenarios and validating no panics plus response correctness. - Added
encoding/jsonimport to support structured JSON decoding assertions.
Show a summary per file
| File | Description |
|---|---|
| internal/server/http_helpers_test.go | Adds new unit tests for writeErrorResponse and rejectRequest and supporting JSON decoding import. |
Copilot's findings
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Files reviewed: 1/1 changed files
- Comments generated: 0
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.
Test Improvements:
internal/server/http_helpers_test.goFile Analyzed
internal/server/http_helpers_test.gointernal/serverImprovements Made
1. Increased Coverage — Direct Tests for
writeErrorResponseProblem:
writeErrorResponseinhttp_helpers.gois the canonical helper called by every HTTP error path in the server package (auth rejections, shutdown rejections, etc.). Despite this central role, it had no dedicated unit test — it was only exercised indirectly via higher-level tests such asTestWrapWithMiddleware.New test:
TestWriteErrorResponse— 6 subtests covering all common HTTP error status codes:bad_requesterror code and messageunauthorizederror codeforbiddenerror codenot_founderror codeinternal_errorerror codeservice_unavailableerror codeEach subtest asserts:
Content-Type: application/jsonheader is set"error"field equals thecodeargument"message"field equals themessageargument2. Increased Coverage — Direct Tests for
rejectRequestProblem:
rejectRequestcomposeslogRuntimeError+writeErrorResponseinto a single call used at every request-rejection site. It had no direct test — its behavior was only observed through integration-style middleware tests.New test:
TestRejectRequest— 3 subtests covering realistic rejection scenarios:Each subtest asserts:
Content-Type: application/jsonheader is set"error"and"message"fields3. Cleaner & More Stable Tests
"encoding/json"import (only addition to the import block)require.NoErrorfor JSON decode (stops test on body parse failure before further assertions)assert.Equalfor all field-level checks (clear diff output on failure)httptest.NewRecorder()andhttptest.NewRequest()— no shared stateWhy These Changes?
writeErrorResponseis the single source of truth for the JSON shape of all HTTP error responses across the server package. A regression there (e.g., wrong field name, wrong content-type) would silently break clients. Despite being simple, it had no direct test. Similarly,rejectRequest— which drives auth and shutdown rejection — was only covered as a side-effect of middleware tests, making failures harder to pinpoint.These two tests are small, fast, and deterministic: they exercise only in-process HTTP plumbing with no external dependencies.
Generated by Test Improver Workflow
Focuses on better patterns, increased coverage, and more stable tests