Port TraceMapperV1 payload test from Spock to JUnit 5 - #12124
Conversation
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
AlexeyKuznetsov-DD
added
tag: ai generated
This comment has been minimized.
This comment has been minimized.
🟢 Java Benchmark SLOs — All performance SLOs passed
PR vs. master results
Commit: Load and DaCapo benchmarks can be triggered manually in the GitLab pipeline. Results will appear in the Benchmarking Platform UI after completion. |
![datadog-prod-us1-6[bot]](https://avatars.githubusercontent.com/u/365230?s=60&v=4){kind=small}
There was a problem hiding this comment.
More details
The Java migration preserves all original Spock scenarios and the shared reader changes only package visibility for test constants; no diff-only behavioral regression was identified. The targeted test could not execute because the checkout requires a Java 25 toolchain, while this sandbox provides JDK 8/11/17/21.
🤖 Datadog Autotest · Commit 6074405 · What is Autotest? · @DataDog review to ask questions · Any feedback? Reach out in #autotest
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 607440569b
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
verifySpan asserted a 16-entry span map but not that each field id appeared exactly once, so a repeated id plus an omitted one left the omitted field's decoded value at its initialiser. For parentId (0) and error (false) those sentinels equal the expected values, and TraceGenerator always uses DDSpanId.ZERO as the parent id, so the parent-id check was inert. Assert the decoded field-id set instead, which covers all 16 fields and names the missing id on failure. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
/merge |
|
View all feedbacks in Devflow UI.
The expected merge time in
|
ff733f5
into
master
3 participants
What Does This Do
Replaces
TraceMapperV1PayloadTest.groovywith a JUnit 5 / Java equivalent. All 25 Spock test instances are preserved; the count goes to 31 becausetest span kind value conversion(oneexpectblock with 7 assertions) became a 7-row@TableTest.While porting, duplicated helpers were dropped in favour of the existing shared ones:
traceIdBytes,newStringTableand thereadFirstSpan(byte[])/readFirstChunk(byte[])overloads now come fromV1PayloadReader, collapsing the hand-rolled unpacker walks in most tests to a single line.PayloadVerifiers.CapturingChannelandPayloadVerifiers.assertEqualsWithNullAsEmptyreplace the test's ownByteArrayChanneland null-as-empty helper.V1PayloadReader.PayloadField/ChunkField/SpanField. This required droppingprivatefrom those three nested constant classes (test-only file, same package).PojoSpanconstructions are behind small named factories (span,spanWithBaggage,spanWithIds,spanWithLinks).Two behaviour-visible tightenings:
spanKindValueConversionasserts the literal wire values0–5instead of comparingTraceMapperV1.SPAN_KIND_*to itself, so a protocol-value change now fails the test.attributes.size()assertions to the attribute tests, pinning that the encoder emits nothing beyond the tags under test plusthread.id/thread.name.Motivation
The equivalent V0.4 and V0.5 payload tests were already migrated in #11871, and
V1PayloadReaderwas written with this port in mind. This brings V1 in line with its siblings and lets the three tests share the same decode helpers.Additional Notes
SpanLink's constructor isprotected, which Groovy could call across packages but Java cannot, hence the smallTestSpanLinksubclass. Config isolation moves fromDDSpecificationto@ExtendWith(WithConfigExtension.class)plus aProcessTags.reset(Config.get())@BeforeEach, matching the V0.4/V0.5 tests.No production code is touched.
🤖 Generated with Claude Code