-
Notifications
You must be signed in to change notification settings - Fork 829
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Move autoconfigure otlp tests #5060
Move autoconfigure otlp tests #5060
Conversation
Codecov ReportBase: 91.19% // Head: 91.06% // Decreases project coverage by
Additional details and impacted files@@ Coverage Diff @@
## main #5060 +/- ##
============================================
- Coverage 91.19% 91.06% -0.14%
- Complexity 4877 4887 +10
============================================
Files 553 553
Lines 14402 14460 +58
Branches 1375 1380 +5
============================================
+ Hits 13134 13168 +34
- Misses 879 898 +19
- Partials 389 394 +5
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report at Codecov. |
hmm. This all looks good, but it looks like it reduced coverage a bit. Any idea how difficult it would be to backfill those gaps? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not happy with the slightly reduced coverage numbers, but otherwise looks fine to me.
I've added some tests to increase the coverage. The coverage should be equivalent now. I think any remaining diff is just one of those quirks with the codecov tool. |
Resolves #4949.
This is likely an unreviewable change. Here's a summary of it and why its hard to break up:
:sdk-extensions:autoconfigure
and into:exporter:otlp
.:sdk-extensions:autoconfigure
provide a decent amount of coverage for theOtlpHttp{Signal}Exporter
s that doesn't otherwise exist. TheOtlpGrpc{Signal}Exporter
s don't have this same problem because they have an abstract test class with a bunch of implementations.AbstractHttpTelemetryExporterTest
and corresponding implementations. Its largely a copy ofAbstractGrpcTelemetryExporterTest
with changes as needed to reflect differences in grpc and http/protobuf.ConfigProperties
and configure the builders correctly. The newOtlp{Signal}ExporterProviderTest
s provide this coverage. I thought about abstracting the common parts away, but concluded it would make the test code too hard to comprehend.This could be reasonably broken up into two PRs, but even those would be pretty large and hard to review carefully.