Migrate dd-trace-core groovy files to java part 9#11488
Conversation
|
Hi! 👋 Thanks for your pull request! 🎉 To help us review it, please make sure to:
If you need help, please check our contributing guidelines. |
|
bric3
left a comment
There was a problem hiding this comment.
Conversion-specific Mockito comments.
| singletonMap("k", "v")); | ||
| Map<String, String> carrier = mock(Map.class); | ||
|
|
||
| mockedContext.beginEndToEnd(); |
There was a problem hiding this comment.
The Groovy test verified both time-source calls before 0 * _, but this conversion only checks the carrier. Suggested fix: clear the stubbing/build noise before beginEndToEnd(), then verify the two time-source calls too.
clearInvocations(timeSource);
mockedContext.beginEndToEnd();
injector.inject(mockedContext, carrier, MapSetter.INSTANCE);
verify(timeSource).getCurrentTimeNanos();
verify(timeSource).getNanoTicks();
verify(carrier)
.put(
"X-Amzn-Trace-Id",
"Root=1-633c7675-000000000000000000000001;Parent=0000000000000002;_dd.origin=fakeOrigin;t0=1664906869195;k=v");
verifyNoMoreInteractions(timeSource, carrier);That keeps the Mockito version as strict as the original Spock interaction block.
|
|
||
| propagator.inject(span, carrier, setter); | ||
|
|
||
| verify(injector).inject(any(DDSpanContext.class), same(carrier), any()); |
There was a problem hiding this comment.
The Spock assertion matched span.context() specifically. Suggested fix: keep that exact-context check in the Mockito verify instead of accepting any DDSpanContext.
verify(injector).inject(same((DDSpanContext) span.context()), same(carrier), any());same is already statically imported here, so this keeps the conversion tight without adding much noise.
|
|
||
| propagator.inject(span, carrier, setter); | ||
|
|
||
| verify(injector, times(injected)).inject(any(DDSpanContext.class), same(carrier), any()); |
There was a problem hiding this comment.
Same conversion concern here: the original interaction was tied to span.context(). Suggested fix: keep the times(injected) behavior, but match the same context instance.
verify(injector, times(injected))
.inject(same((DDSpanContext) span.context()), same(carrier), any());That preserves the old assertion while keeping the converted table-test flow.
we migrate 20 tests from the propagation package: - TagKeyTest - TagValueTest - B3HttpExtractorTest - B3HttpInjectorPaddedTest - B3HttpInjectorTest - DatadogHttpExtractorTest - DatadogHttpInjectorTest - DatadogPropagationTagsTest - HaystackHttpExtractorTest - HaystackHttpInjectorTest - HttpExtractorTest - HttpInjectorB3PaddingTest - HttpInjectorTest - NoneHttpExtractorTest - TracingPropagatorTest - W3CHttpExtractorTest - W3CHttpInjectorTest - W3CPropagationTagsTest - XRayHttpExtractorTest - XRayHttpInjectorTest
952a80e to
588ea9b
Compare
🟢 Java Benchmark SLOs — All performance SLOs passed
PR vs. master resultsStartup Time
Commit: Load and DaCapo benchmarks can be triggered manually in the GitLab pipeline. Results will appear in the Benchmarking Platform UI after completion. |
What Does This Do
we migrate 20 tests from the propagation package:
Motivation
this is part of the effort to migrate groovy tests to Java/JUnit
part1: #11053
part2: #11062
part3: #11085
part4: #11146
part5: #11217
part6: #11362
part7: #11374
part8: #11437
Additional Notes
Contributor Checklist
type:and (comp:orinst:) labels in addition to any other useful labelsclose,fix, or any linking keywords when referencing an issueUse
solvesinstead, and assign the PR milestone to the issue/merge. You can also:/merge --commit-message "..."/merge -c/merge -f --reason "reason"; please use this judiciously, as some checks do not run at the PR-levelJira ticket: [PROJ-IDENT]