Skip to content

Commit 3050fcf

Browse files
authored
[test-improver] Improve tests for tracing package (#5033)
# Test Improvements: `internal/tracing/http_test.go` ## File Analyzed - **New Test File**: `internal/tracing/http_test.go` - **Package**: `internal/tracing` - **Implementation**: `internal/tracing/http.go` — `statusResponseWriter` wrapper type ## Improvements Made ### 1. New Test Coverage for `statusResponseWriter` The `statusResponseWriter` type (used inside `WrapHTTPHandler`) had no dedicated unit tests: | Method | Before | After | |--------|--------|-------| | `Write` | 66.7% | 100% | | `Unwrap` | **0.0%** | **100%** | | `WriteHeader` | 100% | 100% | **Package total: 95.1% → 96.7%** ### 2. Tests Added - ✅ `TestStatusResponseWriter_WriteHeader` — verifies status code is captured and forwarded - ✅ `TestStatusResponseWriter_Write_SetsImplicit200` — verifies implicit 200 set when `Write` called without prior `WriteHeader` - ✅ `TestStatusResponseWriter_Write_PreservesExplicitStatus` — verifies an already-set status isn't overwritten by `Write` - ✅ `TestStatusResponseWriter_Unwrap_ReturnsUnderlying` — verifies `Unwrap` returns the wrapped `ResponseWriter` - ✅ `TestStatusResponseWriter_Unwrap_ExposesOptionalInterfaces` — verifies `http.ResponseController` can discover `http.Flusher` through `Unwrap` (the primary reason `Unwrap` exists) ### 3. Idiomatic testify usage Uses `require.NoError` for fatal checks and `assert.Equal`/`assert.True` for non-fatal assertions, consistent with the rest of the codebase. ## Test Execution ``` --- PASS: TestStatusResponseWriter_WriteHeader (0.00s) --- PASS: TestStatusResponseWriter_Write_SetsImplicit200 (0.00s) --- PASS: TestStatusResponseWriter_Write_PreservesExplicitStatus (0.00s) --- PASS: TestStatusResponseWriter_Unwrap_ReturnsUnderlying (0.00s) --- PASS: TestStatusResponseWriter_Unwrap_ExposesOptionalInterfaces (0.00s) ok github.com/github/gh-aw-mcpg/internal/tracing 0.018s coverage: 96.7% of statements ``` ## Why This File? `statusResponseWriter.Unwrap()` existed specifically to allow `http.ResponseController` to detect optional interfaces on the underlying writer (e.g. `http.Flusher`), but had 0% coverage — the most critical path was completely untested. The `Write` method also had a branch (implicit-200 assignment) that wasn't exercised. These are small, focused tests that meaningfully verify the contract of the type. --- *Generated by Test Improver Workflow* *Focuses on better patterns, increased coverage, and more stable tests* > [!WARNING] > **⚠️ Firewall blocked 1 domain** > > The following domain was blocked by the firewall during workflow execution: > > - `invalidhostthatdoesnotexist12345.com` > > To allow these domains, add them to the `network.allowed` list in your workflow frontmatter: > > ```yaml > network: > allowed: > - defaults > - "invalidhostthatdoesnotexist12345.com" > ``` > > See [Network Configuration](https://github.github.com/gh-aw/reference/network/) for more information. > Generated by [Test Improver](https://github.com/github/gh-aw-mcpg/actions/runs/25277977858/agentic_workflow) · ● 863.1K · [◷](https://github.com/search?q=repo%3Agithub%2Fgh-aw-mcpg+%22gh-aw-workflow-id%3A+test-improver%22&type=pullrequests) <!-- gh-aw-agentic-workflow: Test Improver, engine: copilot, version: 1.0.21, model: auto, id: 25277977858, workflow_id: test-improver, run: https://github.com/github/gh-aw-mcpg/actions/runs/25277977858 --> <!-- gh-aw-workflow-id: test-improver -->
2 parents 41ed4be + d47d0ec commit 3050fcf

1 file changed

Lines changed: 73 additions & 0 deletions

File tree

internal/tracing/http_test.go

Lines changed: 73 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,73 @@
1+
package tracing
2+
3+
import (
4+
"net/http"
5+
"net/http/httptest"
6+
"testing"
7+
8+
"github.com/stretchr/testify/assert"
9+
"github.com/stretchr/testify/require"
10+
)
11+
12+
// mockFlusher implements http.ResponseWriter and http.Flusher so we can verify
13+
// that Unwrap gives callers transparent access to the underlying writer's optional
14+
// interfaces.
15+
type mockFlusher struct {
16+
httptest.ResponseRecorder
17+
flushed bool
18+
}
19+
20+
func (m *mockFlusher) Flush() { m.flushed = true }
21+
22+
func TestStatusResponseWriter_WriteHeader(t *testing.T) {
23+
rec := httptest.NewRecorder()
24+
srw := &statusResponseWriter{ResponseWriter: rec}
25+
26+
srw.WriteHeader(http.StatusCreated)
27+
28+
assert.Equal(t, http.StatusCreated, srw.statusCode)
29+
assert.Equal(t, http.StatusCreated, rec.Code)
30+
}
31+
32+
func TestStatusResponseWriter_Write_SetsImplicit200(t *testing.T) {
33+
rec := httptest.NewRecorder()
34+
srw := &statusResponseWriter{ResponseWriter: rec}
35+
36+
// statusCode starts at zero – Write should set it to 200 implicitly.
37+
n, err := srw.Write([]byte("hello"))
38+
require.NoError(t, err)
39+
assert.Equal(t, 5, n)
40+
assert.Equal(t, http.StatusOK, srw.statusCode)
41+
}
42+
43+
func TestStatusResponseWriter_Write_PreservesExplicitStatus(t *testing.T) {
44+
rec := httptest.NewRecorder()
45+
srw := &statusResponseWriter{ResponseWriter: rec}
46+
47+
srw.WriteHeader(http.StatusAccepted)
48+
49+
_, err := srw.Write([]byte("body"))
50+
require.NoError(t, err)
51+
// statusCode was already set via WriteHeader; Write must not overwrite it.
52+
assert.Equal(t, http.StatusAccepted, srw.statusCode)
53+
assert.Equal(t, http.StatusAccepted, rec.Code)
54+
}
55+
56+
func TestStatusResponseWriter_Unwrap_ReturnsUnderlying(t *testing.T) {
57+
rec := httptest.NewRecorder()
58+
srw := &statusResponseWriter{ResponseWriter: rec}
59+
60+
underlying := srw.Unwrap()
61+
assert.Same(t, rec, underlying, "Unwrap should return the wrapped ResponseWriter")
62+
}
63+
64+
func TestStatusResponseWriter_Unwrap_ExposesOptionalInterfaces(t *testing.T) {
65+
mf := &mockFlusher{ResponseRecorder: *httptest.NewRecorder()}
66+
srw := &statusResponseWriter{ResponseWriter: mf}
67+
68+
// http.ResponseController uses Unwrap to discover optional interfaces like Flusher.
69+
rc := http.NewResponseController(srw)
70+
err := rc.Flush()
71+
require.NoError(t, err, "Flush via ResponseController should succeed when underlying writer is a Flusher")
72+
assert.True(t, mf.flushed, "underlying Flusher should have been called through Unwrap")
73+
}

0 commit comments

Comments
 (0)