Skip to content

[test-improver] Improve tests for tracing package#6716

Merged
lpcox merged 5 commits into
mainfrom
test-improver/tracing-coverage-b59aa1a9a55d3c7c
May 30, 2026
Merged

[test-improver] Improve tests for tracing package#6716
lpcox merged 5 commits into
mainfrom
test-improver/tracing-coverage-b59aa1a9a55d3c7c

Conversation

@github-actions
Copy link
Copy Markdown
Contributor

Test Improvements: tracing package

Files Analyzed

  • Test Files: internal/tracing/parse_headers_test.go, internal/tracing/http_test.go
  • Package: internal/tracing

Improvements Made

1. Increased Coverage

  • ✅ Added TestParseOTLPHeadersWithDecoder_InvalidPercentEncoding — exercises the url.PathUnescape error branch in parseOTLPHeadersWithDecoder (previously uncovered). Verifies the raw value is preserved when percent-decoding fails (e.g. %GZ is not valid percent-encoding).

  • ✅ Added TestParseOTLPHeadersWithDecoder_ValidPercentEncoding — documents and verifies that well-formed percent-encoded values are decoded when decodeValues=true.

  • ✅ Added TestParseOTLPHeadersWithDecoder_NoDecoding — documents and verifies that percent-encoded values are preserved as-is when decodeValues=false.

  • ✅ Added TestWrapHTTPHandler_PatternMethodMismatch_ClearsRoute — exercises the else { route = "" } branch in WrapHTTPHandler (previously uncovered). This path is triggered when r.Pattern contains a method prefix that differs from the actual request method.

  • Previous Coverage: 96.4%

  • New Coverage: 97.6%

  • Improvement: +1.2%

2. Better Testing Patterns

  • ✅ New tests use table-driven style where multiple variants are needed
  • ✅ Tests include descriptive names following Test<Func>_<Scenario> convention
  • ✅ Tests are independent with no side effects

Test Execution

All tests pass:

ok  	github.com/github/gh-aw-mcpg/internal/tracing	0.017s	coverage: 97.6% of statements

All other Go tests continue to pass (make test-all).

Why These Changes?

The internal/tracing package had two meaningful uncovered branches:

  1. parseOTLPHeadersWithDecoder — The url.PathUnescape error path was untested. This is a real defensive code path that protects against invalid percent-encoded OTLP header values arriving via the OTEL_EXPORTER_OTLP_HEADERS environment variable.

  2. WrapHTTPHandler — The route = "" else-branch when the pattern's HTTP method doesn't match the request method was untested. While this rarely occurs in normal mux routing, it is a defensive code path worth covering.


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:

  • index.crates.io

To allow these domains, add them to the network.allowed list in your workflow frontmatter:

network:
  allowed:
    - defaults
    - "index.crates.io"

See Network Configuration for more information.

Generated by Test Improver · sonnet46 4.9M ·

Add missing coverage for:
- parseOTLPHeadersWithDecoder: test invalid percent-encoding error path
  (url.PathUnescape failure → raw value preserved)
- WrapHTTPHandler: test route cleared when pattern method mismatches
  request method (exercises else-branch setting route = "")
- Additional tests for explicit decode/no-decode behaviour in
  parseOTLPHeadersWithDecoder

Coverage: 96.4% → 97.6%

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@lpcox lpcox marked this pull request as ready for review May 30, 2026 13:30
Copilot AI review requested due to automatic review settings May 30, 2026 13:30
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR improves unit test coverage in internal/tracing by adding targeted tests for previously uncovered defensive branches in OTLP header parsing and HTTP handler route extraction.

Changes:

  • Added coverage for parseOTLPHeadersWithDecoder percent-decoding behavior (valid decode, invalid decode fallback, and no-decoding mode).
  • Added coverage for the WrapHTTPHandler branch where r.Pattern contains a mismatched HTTP method prefix (forcing route = "").
Show a summary per file
File Description
internal/tracing/parse_headers_test.go Adds tests for percent-decoding success/failure and decode toggle behavior in OTLP header parsing.
internal/tracing/http_test.go Adds a new test to hit the defensive route = "" branch when the request pattern’s method mismatches the request method.

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: 6

Comment thread internal/tracing/parse_headers_test.go Outdated
Comment thread internal/tracing/http_test.go Outdated
Comment thread internal/tracing/http_test.go Outdated
Comment thread internal/tracing/parse_headers_test.go Outdated
Comment on lines +158 to +161
result := parseOTLPHeadersWithDecoder("X-Token=Bearer%GZvalue,X-Other=ok", true)
// The raw (un-decoded) value must be used when decoding fails.
assert.Equal(t, "Bearer%GZvalue", result["X-Token"], "raw value should be preserved on decode failure")
assert.Equal(t, "ok", result["X-Other"], "valid pairs must still be decoded correctly")
Comment thread internal/tracing/http_test.go Outdated
Comment on lines +66 to +70
// TestWrapHTTPHandler_PatternMethodMismatch_ClearsRoute verifies that when a request
// pattern contains a different HTTP method than the actual request method, the
// http.route attribute is not added to the span.
// This exercises the `route = ""` branch in WrapHTTPHandler.
func TestWrapHTTPHandler_PatternMethodMismatch_ClearsRoute(t *testing.T) {
Comment thread internal/tracing/http_test.go Outdated
Comment on lines +71 to +73
// Build a request whose Pattern method differs from its actual Method.
// In normal mux routing this cannot happen, but direct manipulation lets us
// verify that WrapHTTPHandler handles it gracefully without setting http.route.
lpcox and others added 3 commits May 30, 2026 06:34
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
@lpcox
Copy link
Copy Markdown
Collaborator

lpcox commented May 30, 2026

@copilot address review feedback

Copy link
Copy Markdown
Contributor

Copilot AI commented May 30, 2026

@copilot address review feedback

Addressed in 435c8a8. The tracing HTTP mismatch test now asserts the emitted span omits http.route while still checking the defensive no-panic path, and the branch passes targeted and full repo validation.

Copilot finished work on behalf of lpcox May 30, 2026 13:45
Copilot AI requested a review from lpcox May 30, 2026 13:45
@lpcox lpcox merged commit 4f02042 into main May 30, 2026
16 checks passed
@lpcox lpcox deleted the test-improver/tracing-coverage-b59aa1a9a55d3c7c branch May 30, 2026 13:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants