Address review comments

Author: sarahchen6

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

@@ -32,10 +32,6 @@ When converting Groovy code to Java code, make sure that:
 - When translating Spock `Mock(...)` usage, use `libs.bundles.mockito` instead of writing manual recording/stub implementations
 
 TableTest usage
-  Dependency, if missing add:
-    - Groovy: testImplementation libs.tabletest
-    - Kotlin: testImplementation(libs.tabletest)
-
   Import: `import org.tabletest.junit.TableTest;`
 
   JDK 8 rules:

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

@@ -11,10 +11,6 @@ Process (do in this order):
 4) Write each modified file once in full using Write (no incremental per-test edits).
 5) Run module tests once and verify "BUILD SUCCESSFUL". If failed, inspect JUnit XML report.
 
-Dependency:
-- @TableTest should be in `gradle/java_deps.gradle` as `testImplementation libs.tabletest` where Java modules inherit it via `gradle/java.gradle`.
-- As a fallback, add module-specific `testImplementation(libs.tabletest)` or `testImplementation libs.tabletest`.
-
 Import: `import org.tabletest.junit.TableTest;`
 
 JDK 8 rules:
@@ -41,7 +37,8 @@ A) @CsvSource
 - If delimiter is ',' (default): replace ',' with '|' in rows.
 
 B) @ValueSource
-- Convert to @TableTest with header from parameter name.
+- Keep single-parameter tests on `@ValueSource` (and `@NullSource` when null cases are needed).
+- Otherwise convert to @TableTest with header from parameter name.
 - Each value becomes one row.
 - Add "scenario" column using common sense for name.
 
@@ -59,6 +56,11 @@ C) @MethodSource (convert only if values are representable as strings)
 - '' = empty string.
 - For String params that start with '[' or '{', quote to avoid collection parsing (prefer '[]'/'{}').
 
+D) @TypeConverter
+- Use `@TypeConverter` for symbolic constants used by migrated table rows (e.g. `Long.MAX_VALUE`, `DDSpanId.MAX`).
+- Prefer explicit one-case-one-return mappings.
+- Prefer shared converter utilities (e.g. in `utils/test-utils`) when reuse across modules is likely.
+
 Scenario handling:
 - 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.
 

components/json/build.gradle.kts

@@ -7,7 +7,3 @@ apply(from = "$rootDir/gradle/java.gradle")
 jmh {
   jmhVersion = libs.versions.jmh.get()
 }
-
-dependencies {
-  testImplementation(libs.tabletest)
-}

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

@@ -17,6 +17,8 @@
 import org.junit.jupiter.params.ParameterizedTest;
 import org.junit.jupiter.params.provider.Arguments;
 import org.junit.jupiter.params.provider.MethodSource;
+import org.junit.jupiter.params.provider.NullSource;
+import org.junit.jupiter.params.provider.ValueSource;
 import org.tabletest.junit.Scenario;
 import org.tabletest.junit.TableTest;
 
@@ -77,28 +79,16 @@ static Stream<Arguments> testMappingToJsonObjectArguments() {
             "{\"key1\":null,\"key2\":\"bar\",\"key3\":3,\"key4\":3456789123,\"key5\":3.142,\"key6\":3.141592653589793,\"key7\":true,\"key8\":\"toString\"}"));
   }
 
-  @TableTest({
-    "Scenario     | Json  ",
-    "null         |       ",
-    "null string  | 'null'",
-    "empty string | ''    ",
-    "empty object | '{}'  "
-  })
   @ParameterizedTest(name = "test mapping to Map from empty JSON object: {0}")
+  @NullSource
+  @ValueSource(strings = {"null", "", "{}"})
   void testMappingToMapFromEmptyJsonObject(String json) throws IOException {
     Map<String, Object> parsed = JsonMapper.fromJsonToMap(json);
     assertEquals(emptyMap(), parsed);
   }
 
-  // temporary disable spotless, will open issue to fix this.
-  // spotless:off
-  @TableTest({
-      "Scenario | Json  ",
-      "integer  | 1     ",
-      "array    | [1, 2]"
-  })
-  // spotless:on
   @ParameterizedTest(name = "test mapping to Map from non-object JSON: {0}")
