Skip to content

test(middleware/tracing): add assertions for ClientHandler statshandler tests#3812

Open
sevico wants to merge 1 commit into
go-kratos:mainfrom
sevico:test-tracing-handler
Open

test(middleware/tracing): add assertions for ClientHandler statshandler tests#3812
sevico wants to merge 1 commit into
go-kratos:mainfrom
sevico:test-tracing-handler

Conversation

@sevico
Copy link
Copy Markdown

@sevico sevico commented Apr 5, 2026

Summary

  • TestClient_HandleConn and TestClient_HandleRPC used _ *testing.T, discarding the test parameter, making it impossible to assert any behavior.
  • Refactored TestClient_HandleRPC into subtests covering all branches: non-OutHeader ignored, missing peer early return, invalid span context skipped, and valid span with peer correctly setting net.peer.ip and net.peer.port attributes.
  • Replaced the unused mockSpan with a recordingSpan that captures SetAttributes calls for actual verification.

Test plan

  • go test ./middleware/tracing/ -v -run "TestClient" passes
  • All existing tests in the package remain green

…er tests

TestClient_HandleConn and TestClient_HandleRPC used `_ *testing.T`,
discarding the test parameter entirely, which made it impossible to
add any assertions. The tests only exercised code paths without
verifying behavior.

Refactor TestClient_HandleRPC into subtests covering all branches:
- non-OutHeader stats are ignored
- missing peer context causes early return
- invalid span context skips attribute setting
- valid span with peer correctly sets net.peer.ip and net.peer.port

Replace the unused mockSpan with a recordingSpan that captures
SetAttributes calls, enabling actual verification of the tracing
instrumentation logic.
@dosubot dosubot Bot added the size:L This PR changes 100-499 lines, ignoring generated files. label Apr 5, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:L This PR changes 100-499 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant