-
Notifications
You must be signed in to change notification settings - Fork 2.5k
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
Add test for storage-exporter #5078
Add test for storage-exporter #5078
Conversation
d249735
to
a74ce6e
Compare
The config is set now. However, I am unable to find a way to properly set the Component Host. |
Did you check how exporters are tested in opentelemetry-collector? We should use the same patterns. |
Signed-off-by: Chirag Ghosh <[email protected]>
Signed-off-by: Chirag Ghosh <[email protected]>
Signed-off-by: Chirag Ghosh <[email protected]>
Signed-off-by: Chirag Ghosh <[email protected]>
1908f83
to
4195c27
Compare
The exporters in opentelemetry-collector were using the noop component host from componenttest package. Our exporter however required a host with custom configs. I have tried to do that. Please review. |
Signed-off-by: Chirag Ghosh <[email protected]>
Signed-off-by: Chirag Ghosh <[email protected]>
Signed-off-by: Chirag Ghosh <[email protected]>
Signed-off-by: Chirag Ghosh <[email protected]>
Signed-off-by: Chirag Ghosh <[email protected]>
Signed-off-by: Chirag Ghosh <[email protected]>
Signed-off-by: Chirag Ghosh <[email protected]>
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #5078 +/- ##
==========================================
- Coverage 95.59% 95.55% -0.04%
==========================================
Files 316 319 +3
Lines 18322 18376 +54
==========================================
+ Hits 17515 17560 +45
- Misses 648 654 +6
- Partials 159 162 +3
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
Signed-off-by: Chirag Ghosh <[email protected]>
Looks almost ready. See some comments above, and also there are a few error returns that are not covered by tests, can you add them? They look fairly easy to simulate (except |
Signed-off-by: Chirag Ghosh <[email protected]>
Do you mean like intentionally introducing errors and then test that errors are indeed thrown? |
Signed-off-by: Chirag Ghosh <[email protected]>
Signed-off-by: Chirag Ghosh <[email protected]>
Signed-off-by: Chirag Ghosh <[email protected]>
Signed-off-by: Yuri Shkuro <[email protected]>
Signed-off-by: Yuri Shkuro <[email protected]>
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.
Thanks!
Which problem is this PR solving?
Description of the changes
How was this change tested?
Checklist
jaeger
:make lint test
jaeger-ui
:yarn lint
andyarn test