Skip to content

[test-improver] Improve tests for tracing span helpers#6651

Merged
lpcox merged 2 commits into
mainfrom
test-improver/span-helpers-coverage-014ddaed2c3bfd33
May 29, 2026
Merged

[test-improver] Improve tests for tracing span helpers#6651
lpcox merged 2 commits into
mainfrom
test-improver/span-helpers-coverage-014ddaed2c3bfd33

Conversation

@github-actions

Copy link
Copy Markdown
Contributor

Test Improvements: span_helpers_test.go

File Analyzed

  • Test File: internal/tracing/span_helpers_test.go (new file)
  • Package: internal/tracing
  • Implementation: internal/tracing/span_helpers.go

Improvements Made

1. Increased Coverage

  • ✅ Added TestRecordSpanError_SetsStatusAndRecordsEvent — verifies RecordSpanError sets span status to Error, records the correct status description, and creates an exception event with a stack trace

  • ✅ Added TestRecordSpanErrorOnAll_RecordsOnAllSpans — verifies RecordSpanErrorOnAll propagates the error to every span in the variadic list

  • ✅ Added TestRecordSpanErrorOnAll_NoSpans_DoesNotPanic — edge case: calling with no spans must not panic

  • ✅ Added TestRecordSpanErrorOnAll_SingleSpan_BehavesLikeRecordSpanError — single-span call produces the same result as calling RecordSpanError directly

  • Previous Coverage (RecordSpanErrorOnAll): 0.0%

  • New Coverage (RecordSpanErrorOnAll): 100.0%

  • Package Coverage: 95.1% → 96.3% (+1.2%)

2. Better Testing Patterns

  • ✅ Extracted newRecordingSpan test helper with t.Helper() and t.Cleanup() for SDK tracer teardown — keeps individual tests concise and avoids resource leaks
  • ✅ Uses tracetest.InMemoryExporter + sdktrace.NewSimpleSpanProcessor to record real span events without network or external dependencies
  • ✅ Uses require for critical setup assertions (span type assertion, slice lengths) and assert for non-fatal checks
  • ✅ Descriptive TestFunctionName_Scenario naming convention

3. Cleaner & More Stable Tests

  • t.Cleanup shuts down the SDK tracer provider after each test — no global state leak
  • ✅ No timing dependencies; all assertions are on synchronously collected span stubs
  • ✅ Each test is fully independent

Test Execution

=== RUN   TestRecordSpanError_SetsStatusAndRecordsEvent
--- PASS: TestRecordSpanError_SetsStatusAndRecordsEvent (0.00s)
=== RUN   TestRecordSpanErrorOnAll_RecordsOnAllSpans
--- PASS: TestRecordSpanErrorOnAll_RecordsOnAllSpans (0.00s)
=== RUN   TestRecordSpanErrorOnAll_NoSpans_DoesNotPanic
--- PASS: TestRecordSpanErrorOnAll_NoSpans_DoesNotPanic (0.00s)
=== RUN   TestRecordSpanErrorOnAll_SingleSpan_BehavesLikeRecordSpanError
--- PASS: TestRecordSpanErrorOnAll_SingleSpan_BehavesLikeRecordSpanError (0.00s)
PASS
ok  github.com/github/gh-aw-mcpg/internal/tracing  0.017s  coverage: 96.3% of statements

Why These Changes?

RecordSpanErrorOnAll had 0% coverage despite being a public utility used across the codebase (e.g. in unified.go for tool-call error paths). The function is straightforward but important — a bug here would silently fail to mark spans as errors, hiding failures in distributed traces. The new tests confirm correct behavior on zero, one, and multiple spans, and validate that the underlying RecordSpanError path (status code, description, exception event) works as expected.


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 1.8M ·

Cover RecordSpanError and RecordSpanErrorOnAll which had 0% and 100%
coverage respectively, improving package coverage from 95.1% to 96.3%.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@lpcox lpcox marked this pull request as ready for review May 29, 2026 11:48
Copilot AI review requested due to automatic review settings May 29, 2026 11:48

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

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 adds focused unit tests for tracing span error helper behavior in internal/tracing, improving coverage around status/error propagation for OpenTelemetry spans.

Changes:

  • Adds an in-memory span test helper for recording span output.
  • Tests RecordSpanError status/event behavior.
  • Tests RecordSpanErrorOnAll for zero, one, and multiple spans.
Show a summary per file
File Description
internal/tracing/span_helpers_test.go Adds new tests for span error recording helper functions.

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 thread internal/tracing/span_helpers_test.go Outdated
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
@lpcox lpcox merged commit 6039db5 into main May 29, 2026
16 checks passed
@lpcox lpcox deleted the test-improver/span-helpers-coverage-014ddaed2c3bfd33 branch May 29, 2026 11:58
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