+  @ValueSource(strings = {"1", "[1, 2]"})
   void testMappingToMapFromNonObjectJson(String json) {
     assertThrows(IOException.class, () -> JsonMapper.fromJsonToMap(json));
   }
@@ -138,14 +128,9 @@ void testMappingArrayToJsonArray(String ignoredScenario, String[] input, String
     assertArrayEquals(input != null ? input : new String[] {}, parsed);
   }
 
-  @TableTest({
-    "Scenario      | Json  ",
-    "null          |       ",
-    "null string   | 'null'",
-    "empty string  | ''    ",
-    "empty array   | '[]'  "
-  })
   @ParameterizedTest(name = "test mapping to List from empty JSON object: {0}")
+  @NullSource
+  @ValueSource(strings = {"null", "", "[]"})
   void testMappingToListFromEmptyJsonObject(String json) throws IOException {
     List<String> parsed = JsonMapper.fromJsonToList(json);
     assertEquals(emptyList(), parsed);

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

@@ -7,20 +7,25 @@
 import static org.junit.jupiter.api.Assertions.assertNull;
 import static org.junit.jupiter.api.Assertions.assertThrows;
 
+import datadog.trace.test.util.TableTestTypeConverters;
 import java.util.HashSet;
 import java.util.Set;
 import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.NullSource;
+import org.junit.jupiter.params.provider.ValueSource;
 import org.tabletest.junit.TableTest;
+import org.tabletest.junit.TypeConverterSources;
 
+@TypeConverterSources(TableTestTypeConverters.class)
 class DDSpanIdTest {
 
   @TableTest({
     "scenario           | stringId               | expectedId          ",
     "zero               | '0'                    | 0                   ",
     "one                | '1'                    | 1                   ",
-    "max                | '18446744073709551615' | -1                  ",
-    "long max           | '9223372036854775807'  | 9223372036854775807 ",
-    "long max plus one  | '9223372036854775808'  | -9223372036854775808"
+    "max                | '18446744073709551615' | DDSpanId.MAX        ",
+    "long max           | '9223372036854775807'  | Long.MAX_VALUE      ",
+    "long max plus one  | '9223372036854775808'  | Long.MIN_VALUE      "
   })
   @ParameterizedTest
   void convertIdsFromToString(String stringId, long expectedId) {
@@ -30,18 +35,18 @@ void convertIdsFromToString(String stringId, long expectedId) {
     assertEquals(stringId, DDSpanId.toString(ddid));
   }
 
-  @TableTest({
-    "scenario              | stringId               ",
-    "null                  |                        ",
-    "empty                 | ''                     ",
-    "negative one          | '-1'                   ",
-    "too large             | '18446744073709551616'",
-    "too large variant     | '18446744073709551625'",
-    "too large long        | '184467440737095516150'",
-    "contains alpha first  | '18446744073709551a1' ",
-    "contains alpha last   | '184467440737095511a' "
-  })
   @ParameterizedTest
+  @NullSource
+  @ValueSource(
+      strings = {
+        "",
+        "-1",
+        "18446744073709551616",
+        "18446744073709551625",
+        "184467440737095516150",
+        "18446744073709551a1",
+        "184467440737095511a"
+      })
   void failOnIllegalString(String stringId) {
     assertThrows(NumberFormatException.class, () -> DDSpanId.from(stringId));
   }
@@ -50,10 +55,10 @@ void failOnIllegalString(String stringId) {
     "scenario                    | hexId                  | expectedId          ",
     "zero                        | '0'                    | 0                   ",
     "one                         | '1'                    | 1                   ",
-    "max                         | 'ffffffffffffffff'     | -1                  ",
-    "long max                    | '7fffffffffffffff'     | 9223372036854775807 ",
-    "long min                    | '8000000000000000'     | -9223372036854775808",
-    "long min with leading zeros | '00008000000000000000' | -9223372036854775808",
+    "max                         | 'ffffffffffffffff'     | DDSpanId.MAX        ",
+    "long max                    | '7fffffffffffffff'     | Long.MAX_VALUE      ",
+    "long min                    | '8000000000000000'     | Long.MIN_VALUE      ",
+    "long min with leading zeros | '00008000000000000000' | Long.MIN_VALUE      ",
     "cafebabe                    | 'cafebabe'             | 3405691582          ",
     "fifteen hex digits          | '123456789abcdef'      | 81985529216486895   "
   })
@@ -73,17 +78,17 @@ void convertIdsFromToHexString(String hexId, long expectedId) {
   }
 
   @TableTest({
-    "scenario                                | hexId               | start | length | lowerCaseOnly | expectedId",
-    "null input                              |                     | 1     | 1      | false         |          ",
-    "empty input                             | ''                  | 1     | 1      | false         |          ",
-    "negative start                          | '00'                | -1    | 1      | false         |          ",
-    "zero length                             | '00'                | 0     | 0      | false         |          ",
-    "single zero at index 0                  | '00'                | 0     | 1      | false         | 0        ",
-    "single zero at index 1                  | '00'                | 1     | 1      | false         | 0        ",
-    "single zero at index 1 duplicate        | '00'                | 1     | 1      | false         | 0        ",
-    "max lower-case                          | 'ffffffffffffffff'  | 0     | 16     | true          | -1       ",
-    "upper-case rejected when lower-case only| 'ffffffffffffFfff'  | 0     | 16     | true          |          ",
-    "upper-case accepted when lower disabled | 'ffffffffffffFfff'  | 0     | 16     | false         | -1       "
+    "scenario                                | hexId               | start | length | lowerCaseOnly | expectedId   ",
+    "null input                              |                     | 1     | 1      | false         |              ",
+    "empty input                             | ''                  | 1     | 1      | false         |              ",
+    "negative start                          | '00'                | -1    | 1      | false         |              ",
+    "zero length                             | '00'                | 0     | 0      | false         |              ",
+    "single zero at index 0                  | '00'                | 0     | 1      | false         | DDSpanId.ZERO",
+    "single zero at index 1                  | '00'                | 1     | 1      | false         | DDSpanId.ZERO",
+    "single zero at index 1 duplicate        | '00'                | 1     | 1      | false         | DDSpanId.ZERO",
+    "max lower-case                          | 'ffffffffffffffff'  | 0     | 16     | true          | DDSpanId.MAX ",
+    "upper-case rejected when lower-case only| 'ffffffffffffFfff'  | 0     | 16     | true          |              ",
+    "upper-case accepted when lower disabled | 'ffffffffffffFfff'  | 0     | 16     | false         | DDSpanId.MAX "
   })
   @ParameterizedTest
   void convertIdsFromPartOfHexString(
@@ -102,27 +107,15 @@ void convertIdsFromPartOfHexString(
     }
   }
 
-  @TableTest({
-    "scenario             | hexId              ",
-    "null                 |                    ",
-    "empty                | ''                 ",
-    "negative one         | '-1'               ",
-    "too long             | '10000000000000000'",
-    "invalid middle       | 'ffffffffffffffzf' ",
-    "invalid tail         | 'fffffffffffffffz' "
-  })
   @ParameterizedTest
+  @NullSource
+  @ValueSource(strings = {"", "-1", "10000000000000000", "ffffffffffffffzf", "fffffffffffffffz"})
   void failOnIllegalHexString(String hexId) {
     assertThrows(NumberFormatException.class, () -> DDSpanId.fromHex(hexId));
   }
 
-  @TableTest({
-    "scenario      | strategyName ",
-    "random        | RANDOM       ",
-    "sequential    | SEQUENTIAL   ",
-    "secure random | SECURE_RANDOM"
-  })
   @ParameterizedTest
+  @ValueSource(strings = {"RANDOM", "SEQUENTIAL", "SECURE_RANDOM"})
   void generateIdWithStrategy(String strategyName) {
     IdGenerationStrategy strategy = IdGenerationStrategy.fromName(strategyName);
     Set<Long> checked = new HashSet<Long>();