Skip to content

Commit 9352dfa

Browse files
sarahchen6sarahchen6
authored
Use TableTest in dd-trace-api tests (#10844)
Use TableTest in dd-trace-api Fix test formatting and add tabletest to java_deps Address review comments Merge branch 'master' into sarahchen6/use-table-test Add params and re-org converters Use ValueSets Co-authored-by: sarah.chen <sarah.chen@datadoghq.com>
1 parent 1fbf60f commit 9352dfa

File tree

12 files changed

+259
-341
lines changed

12 files changed

+259
-341
lines changed

‎.claude/skills/migrate-groovy-to-java/SKILL.md‎

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -32,10 +32,6 @@ When converting Groovy code to Java code, make sure that:
3232
- When translating Spock `Mock(...)` usage, use `libs.bundles.mockito` instead of writing manual recording/stub implementations
3333

3434
TableTest usage
35-
Dependency, if missing add:
36-
- Groovy: testImplementation libs.tabletest
37-
- Kotlin: testImplementation(libs.tabletest)
38-
3935
Import: `import org.tabletest.junit.TableTest;`
4036

4137
JDK 8 rules:

‎.claude/skills/migrate-junit-source-to-tabletest/SKILL.md‎

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -11,11 +11,6 @@ Process (do in this order):
1111
4) Write each modified file once in full using Write (no incremental per-test edits).
1212
5) Run module tests once and verify "BUILD SUCCESSFUL". If failed, inspect JUnit XML report.
1313

14-
Dependency:
15-
- If missing, add:
16-
- Groovy: testImplementation libs.tabletest
17-
- Kotlin: testImplementation(libs.tabletest)
18-
1914
Import: `import org.tabletest.junit.TableTest;`
2015

2116
JDK 8 rules:
@@ -34,6 +29,7 @@ Table formatting rules (mandatory):
3429
- Use '|' as delimiter.
3530
- Align columns with spaces so pipes line up vertically.
3631
- Prefer single quotes for strings requiring quotes (e.g., 'a|b', '[]', '{}', ' ').
32+
- Use value sets (`{a, b, c}`) instead of matrix-style repetition when only one dimension varies across otherwise-identical rows.
3733

3834
Conversions:
3935
A) @CsvSource
@@ -42,7 +38,8 @@ A) @CsvSource
4238
- If delimiter is ',' (default): replace ',' with '|' in rows.
4339

4440
B) @ValueSource
45-
- Convert to @TableTest with header from parameter name.
41+
- Keep single-parameter tests on `@ValueSource` (and `@NullSource` when null cases are needed).
42+
- Otherwise convert to @TableTest with header from parameter name.
4643
- Each value becomes one row.
4744
- Add "scenario" column using common sense for name.
4845

@@ -60,6 +57,11 @@ C) @MethodSource (convert only if values are representable as strings)
6057
- '' = empty string.
6158
- For String params that start with '[' or '{', quote to avoid collection parsing (prefer '[]'/'{}').
6259

60+
D) @TypeConverter
61+
- Use `@TypeConverter` for symbolic constants used by migrated table rows (e.g. `Long.MAX_VALUE`, `DDSpanId.MAX`).
62+
- Prefer explicit one-case-one-return mappings.
63+
- Prefer shared converter utilities (e.g. in `utils/test-utils`) when reuse across modules is likely.
64+
6365
Scenario handling:
6466
- If MethodSource includes a leading description string OR @ParameterizedTest(name=...) uses {0}, convert that to a scenario column and remove that parameter from method signature.
6567

‎components/json/build.gradle.kts‎

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,3 @@ apply(from = "$rootDir/gradle/java.gradle")
77
jmh {
88
jmhVersion = libs.versions.jmh.get()
99
}
10-
11-
dependencies {
12-
testImplementation(libs.tabletest)
13-
}

‎components/json/src/test/java/datadog/json/JsonMapperTest.java‎

Lines changed: 7 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,8 @@
1717
import org.junit.jupiter.params.ParameterizedTest;
1818
import org.junit.jupiter.params.provider.Arguments;
1919
import org.junit.jupiter.params.provider.MethodSource;
20+
import org.junit.jupiter.params.provider.NullSource;
21+
import org.junit.jupiter.params.provider.ValueSource;
2022
import org.tabletest.junit.Scenario;
2123
import org.tabletest.junit.TableTest;
2224

@@ -77,28 +79,16 @@ static Stream<Arguments> testMappingToJsonObjectArguments() {
7779
"{\"key1\":null,\"key2\":\"bar\",\"key3\":3,\"key4\":3456789123,\"key5\":3.142,\"key6\":3.141592653589793,\"key7\":true,\"key8\":\"toString\"}"));
7880
}
7981

