diff --git a/.claude/skills/migrate-groovy-to-java/SKILL.md b/.claude/skills/migrate-groovy-to-java/SKILL.md index eef88521da7..d75b8134607 100644 --- a/.claude/skills/migrate-groovy-to-java/SKILL.md +++ b/.claude/skills/migrate-groovy-to-java/SKILL.md @@ -16,8 +16,11 @@ Migrate test Groovy files to Java using JUnit 5 When converting Groovy code to Java code, make sure that: - The Java code generated is compatible with JDK 8 -- When translating Spock tests, favor using `@CsvSource` with `|` delimiters -- When using `@MethodSource`, name the arguments method by appending `Arguments` using camelCase to the test method name (e.g. `testMethodArguments`) and return a `Stream` of arguments using `Stream.of(...)` and `arguments(...)` with static import. +- When translating Spock tests, prefer `@TableTest` for data rows that are naturally tabular. See detailed guidance in the "TableTest usage" section. +- `@TableTest` and `@MethodSource` may be combined on the same `@ParameterizedTest` when most cases are tabular but a few cases require programmatic setup. +- In combined mode, keep table-friendly cases in `@TableTest`, and put only non-tabular/complex cases in `@MethodSource`. +- If `@TableTest` is not viable for the test at all, use `@MethodSource` only. +- For `@MethodSource`, name the arguments method `Arguments` (camelCase, e.g. `testMethodArguments`) and return `Stream` using `Stream.of(...)` and `arguments(...)` with static import. - Ensure parameterized test names are human-readable (i.e. no hashcodes); instead add a description string as the first `Arguments.arguments(...)` value or index the test case - When converting tuples, create a light dedicated structure instead to keep the typing system - Instead of checking a state and throwing an exception, use JUnit asserts @@ -27,3 +30,35 @@ When converting Groovy code to Java code, make sure that: - Do not mark local variables `final` - Ensure variables are human-readable; avoid single-letter names and pre-define variables that are referenced multiple times - 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: + - No text blocks. + - @TableTest must use String[] annotation array syntax: + ``` + @TableTest({ + "a | b", + "1 | 2" + }) + ``` + + Spock `where:` → @TableTest: + - First row = header (column names = method parameters). + - Add `scenario` column as first column (display name, not a method parameter). + - Use `|` delimiter; align columns so pipes line up vertically. + - Prefer single quotes for strings with special chars (e.g., `'a|b'`, `'[]'`). + - Blank cell = null (object types); `''` = empty string. + - Collections: `[a, b]` = List/array, `{a, b}` = Set, `[k: v]` = Map. + + Mixed eligibility: + - Prefer combining `@TableTest` + `@MethodSource` on one `@ParameterizedTest` when only some cases are complex. + - Use `@MethodSource` only when tabular representation is not practical for the test. + + Do NOT use @TableTest when: + - Majority of rows require complex objects or custom converters. diff --git a/.claude/skills/migrate-junit-source-to-tabletest/SKILL.md b/.claude/skills/migrate-junit-source-to-tabletest/SKILL.md new file mode 100644 index 00000000000..ae4008039c5 --- /dev/null +++ b/.claude/skills/migrate-junit-source-to-tabletest/SKILL.md @@ -0,0 +1,77 @@ +--- +name: migrate-junit-source-to-tabletest +description: Convert JUnit 5 @MethodSource/@CsvSource/@ValueSource parameterized tests to @TableTest (JDK8) +--- +Goal: Migrate JUnit 5 parameterized tests using @MethodSource/@CsvSource/@ValueSource to @TableTest with minimal churn and passing tests. + +Process (do in this order): +1) Locate targets via Grep (no agent subprocess). Search for: "@ParameterizedTest", "@MethodSource", "@CsvSource", "@ValueSource". +2) Read all matching files up front (parallel is OK). +3) Convert eligible tests to @TableTest. +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: +- If missing, add: + - Groovy: testImplementation libs.tabletest + - Kotlin: testImplementation(libs.tabletest) + +Import: `import org.tabletest.junit.TableTest;` + +JDK 8 rules: +- No text blocks. +- @TableTest must use String[] annotation array syntax: + ``` + @TableTest({ + "a | b", + "1 | 2" + }) + ``` + +Table formatting rules (mandatory): +- Always include a header row (parameter names). +- Always add a "scenario" column; using common sense for naming; scenario is NOT a method parameter. +- Use '|' as delimiter. +- Align columns with spaces so pipes line up vertically. +- Prefer single quotes for strings requiring quotes (e.g., 'a|b', '[]', '{}', ' '). + +Conversions: +A) @CsvSource +- Remove @ParameterizedTest and @CsvSource. +- If delimiter is '|': rows map directly to @TableTest. +- If delimiter is ',' (default): replace ',' with '|' in rows. + +B) @ValueSource +- Convert to @TableTest with header from parameter name. +- Each value becomes one row. +- Add "scenario" column using common sense for name. + +C) @MethodSource (convert only if values are representable as strings) +- Convert when argument values are primitives, strings, enums, booleans, nulls, and simple collection literals supported by TableTest: + - Array: [a, b, ...] + - List: [a, b, ...] + - Set: {a, b, ...} + - Map: [k: v, ...] +- `@TableTest` and `@MethodSource` may be combined on the same `@ParameterizedTest` when most cases are tabular but a few cases require programmatic setup. +- In combined mode, keep table-friendly cases in `@TableTest`, and put only non-tabular/complex cases in `@MethodSource`. +- If `@TableTest` is not viable for the test at all, use `@MethodSource` only. +- For `@MethodSource`, name the arguments method `Arguments` (camelCase, e.g. `testMethodArguments`) and return `Stream` using `Stream.of(...)` and `arguments(...)` with static import. +- Blank cell = null (non-primitive). +- '' = empty string. +- For String params that start with '[' or '{', quote to avoid collection parsing (prefer '[]'/'{}'). + +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. + +Cleanup: +- Delete now-unused @MethodSource provider methods and unused imports. + +Mixed eligibility: +- Prefer combining `@TableTest` + `@MethodSource` on one `@ParameterizedTest` when only some cases are complex. + +Do NOT convert when: +- Most rows require complex builders/mocks. + +Test command (exact): +./gradlew :path:to:module:test --rerun-tasks 2>&1 | tail -20 +- If BUILD FAILED: cat path/to/module/build/test-results/test/TEST-*.xml diff --git a/components/json/build.gradle.kts b/components/json/build.gradle.kts index ce67f74ff29..fda68a66ba8 100644 --- a/components/json/build.gradle.kts +++ b/components/json/build.gradle.kts @@ -7,3 +7,7 @@ apply(from = "$rootDir/gradle/java.gradle") jmh { jmhVersion = libs.versions.jmh.get() } + +dependencies { + testImplementation(libs.tabletest) +} diff --git a/components/json/src/test/java/datadog/json/JsonMapperTest.java b/components/json/src/test/java/datadog/json/JsonMapperTest.java index 3e48ac3492f..d6cacd83b25 100644 --- a/components/json/src/test/java/datadog/json/JsonMapperTest.java +++ b/components/json/src/test/java/datadog/json/JsonMapperTest.java @@ -10,9 +10,6 @@ import static org.junit.jupiter.params.provider.Arguments.arguments; import java.io.IOException; -import java.util.ArrayList; -import java.util.Arrays; -import java.util.HashMap; import java.util.LinkedHashMap; import java.util.List; import java.util.Map; @@ -20,14 +17,22 @@ 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.ValueSource; +import org.tabletest.junit.Scenario; +import org.tabletest.junit.TableTest; class JsonMapperTest { - + @TableTest({ + "Scenario | Input | Expected ", + "null input | | '{}' ", + "empty map | [:] | '{}' ", + "single entry | [key1: value1] | '{\"key1\":\"value1\"}' ", + "two entries | [key1: value1, key2: value2] | '{\"key1\":\"value1\",\"key2\":\"value2\"}' ", + "quoted entries | [key1: va\"lu\"e1, ke\"y2: value2] | '{\"key1\":\"va\\\"lu\\\"e1\",\"ke\\\"y2\":\"value2\"}'" + }) @ParameterizedTest(name = "test mapping to JSON object: {0}") @MethodSource("testMappingToJsonObjectArguments") void testMappingToJsonObject( - @SuppressWarnings("unused") String testCase, Map input, String expected) + @Scenario String ignoredScenario, Map input, String expected) throws IOException { String json = JsonMapper.toJson(input); assertEquals(expected, json); @@ -55,17 +60,6 @@ void testMappingToJsonObject( } static Stream testMappingToJsonObjectArguments() { - Map singleEntry = new LinkedHashMap<>(); - singleEntry.put("key1", "value1"); - - Map twoEntries = new LinkedHashMap<>(); - twoEntries.put("key1", "value1"); - twoEntries.put("key2", "value2"); - - Map quotedEntries = new LinkedHashMap<>(); - quotedEntries.put("key1", "va\"lu\"e1"); - quotedEntries.put("ke\"y2", "value2"); - Map complexMap = new LinkedHashMap<>(); complexMap.put("key1", null); complexMap.put("key2", "bar"); @@ -77,39 +71,47 @@ static Stream testMappingToJsonObjectArguments() { complexMap.put("key8", new UnsupportedType()); return Stream.of( - arguments("null input", null, "{}"), - arguments("empty map", new HashMap<>(), "{}"), - arguments("single entry", singleEntry, "{\"key1\":\"value1\"}"), - arguments("two entries", twoEntries, "{\"key1\":\"value1\",\"key2\":\"value2\"}"), - arguments( - "quoted entries", - quotedEntries, - "{\"key1\":\"va\\\"lu\\\"e1\",\"ke\\\"y2\":\"value2\"}"), arguments( "complex map", complexMap, "{\"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}") - @MethodSource("testMappingToMapFromEmptyJsonObjectArguments") void testMappingToMapFromEmptyJsonObject(String json) throws IOException { Map parsed = JsonMapper.fromJsonToMap(json); assertEquals(emptyMap(), parsed); } - static Stream testMappingToMapFromEmptyJsonObjectArguments() { - return Stream.of(arguments((Object) null), arguments("null"), arguments(""), arguments("{}")); - } - + // 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)); } + @TableTest({ + "Scenario | Input | Expected ", + "null input | | '[]' ", + "empty list | [] | '[]' ", + "single value | [value1] | '[\"value1\"]' ", + "two values | [value1, value2] | '[\"value1\",\"value2\"]' ", + "quoted values | [va\"lu\"e1, value2] | '[\"va\\\"lu\\\"e1\",\"value2\"]' " + }) @ParameterizedTest(name = "test mapping iterable to JSON array: {0}") - @MethodSource("testMappingIterableToJsonArrayArguments") void testMappingIterableToJsonArray(List input, String expected) throws IOException { String json = JsonMapper.toJson(input); assertEquals(expected, json); @@ -118,18 +120,16 @@ void testMappingIterableToJsonArray(List input, String expected) throws assertEquals(input != null ? input : emptyList(), parsed); } - static Stream testMappingIterableToJsonArrayArguments() { - return Stream.of( - arguments(null, "[]"), - arguments(new ArrayList<>(), "[]"), - arguments(Arrays.asList("value1"), "[\"value1\"]"), - arguments(Arrays.asList("value1", "value2"), "[\"value1\",\"value2\"]"), - arguments(Arrays.asList("va\"lu\"e1", "value2"), "[\"va\\\"lu\\\"e1\",\"value2\"]")); - } - + @TableTest({ + "Scenario | Input | Expected ", + "null input | | '[]' ", + "empty array | [] | '[]' ", + "single element | [value1] | '[\"value1\"]' ", + "two elements | [value1, value2] | '[\"value1\",\"value2\"]' ", + "escaped quotes | [va\"lu\"e1, value2] | '[\"va\\\"lu\\\"e1\",\"value2\"]'" + }) @ParameterizedTest(name = "test mapping array to JSON array: {0}") - @MethodSource("testMappingArrayToJsonArrayArguments") - void testMappingArrayToJsonArray(String testCase, String[] input, String expected) + void testMappingArrayToJsonArray(String ignoredScenario, String[] input, String expected) throws IOException { String json = JsonMapper.toJson(input); assertEquals(expected, json); @@ -138,52 +138,45 @@ void testMappingArrayToJsonArray(String testCase, String[] input, String expecte assertArrayEquals(input != null ? input : new String[] {}, parsed); } - static Stream testMappingArrayToJsonArrayArguments() { - return Stream.of( - arguments("null input", (Object) null, "[]"), - arguments("empty array", new String[] {}, "[]"), - arguments("single element", new String[] {"value1"}, "[\"value1\"]"), - arguments("two elements", new String[] {"value1", "value2"}, "[\"value1\",\"value2\"]"), - arguments( - "escaped quotes", - new String[] {"va\"lu\"e1", "value2"}, - "[\"va\\\"lu\\\"e1\",\"value2\"]")); - } - + @TableTest({ + "Scenario | Json ", + "null | ", + "null string | 'null'", + "empty string | '' ", + "empty array | '[]' " + }) @ParameterizedTest(name = "test mapping to List from empty JSON object: {0}") - @MethodSource("testMappingToListFromEmptyJsonObjectArguments") void testMappingToListFromEmptyJsonObject(String json) throws IOException { List parsed = JsonMapper.fromJsonToList(json); assertEquals(emptyList(), parsed); } - static Stream testMappingToListFromEmptyJsonObjectArguments() { - return Stream.of(arguments((Object) null), arguments("null"), arguments(""), arguments("[]")); - } - + @TableTest({ + "Scenario | input | expected ", + " null value | | '' ", + " empty string | '' | '' ", + " \\b | '\b' | '\"\\b\"' ", + " \\t | '\t' | '\"\\t\"' ", + " \\f | '\f' | '\"\\f\"' ", + " a | 'a' | '\"a\"' ", + " / | '/' | '\"\\/\"' ", + }) @ParameterizedTest(name = "test mapping to JSON string: {0}") @MethodSource("testMappingToJsonStringArguments") - void testMappingToJsonString(String input, String expected) { + void testMappingToJsonString(@Scenario String ignoredScenario, String input, String expected) { String json = JsonMapper.toJson(input); assertEquals(expected, json); } static Stream testMappingToJsonStringArguments() { return Stream.of( - arguments((Object) null, ""), - arguments("", ""), - arguments(String.valueOf((char) 4096), "\"\\u1000\""), - arguments(String.valueOf((char) 256), "\"\\u0100\""), - arguments(String.valueOf((char) 128), "\"\\u0080\""), - arguments("\b", "\"\\b\""), - arguments("\t", "\"\\t\""), - arguments("\n", "\"\\n\""), - arguments("\f", "\"\\f\""), - arguments("\r", "\"\\r\""), - arguments("\"", "\"\\\"\""), - arguments("/", "\"\\/\""), - arguments("\\", "\"\\\\\""), - arguments("a", "\"a\"")); + arguments("char #4096", String.valueOf((char) 4096), "\"\\u1000\""), + arguments("char #256", String.valueOf((char) 256), "\"\\u0100\""), + arguments("char #128", String.valueOf((char) 128), "\"\\u0080\""), + arguments("\\n", "\n", "\"\\n\""), + arguments("\\r", "\r", "\"\\r\""), + arguments("\"", "\"", "\"\\\"\""), + arguments("\\", "\\", "\"\\\\\"")); } private static class UnsupportedType { diff --git a/gradle/libs.versions.toml b/gradle/libs.versions.toml index 8c6bcb2ec13..30290ef3a2f 100644 --- a/gradle/libs.versions.toml +++ b/gradle/libs.versions.toml @@ -70,6 +70,7 @@ junit5 = "5.14.1" junit-platform = "1.14.1" mockito = "4.4.0" spock = "2.4-groovy-3.0" +tabletest = "1.2.0" testcontainers = "1.21.4" [libraries] @@ -161,6 +162,7 @@ objenesis = { module = "org.objenesis:objenesis", version = "3.3" } # Used by Sp spock-core = { module = "org.spockframework:spock-core", version.ref = "spock" } spock-junit4 = { module = "org.spockframework:spock-junit4", version.ref = "spock" } spock-spring = { module = "org.spockframework:spock-spring", version.ref = "spock" } +tabletest = { module = "org.tabletest:tabletest-junit", version.ref = "tabletest" } testcontainers = { module = "org.testcontainers:testcontainers", version.ref = "testcontainers" } testcontainers-localstack = { module = "org.testcontainers:localstack", version.ref = "testcontainers" } diff --git a/gradle/repositories.gradle b/gradle/repositories.gradle index 98085e93c50..b5bccce0969 100644 --- a/gradle/repositories.gradle +++ b/gradle/repositories.gradle @@ -7,6 +7,13 @@ repositories { maven { url project.rootProject.property("mavenRepositoryProxy") allowInsecureProtocol = true + content { + // TODO: For unknown reasons `org.tabletest` artifacts resolved as invalid jars via `mavenRepositoryProxy`. + // Build is failing with message: `error reading .gradle/caches/.../tabletest-junit-1.2.0.jar; zip END header not found` + // Revisit this place once `org.tabletest` artifacts will be updated, there is a chance that issue will be fixed. + // Temporary exclude it here so Gradle resolves it directly from mavenCentral(). + excludeGroup "org.tabletest" + } } } mavenCentral()