Skip to content

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

Merged
lpcox merged 1 commit intomainfrom
test-improver/tracing-provider-coverage-1132a3fbdcdf5a6f
Apr 16, 2026
Merged

[test-improver] Improve tests for tracing package#3938
lpcox merged 1 commit intomainfrom
test-improver/tracing-provider-coverage-1132a3fbdcdf5a6f

Conversation

@github-actions
Copy link
Copy Markdown
Contributor

File Analyzed

  • Test File: internal/tracing/provider_test.go
  • Package: internal/tracing
  • Lines of Code: 500 → 602

Improvements Made

1. Increased Coverage

  • ✅ Added TestInitProvider_InvalidSampleRate — exercises the warning/fallback path in resolveSampleRate when the configured rate is outside [0.0, 1.0] (e.g., -0.5 or 1.5); verifies the default AlwaysSample is used

  • ✅ Added TestParentContext_InvalidSpanID — covers the resolveParentContext fallback to a random span ID when spanId fails hex decode or has wrong byte length; uses three sub-cases (non-hex, too-short, too-long)

  • ✅ Added TestParentContext_AllZerosTraceID — covers the !sc.IsValid() guard that returns the original context when traceId decodes to all-zero bytes (invalid per W3C Trace Context spec)

  • Previous Coverage: 91.7%

  • New Coverage: 96.3%

  • Improvement: +4.6%

Function Before After
resolveSampleRate 60.0% 100.0%
resolveParentContext 81.5% 92.6%
generateRandomSpanID 75.0% 75.0% (error path requires mocking)

2. Better Testing Patterns

  • ✅ Table-driven sub-tests for TestInitProvider_InvalidSampleRate and TestParentContext_InvalidSpanID, consistent with existing test style
  • ✅ Clear assertions documenting the fallback behavior (e.g., "span should be sampled when rate falls back to default 1.0")
  • ✅ W3C traceparent round-trip check in TestParentContext_InvalidSpanID to confirm the enriched context carries the correct traceId

Test Execution

All tests in the tracing package pass:

ok  github.com/github/gh-aw-mcpg/internal/tracing  0.013s  coverage: 96.3%

Note: TestFetchAndFixSchema_NetworkError in internal/config fails on the baseline (pre-existing, unrelated to this PR).

Why These Changes?

The tracing package had three functions with meaningful untested branches. resolveSampleRate at 60% left the warning/default path completely uncovered, meaning misconfigured sample rates (outside [0.0, 1.0]) were not validated by tests. resolveParentContext at 81.5% missed the span ID fallback logic that silently generates a random span ID — an important spec behaviour (T-OTEL-008). The all-zeros TraceID path guarded by !sc.IsValid() was also untested. All three gaps are now covered with clear, focused test cases.


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:

network:
  allowed:
    - defaults
    - "invalidhostthatdoesnotexist12345.com"

See Network Configuration for more information.

Generated by Test Improver · ● 3.3M ·

Add tests for three previously uncovered paths in the tracing package:

- TestInitProvider_InvalidSampleRate: exercises the warning/fallback
  path in resolveSampleRate when the configured rate is outside [0.0, 1.0]
- TestParentContext_InvalidSpanID: covers the resolveParentContext fallback
  to a random span ID when spanId fails hex decode or has wrong length
  returns the original context when the traceId is all zeros

Coverage improves from 91.7% to 96.3% for the tracing package.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@lpcox lpcox marked this pull request as ready for review April 16, 2026 14:26
Copilot AI review requested due to automatic review settings April 16, 2026 14:26
@lpcox lpcox merged commit c91f435 into main Apr 16, 2026
3 checks passed
@lpcox lpcox deleted the test-improver/tracing-provider-coverage-1132a3fbdcdf5a6f branch April 16, 2026 14:26
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

Improves unit test coverage for the internal/tracing provider initialization and W3C parent context resolution paths, focusing on previously untested invalid configuration inputs.

Changes:

  • Add table-driven coverage for invalid tracing SampleRate values to exercise the fallback-to-default path.
  • Add coverage for invalid spanId inputs to confirm ParentContext falls back to generating a random span ID while preserving the configured traceId.
  • Add coverage for the all-zero traceId validity guard to ensure ParentContext returns the original context unchanged.
Show a summary per file
File Description
internal/tracing/provider_test.go Adds new test cases for invalid sample rates and additional ParentContext fallback/guard branches.

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

Comment on lines +544 to +545
// generating a random span ID when the configured spanId is not valid hex, and still
// returns an enriched context (per spec T-OTEL-008).
Copy link

Copilot AI Apr 16, 2026

Choose a reason for hiding this comment

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

The doc comment says the random span ID path is only when spanId is "not valid hex", but this test also covers valid-hex values with invalid byte length (too short/too long). Update the comment to reflect both decode errors and wrong-length inputs so the test intent stays accurate.

Suggested change
// generating a random span ID when the configured spanId is not valid hex, and still
// returns an enriched context (per spec T-OTEL-008).
// generating a random span ID when the configured spanId is not valid hex or is valid
// hex with the wrong byte length, and still returns an enriched context (per spec
// T-OTEL-008).

Copilot uses AI. Check for mistakes.
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.

2 participants