80-
@TableTest({
81-
"Scenario | Json ",
82-
"null | ",
83-
"null string | 'null'",
84-
"empty string | '' ",
85-
"empty object | '{}' "
86-
})
8782
@ParameterizedTest(name = "test mapping to Map from empty JSON object: {0}")
83+
@NullSource
84+
@ValueSource(strings = {"null", "", "{}"})
8885
void testMappingToMapFromEmptyJsonObject(String json) throws IOException {
8986
Map<String, Object> parsed = JsonMapper.fromJsonToMap(json);
9087
assertEquals(emptyMap(), parsed);
9188
}
9289

93-
// temporary disable spotless, will open issue to fix this.
94-
// spotless:off
95-
@TableTest({
96-
"Scenario | Json ",
97-
"integer | 1 ",
98-
"array | [1, 2]"
99-
})
100-
// spotless:on
10190
@ParameterizedTest(name = "test mapping to Map from non-object JSON: {0}")
91+
@ValueSource(strings = {"1", "[1, 2]"})
10292
void testMappingToMapFromNonObjectJson(String json) {
10393
assertThrows(IOException.class, () -> JsonMapper.fromJsonToMap(json));
10494
}
@@ -138,14 +128,9 @@ void testMappingArrayToJsonArray(String ignoredScenario, String[] input, String
138128
assertArrayEquals(input != null ? input : new String[] {}, parsed);
139129
}
140130

