[test][WIP] Unit test for OTLP receiver in jaeger-v1 #6541
Conversation
Signed-off-by: chahatsagarmain <[email protected]>
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #6541 +/- ##
==========================================
- Coverage 96.25% 96.22% -0.04%
==========================================
Files 372 373 +1
Lines 21371 21406 +35
==========================================
+ Hits 20570 20597 +27
- Misses 610 616 +6
- Partials 191 193 +2
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: chahatsagarmain <[email protected]>
|
dependent PR was merged |
Signed-off-by: chahatsagarmain <[email protected]>
Signed-off-by: chahatsagarmain <[email protected]>
|
|
||
| if equal { | ||
| rSpan := td.ResourceSpans().At(0) | ||
| equal = rSpan.ScopeSpans().Len() == 1 && |
There was a problem hiding this comment.
you don't need so complicated checks, it's ok in the test to just assume that you can directly navigate to the span (and let it panic if not)
| rSpan.ScopeSpans().At(0).Spans().At(0).SpanID() == byteId | ||
| } | ||
| if !equal { | ||
| t.Error("Traces don't match expected structure") |
There was a problem hiding this comment.
this is not a common pattern to perform validation inside of the mock implementation. You can simply store the passed value in an outside variable and then perform assertion in the main body of the test.
| ) | ||
| require.NoError(t, err) | ||
| defer spanProcessor.Close() | ||
| logger, _ := testutils.NewLogger() |
There was a problem hiding this comment.
if you don't need to inspect logger results (you're ignoring the buf return value), use zaptest.NewLogger(t) or zap.NewNop() instead
Signed-off-by: chahatsagarmain <[email protected]>
| var receivedTraces ptrace.Traces | ||
| var mu sync.Mutex |
There was a problem hiding this comment.
you can use atomic.Pointer instead of two vars
yurishkuro
added
the
changelog:test
Which problem is this PR solving?
Description of the changes
How was this change tested?
Checklist
jaeger:make lint testjaeger-ui:npm run lintandnpm run test