[Infra] Refactor EventSource tests#6896
[Infra] Refactor EventSource tests#6896rajkumar-rangaraj merged 5 commits intoopen-telemetry:mainfrom
Conversation
Refactor `EventSource` validation to improve assertions. Complements open-telemetry/opentelemetry-dotnet-contrib#3864.
Fix code analysis warnings.
- Add attribution. - Copy fixes from open-telemetry/opentelemetry-dotnet#6896.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #6896 +/- ##
==========================================
- Coverage 87.30% 87.23% -0.08%
==========================================
Files 263 263
Lines 12387 12387
==========================================
- Hits 10815 10806 -9
- Misses 1572 1581 +9
Flags with carried forward coverage won't be shown. Click here to find out more. |
There was a problem hiding this comment.
Pull request overview
This PR refactors the EventSource test infrastructure to improve error reporting and validation. It replaces the old event listener-based testing approach with a more robust validation method using EventSource.GenerateManifest with strict mode, which performs IL-level inspection to verify that WriteEvent calls match their [Event] attributes. The refactoring also adds explicit duplicate event ID detection.
Changes:
- Replaced the old
MethodsAreImplementedConsistentlyWithTheirAttributesmethod with a newValidateEventSourceIdsmethod that provides clearer error messages - Added comprehensive tests for the helper class itself to validate various error scenarios
- Updated all EventSource test files across multiple projects to use the new validation approach and corrected test method naming (EventSourceTest → EventSourceTests)
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| test/OpenTelemetry.Tests/Shared/TestEventListener.cs | Added CA1812 warning suppression for unused internal class |
| test/OpenTelemetry.Tests/Shared/EventSourceTestHelper.cs | Complete refactor: replaced old validation logic with GenerateManifest-based approach and duplicate ID detection |
| test/OpenTelemetry.Tests/EventSourceTests.cs | Updated to use new ValidateEventSourceIds method and fixed test method name |
| test/OpenTelemetry.Tests/EventSourceTestHelperTests.cs | New file: comprehensive tests for the EventSourceTestHelper validation logic |
| test/OpenTelemetry.Extensions.Propagators.Tests/EventSourceTests.cs | Updated to use new ValidateEventSourceIds method and fixed test method name |
| test/OpenTelemetry.Extensions.Hosting.Tests/EventSourceTests.cs | Updated to use new ValidateEventSourceIds method and fixed test method name |
| test/OpenTelemetry.Exporter.Zipkin.Tests/EventSourceTests.cs | Updated to use new ValidateEventSourceIds method and fixed test method name |
| test/OpenTelemetry.Exporter.Prometheus.HttpListener.Tests/EventSourceTests.cs | Updated to use new ValidateEventSourceIds method and fixed test method name |
| test/OpenTelemetry.Exporter.OpenTelemetryProtocol.Tests/EventSourceTests.cs | Updated to use new ValidateEventSourceIds method and fixed test method names (3 tests) |
| test/OpenTelemetry.Api.Tests/EventSourceTests.cs | Updated to use new ValidateEventSourceIds method and fixed test method name |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Remove unused code. - Fix namespace. - Fix spacing.
Remove unused `using`.
cijothomas
left a comment
There was a problem hiding this comment.
nice!
At some point in future, we should be able to weaver-generate these code from a schema.
Complements open-telemetry/opentelemetry-dotnet-contrib#3864.
Changes
Refactor
EventSourcevalidation to improve assertions.Example assertion failure before:
Example assertion failure after:
Merge requirement checklist
AppropriateCHANGELOG.mdfiles updated for non-trivial changesChanges in public API reviewed (if applicable)