141-
@TableTest({
142-
"Scenario | Json ",
143-
"null | ",
144-
"null string | 'null'",
145-
"empty string | '' ",
146-
"empty array | '[]' "
147-
})
148131
@ParameterizedTest(name = "test mapping to List from empty JSON object: {0}")
132+
@NullSource
133+
@ValueSource(strings = {"null", "", "[]"})
149134
void testMappingToListFromEmptyJsonObject(String json) throws IOException {
150135
List<String> parsed = JsonMapper.fromJsonToList(json);
151136
assertEquals(emptyList(), parsed);

‎dd-trace-api/src/test/java/datadog/trace/api/DDSpanIdTest.java‎

Lines changed: 56 additions & 93 deletions
Original file line numberDiff line numberDiff line change
@@ -7,58 +7,61 @@
77
import static org.junit.jupiter.api.Assertions.assertNull;
88
import static org.junit.jupiter.api.Assertions.assertThrows;
99

10-
import java.math.BigInteger;
1110
import java.util.HashSet;
1211
import java.util.Set;
13-
import java.util.stream.Stream;
1412
import org.junit.jupiter.params.ParameterizedTest;
15-
import org.junit.jupiter.params.provider.Arguments;
16-
import org.junit.jupiter.params.provider.MethodSource;
13+
import org.junit.jupiter.params.provider.NullSource;
1714
import org.junit.jupiter.params.provider.ValueSource;
15+
import org.tabletest.junit.TableTest;
16+
import org.tabletest.junit.TypeConverterSources;
1817

18+
@TypeConverterSources(DDTraceApiTableTestConverters.class)
1919
class DDSpanIdTest {
2020

21-
@ParameterizedTest(name = "convert ids from/to String {0}")
22-
@MethodSource("convertIdsFromToStringArguments")
23-
void convertIdsFromToString(String displayName, String stringId, long expectedId) {
21+
@TableTest({
22+
"scenario | stringId | expectedId ",
23+
"zero | '0' | 0 ",
24+
"one | '1' | 1 ",
25+
"max | '18446744073709551615' | DDSpanId.MAX ",
26+
"long max | '9223372036854775807' | Long.MAX_VALUE ",
27+
"long max plus one | '9223372036854775808' | Long.MIN_VALUE "
28+
})
29+
@ParameterizedTest(name = "convert ids from/to String [{index}]")
30+
void convertIdsFromToString(String stringId, long expectedId) {
2431
long ddid = DDSpanId.from(stringId);
2532

2633
assertEquals(expectedId, ddid);
2734
assertEquals(stringId, DDSpanId.toString(ddid));
2835
}
2936

30-
static Stream<Arguments> convertIdsFromToStringArguments() {
31-
return Stream.of(
32-
Arguments.of("zero", "0", 0L),
33-
Arguments.of("one", "1", 1L),
34-
Arguments.of("max", "18446744073709551615", DDSpanId.MAX),
35-
Arguments.of("long max", String.valueOf(Long.MAX_VALUE), Long.MAX_VALUE),
36-
Arguments.of(
37-
"long max plus one",
38-
BigInteger.valueOf(Long.MAX_VALUE).add(BigInteger.ONE).toString(),
39-
Long.MIN_VALUE));
40-
}
41-
42-
@ParameterizedTest(name = "fail on illegal String {0}")
43-
@MethodSource("failOnIllegalStringArguments")
37+
@ParameterizedTest(name = "fail on illegal String [{index}]")
38+
@NullSource
39+
@ValueSource(
40+
strings = {
41+
"",
42+
"-1",
43+
"18446744073709551616",
44+
"18446744073709551625",
45+
"184467440737095516150",
46+
"18446744073709551a1",
47+
"184467440737095511a"
48+
})
4449
void failOnIllegalString(String stringId) {
4550
assertThrows(NumberFormatException.class, () -> DDSpanId.from(stringId));
4651
}
4752

48-
static Stream<Arguments> failOnIllegalStringArguments() {
49-
return Stream.of(
50-
Arguments.of((Object) null),
51-
Arguments.of(""),
52-
Arguments.of("-1"),
53-
Arguments.of("18446744073709551616"),
54-
Arguments.of("18446744073709551625"),
55-
Arguments.of("184467440737095516150"),
56-
Arguments.of("18446744073709551a1"),
57-
Arguments.of("184467440737095511a"));
58-
}
59-
60-
@ParameterizedTest(name = "convert ids from/to hex String {0}")
61-
@MethodSource("convertIdsFromToHexStringArguments")
53+
@TableTest({
54+
"scenario | hexId | expectedId ",
55+
"zero | '0' | 0 ",
56+
"one | '1' | 1 ",
57+
"max | 'ffffffffffffffff' | DDSpanId.MAX ",
58+
"long max | '7fffffffffffffff' | Long.MAX_VALUE ",
59+
"long min | '8000000000000000' | Long.MIN_VALUE ",
60+
"long min with leading zeros | '00008000000000000000' | Long.MIN_VALUE ",
61+
"hex sample | 'cafebabe' | 3405691582 ",
62+
"fifteen hex digits | '123456789abcdef' | 81985529216486895 "
63+
})
64+
@ParameterizedTest(name = "convert ids from/to hex String [{index}]")
6265
void convertIdsFromToHexString(String hexId, long expectedId) {
6366
long ddid = DDSpanId.fromHex(hexId);
6467
String padded16 =
@@ -73,27 +76,22 @@ void convertIdsFromToHexString(String hexId, long expectedId) {
7376
assertEquals(padded16, DDSpanId.toHexStringPadded(ddid));
7477
}
7578

76-
static Stream<Arguments> convertIdsFromToHexStringArguments() {
77-
return Stream.of(
78-
Arguments.of("0", 0L),
79-
Arguments.of("1", 1L),
80-
Arguments.of(repeat("f", 16), DDSpanId.MAX),
81-
Arguments.of("7" + repeat("f", 15), Long.MAX_VALUE),
82-
Arguments.of("8" + repeat("0", 15), Long.MIN_VALUE),
83-
Arguments.of(repeat("0", 4) + "8" + repeat("0", 15), Long.MIN_VALUE),
84-
Arguments.of("cafebabe", 3405691582L),
85-
Arguments.of("123456789abcdef", 81985529216486895L));
86-
}
87-
88-
@ParameterizedTest(name = "convert ids from part of hex String {0}")
89-
@MethodSource("convertIdsFromPartOfHexStringArguments")
79+
@TableTest({
80+
"scenario | hexId | start | length | lowerCaseOnly | expectedId ",
81+
"null input | | 1 | 1 | false | ",
82+
"empty input | '' | 1 | 1 | false | ",
83+
"negative start | '00' | -1 | 1 | false | ",
84+
"zero length | '00' | 0 | 0 | false | ",
85+
"single zero at index 0 | '00' | 0 | 1 | false | DDSpanId.ZERO",
86+
"single zero at index 1 | '00' | 1 | 1 | false | DDSpanId.ZERO",
87+
"single zero at index 1 duplicate | '00' | 1 | 1 | false | DDSpanId.ZERO",
88+
"max lower-case | 'ffffffffffffffff' | 0 | 16 | true | DDSpanId.MAX ",
89+
"upper-case rejected when lower-case only| 'ffffffffffffFfff' | 0 | 16 | true | ",
90+
"upper-case accepted when lower disabled | 'ffffffffffffFfff' | 0 | 16 | false | DDSpanId.MAX "
91+
})
92+
@ParameterizedTest(name = "convert ids from part of hex String [{index}]")
9093
void convertIdsFromPartOfHexString(
91-
String displayName,
92-
String hexId,
93-
int start,
94-
int length,
95-
boolean lowerCaseOnly,
96-
Long expectedId) {
94+
String hexId, int start, int length, boolean lowerCaseOnly, Long expectedId) {
9795
Long parsedId = null;
9896
try {
9997
parsedId = DDSpanId.fromHex(hexId, start, length, lowerCaseOnly);
@@ -108,48 +106,13 @@ void convertIdsFromPartOfHexString(
108106
}
109107
}
110108

111-
static Stream<Arguments> convertIdsFromPartOfHexStringArguments() {
112-
return Stream.of(
113-
Arguments.of("null input", null, 1, 1, false, null),
114-
Arguments.of("empty input", "", 1, 1, false, null),
115-
Arguments.of("negative start", "00", -1, 1, false, null),
116-
Arguments.of("zero length", "00", 0, 0, false, null),
117-
Arguments.of("single zero at index 0", "00", 0, 1, false, DDSpanId.ZERO),
118-
Arguments.of("single zero at index 1", "00", 1, 1, false, DDSpanId.ZERO),
119-
Arguments.of("single zero at index 1 duplicate", "00", 1, 1, false, DDSpanId.ZERO),
120-
Arguments.of("max lower-case", repeat("f", 16), 0, 16, true, DDSpanId.MAX),
121-
Arguments.of(
122-
"upper-case rejected when lower-case only",
123-
repeat("f", 12) + "Ffff",
124-
0,
125-
16,
126-
true,
127-
null),
128-
Arguments.of(
129-
"upper-case accepted when lower-case disabled",
130-
repeat("f", 12) + "Ffff",
131-
0,
132-
16,
133-
false,
134-
DDSpanId.MAX));
135-
}
136-
137-
@ParameterizedTest(name = "fail on illegal hex String {0}")
138-
@MethodSource("failOnIllegalHexStringArguments")
109+
@ParameterizedTest(name = "fail on illegal hex String [{index}]")
110+
@NullSource
111+
@ValueSource(strings = {"", "-1", "10000000000000000", "ffffffffffffffzf", "fffffffffffffffz"})
139112
void failOnIllegalHexString(String hexId) {
140113
assertThrows(NumberFormatException.class, () -> DDSpanId.fromHex(hexId));
141114
}
142115

143-
static Stream<Arguments> failOnIllegalHexStringArguments() {
144-
return Stream.of(
145-
Arguments.of((Object) null),
146-
Arguments.of(""),
147-
Arguments.of("-1"),
148-
Arguments.of("1" + repeat("0", 16)),
149-
Arguments.of(repeat("f", 14) + "zf"),
150-
Arguments.of(repeat("f", 15) + "z"));
151-
}
152-
153116
@ParameterizedTest(name = "generate id with {0}")
154117
@ValueSource(strings = {"RANDOM", "SEQUENTIAL", "SECURE_RANDOM"})
155118
void generateIdWithStrategy(String strategyName) {

‎dd-trace-api/src/test/java/datadog/trace/api/DDTraceApiTableTestConverters.java‎

Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,51 @@
1+
package datadog.trace.api;
2+
3+
import datadog.trace.test.util.TableTestTypeConverters;
4+
import org.tabletest.junit.TypeConverter;
5+
6+
/** TableTest converters shared by dd-trace-api test classes for unparsable constants. */
7+
public final class DDTraceApiTableTestConverters {
8+
9+
private DDTraceApiTableTestConverters() {}
10+
11+
@TypeConverter
12+
public static long toLong(String value) {
13+
if (value == null) {
14+
throw new IllegalArgumentException("Value cannot be null");
15+
}
16+
String token = value.trim();
17+
switch (token) {
18+
case "DDSpanId.MAX":
19+
return DDSpanId.MAX;
20+
case "DDSpanId.ZERO":
21+
return DDSpanId.ZERO;
22+
default:
23+
return TableTestTypeConverters.toLong(token);
24+
}
25+
}
26+
27+
@TypeConverter
28+
public static DD64bTraceId toDD64bTraceId(String value) {
29+
if (value == null) {
30+
throw new IllegalArgumentException("Value cannot be null");
31+
}
32+
switch (value.trim()) {
33+
case "DD64bTraceId.ZERO":
34+
return DD64bTraceId.from(0L);
35+
case "DD64bTraceId.ONE":
36+
return DD64bTraceId.from(1L);
37+
case "DD64bTraceId.MAX":
38+
return DD64bTraceId.MAX;
39+
case "DD64bTraceId.LONG_MAX":
40+
return DD64bTraceId.from(Long.MAX_VALUE);
41+
case "DD64bTraceId.LONG_MIN":
42+
return DD64bTraceId.from(Long.MIN_VALUE);
43+
case "DD64bTraceId.CAFEBABE":
44+
return DD64bTraceId.from(3405691582L);
45+
case "DD64bTraceId.HEX":
46+
return DD64bTraceId.from(81985529216486895L);
47+
default:
48+
throw new IllegalArgumentException("Unsupported DD64bTraceId token: " + value);
49+
}
50+
}
51+
}

0 commit comments

Comments
 (0)