Skip to content

Commit 4e877f2

Browse files
authored
[test-improver] Improve tests for tracing package (#7740)
# Test Improvements: `provider_internal_test.go` ## File Analyzed - **Test File**: `internal/tracing/provider_internal_test.go` - **Package**: `internal/tracing` - **Lines of Code**: 52 → 117 (+65 lines) ## Improvements Made ### 1. Increased Coverage - ✅ Added `TestGenerateRandomSpanID`: verifies the happy path — produces a valid non-zero 8-byte span ID and that successive calls yield distinct values - ✅ Added `TestGenerateRandomSpanID_Error`: injects a failing `crypto/rand.Reader` to cover the previously dead error-return path in `generateRandomSpanID` - ✅ Added `TestParentContext_RandomSpanIDFailure`: triggers the `genErr != nil` branch in `ParentContext` (lines 322–325 of `provider.go`) by breaking the random source, verifying the original context is returned unchanged | Function | Before | After | |---|---|---| | `generateRandomSpanID` | 83.3% | 100% | | `ParentContext` | 92.6% | 100% | | **Package total** | **95.2%** | **96.2%** | ### 2. Better Testing Patterns - ✅ Error-path injection via `errorReader` struct (same pattern as `auth/header_test.go`) to test unreachable crypto-failure branches - ✅ Tests that must not run in parallel (global `rand.Reader` mutation) are explicitly left non-parallel with a comment explaining why - ✅ `require.NoError` / `require.Error` for blocking assertions, `assert.*` for non-blocking checks — consistent with project conventions - ✅ `t.Parallel()` on the pure happy-path test that has no global side effects ## Test Execution All tests pass: ``` ok github.com/github/gh-aw-mcpg/internal/tracing 0.023s coverage: 96.2% of statements ``` ## Why These Changes? `provider_internal_test.go` was a thin 52-line file that only tested `mergeOTLPHeaders`. The two functions with coverage gaps — `generateRandomSpanID` (83.3%) and `ParentContext` (92.6%) — had unreachable error paths that require `crypto/rand` failure injection. Since the test file is in package `tracing` (not `tracing_test`), it has direct access to the unexported `generateRandomSpanID` and can inject the `rand.Reader` failure needed to exercise both error branches. The improvements follow the exact same `errorReader` / `rand.Reader` swap pattern already established in `internal/auth/header_test.go`. --- *Generated by Test Improver Workflow* *Focuses on better patterns, increased coverage, and more stable tests* > [!WARNING] > <details> > <summary>Firewall blocked 1 domain</summary> > > The following domain was blocked by the firewall during workflow execution: > > - `index.crates.io` >> To allow these domains, add them to the `network.allowed` list in your workflow frontmatter: > > ```yaml > network: > allowed: > - defaults > - "index.crates.io" > ``` > > See [Network Configuration](https://github.github.com/gh-aw/reference/network/) for more information. > > </details> > Generated by [Test Improver](https://github.com/github/gh-aw-mcpg/actions/runs/27797269536) · 795.5 AIC · ⊞ 29.5K · [◷](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.60, model: claude-sonnet-4.6, id: 27797269536, workflow_id: test-improver, run: https://github.com/github/gh-aw-mcpg/actions/runs/27797269536 --> <!-- gh-aw-workflow-id: test-improver --> <!-- gh-aw-workflow-call-id: github/gh-aw-mcpg/test-improver -->
2 parents a6e023e + e74d088 commit 4e877f2

1 file changed

Lines changed: 65 additions & 0 deletions

File tree

internal/tracing/provider_internal_test.go

Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,16 @@
11
package tracing
22

33
import (
4+
"context"
5+
"crypto/rand"
6+
"errors"
47
"testing"
58

69
"github.com/stretchr/testify/assert"
10+
"github.com/stretchr/testify/require"
11+
"go.opentelemetry.io/otel/trace"
12+
13+
"github.com/github/gh-aw-mcpg/internal/config"
714
)
815

916
func TestMergeOTLPHeaders(t *testing.T) {
@@ -50,3 +57,61 @@ func TestMergeOTLPHeaders(t *testing.T) {
5057
})
5158
}
5259
}
60+
61+
// TestGenerateRandomSpanID verifies that generateRandomSpanID produces a valid,
62+
// non-zero 8-byte span ID and that successive calls produce distinct values.
63+
func TestGenerateRandomSpanID(t *testing.T) {
64+
t.Parallel()
65+
66+
id, err := generateRandomSpanID()
67+
require.NoError(t, err, "generateRandomSpanID should not fail with a healthy rand.Reader")
68+
assert.NotEqual(t, trace.SpanID{}, id, "generated span ID should be non-zero")
69+
70+
id2, err := generateRandomSpanID()
71+
require.NoError(t, err)
72+
assert.NotEqual(t, id, id2, "successive calls should produce distinct span IDs")
73+
}
74+
75+
// errorReader is an io.Reader that always returns the configured error.
76+
type errorReader struct{ err error }
77+
78+
func (r *errorReader) Read(_ []byte) (int, error) { return 0, r.err }
79+
80+
// TestGenerateRandomSpanID_Error verifies that generateRandomSpanID propagates
81+
// errors from the underlying random source and returns a zero span ID.
82+
// Must NOT run in parallel: temporarily replaces the global crypto/rand.Reader.
83+
func TestGenerateRandomSpanID_Error(t *testing.T) {
84+
syntheticErr := errors.New("synthetic entropy failure")
85+
86+
origReader := rand.Reader
87+
rand.Reader = &errorReader{err: syntheticErr}
88+
defer func() { rand.Reader = origReader }()
89+
90+
id, err := generateRandomSpanID()
91+
92+
assert.Equal(t, trace.SpanID{}, id, "span ID should be zero on error")
93+
require.Error(t, err, "should return an error when the random source fails")
94+
assert.ErrorIs(t, err, syntheticErr, "error should wrap the underlying source error")
95+
assert.Contains(t, err.Error(), "failed to generate random span ID")
96+
}
97+
98+
// TestParentContext_RandomSpanIDFailure verifies that ParentContext returns the
99+
// original context unchanged when generateRandomSpanID fails due to a broken
100+
// random source. This covers the genErr != nil branch (lines 322–325 of provider.go).
101+
// Must NOT run in parallel: temporarily replaces the global crypto/rand.Reader.
102+
func TestParentContext_RandomSpanIDFailure(t *testing.T) {
103+
ctx := context.Background()
104+
cfg := &config.TracingConfig{
105+
TraceID: "4bf92f3577b34da6a3ce929d0e0e4736", // valid 32-char hex (non-zero)
106+
// SpanID intentionally absent so ParentContext must generate one
107+
}
108+
109+
origReader := rand.Reader
110+
rand.Reader = &errorReader{err: errors.New("entropy unavailable")}
111+
defer func() { rand.Reader = origReader }()
112+
113+
parentCtx := ParentContext(ctx, cfg)
114+
115+
assert.True(t, ctx == parentCtx,
116+
"ParentContext must return the original context when random span ID generation fails")
117+
}

0 commit comments

Comments
 (0)