[test] Add tests for server.peekRequestBody and config.isTransientHTTPError

[github-actions]

Test Coverage Improvement

Functions Analyzed

Function	Package	File	Previous Coverage
peekRequestBody	internal/server	http_helpers.go	0% direct coverage
isTransientHTTPError	internal/config	validation_schema.go	0% direct coverage

Both functions were exercised only indirectly through higher-level tests — peekRequestBody via logHTTPRequestBody tests and isTransientHTTPError via fetchAndFixSchema retry tests — but neither had any direct unit tests.

Why These Functions?

peekRequestBody was selected as the primary target. Despite being a critical helper used by every incoming HTTP request handler, it had no direct unit tests. The function restores the request body after reading it (so downstream handlers can re-read it), which is a correctness property that deserves explicit verification. Its branches span:

• Non-POST method → fast return without touching body
• nil body or http.NoBody → fast return
• Read error during body consumption → propagated error
• Close error after successful read → propagated error
• Empty body → replaced with http.NoBody
• Non-empty body → bytes returned and body reset for re-reading

isTransientHTTPError was added as a complementary target: a small but completely untested function where all 5xx / 429 / 503 branches needed explicit coverage including boundary values (499, 599, 600).

Tests Added

internal/server/peek_request_body_test.go

• ✅ Non-POST methods (GET, PUT, DELETE) return nil, nil
• ✅ POST with nil body returns nil, nil
• ✅ POST with http.NoBody returns nil, nil
• ✅ POST with non-empty body returns bytes and restores the body for re-reading
• ✅ POST with binary (non-text) body — bytes preserved exactly
• ✅ POST with empty body reader returns []byte{} and sets body to http.NoBody
• ✅ POST with read error propagates the error
• ✅ POST with close error propagates the error
• ✅ Body-restoration contract test: downstream io.ReadAll gets full body after peek
• ✅ Empty-body contract test: req.Body is set to http.NoBody exactly

internal/config/transient_http_error_test.go

• ✅ 429 Too Many Requests → true
• ✅ 503 Service Unavailable → true
• ✅ Full 5xx range (500, 501, 502, 504, 599) → true
• ✅ 2xx, 3xx, 4xx (non-429) → false
• ✅ Boundary values: 499 → false, 600 → false

Implementation Notes

• Both test files use table-driven tests with t.Parallel() per sub-test
• Custom io.ReadCloser helpers (errorOnReadReader, errorOnCloseReader) are scoped to the server test file for error injection without external dependencies
• No production code was changed

---

Generated by Test Coverage Improver

Generated by Test Coverage Improver · ● 13.1M · ◷

[Copilot]

Pull request overview

Adds direct unit tests to improve coverage for two previously indirectly-tested helpers: server.peekRequestBody and config.isTransientHTTPError.

Changes:

• Added table-driven unit tests for peekRequestBody, including body restoration and error paths.
• Added table-driven unit tests for isTransientHTTPError, covering 429/503/5xx and boundary values.

Show a summary per file

File	Description
internal/server/peek_request_body_test.go	New unit tests covering peekRequestBody branches and body-restoration contract.
internal/config/transient_http_error_test.go	New unit tests covering isTransientHTTPError status-code classification.

Copilot's findings

Tip

Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

• Files reviewed: 2/2 changed files
• Comments generated: 1

[Copilot]

The "POST with nil body" case doesn’t actually exercise the r.Body == nil branch: httptest.NewRequest(..., nil) sets req.Body to http.NoBody (not nil), so this is effectively duplicating the explicit http.NoBody test. To cover the intended branch, explicitly set req.Body = nil after constructing the request (or rename/remove this case).

[lpcox]

@copilot apply changes based on the comments in this thread

[Copilot]

@copilot apply changes based on the comments in this thread

Fixed in 9f29e6a. The "POST with nil body" test case now explicitly sets req.Body = nil after creating the request, so it correctly exercises the r.Body == nil branch rather than duplicating the http.NoBody case.

[rdevps]

ThreatLenses — AI PR review

Task (understood)
Add unit tests for the functions server.peekRequestBody and config.isTransientHTTPError to improve test coverage.

Summary
The pull request successfully adds comprehensive unit tests for both the peekRequestBody and isTransientHTTPError functions, addressing previously uncovered branches. The tests are well-structured using table-driven tests and include edge cases, which is commendable. However, the review is based on a large diff, and while the tests appear thorough, it's essential to ensure that they are executed in the CI pipeline to validate their effectiveness. Overall, this is a positive contribution to the codebase.