From 40ac47f393c28d255fae853957649e4d2c6b264d Mon Sep 17 00:00:00 2001 From: Jordan Wong Date: Tue, 1 Sep 2026 07:28:55 -0400 Subject: [PATCH 1/2] skill(apm-integrations): clarify Groovy-vs-Java test DSL rule, single source of truth "Full Java instrumentation test support is not yet available" read as an absolute ban, but the repo has real Java/JUnit instrumentation test suites for module families already on that DSL. Rewrote the rule to state plainly: Groovy/Spock is unconditional for new tests; an existing Java-DSL sibling is not license to add more Java tests, and is not license to migrate a Groovy family to Java either. Stated once in tests.md (the single source of truth); SKILL.md and the banner-comments section point to it rather than restating it, so there's one place to update instead of several with slightly different wording. Co-Authored-By: Claude Opus 4.8 (1M context) --- .agents/skills/apm-integrations/SKILL.md | 2 +- .agents/skills/apm-integrations/references/tests.md | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/.agents/skills/apm-integrations/SKILL.md b/.agents/skills/apm-integrations/SKILL.md index 5bacedebeaa..af16044f499 100644 --- a/.agents/skills/apm-integrations/SKILL.md +++ b/.agents/skills/apm-integrations/SKILL.md @@ -93,7 +93,7 @@ Cover all mandatory test types: ### 1. Instrumentation test (mandatory) -**Read [Writing Tests](references/tests.md).** Instrumentation tests are Groovy/Spock (`src/test/groovy/`) — add the `tag: override groovy enforcement` label to suppress the `Enforce Groovy Migration` CI check (which blocks new `.groovy` files by default — instrumentation tests are intentionally Groovy/Spock). Must cover error/exception scenarios. When adding new integration names, register them per [Supported Configurations](references/supported-configurations.md). When `compileOnly` and `testImplementation` use different versions, comment the specific class that requires the higher version. Include sibling version modules as `testImplementation` dependencies for mutual-exclusion tests. +**Read [Writing Tests](references/tests.md) — it defines the Groovy-vs-Java DSL rule; follow it before creating any test files.** Adding new `.groovy` files to a PR triggers the `Enforce Groovy Migration` CI check — add the `tag: override groovy enforcement` label to suppress it. Must cover error/exception scenarios. When adding new integration names, register them per [Supported Configurations](references/supported-configurations.md). When `compileOnly` and `testImplementation` use different versions, comment the specific class that requires the higher version. Include sibling version modules as `testImplementation` dependencies for mutual-exclusion tests. ### 2. Muzzle directives (mandatory) diff --git a/.agents/skills/apm-integrations/references/tests.md b/.agents/skills/apm-integrations/references/tests.md index c1be9b8cc07..49f7904e280 100644 --- a/.agents/skills/apm-integrations/references/tests.md +++ b/.agents/skills/apm-integrations/references/tests.md @@ -4,7 +4,7 @@ ## 1. Instrumentation test (mandatory) -**Write Groovy/Spock tests for instrumentation tests** (per `AGENTS.md`: "Only use Groovy / Spock tests for instrumentation and smoke tests"). Full Java instrumentation test support is not yet available. Adding new `.groovy` files to a PR will trigger the `Enforce Groovy Migration` bot — add the `tag: override groovy enforcement` label to bypass it. +**Write Groovy/Spock tests for instrumentation tests** (per `AGENTS.md`: "Only use Groovy / Spock tests for instrumentation and smoke tests"). This is unconditional — including for modules whose existing siblings happen to use Java/JUnit — an existing Java-DSL sibling is NOT license to add more Java tests; do not migrate a Groovy family to Java either. Confirm what the family is on with `ls src/test/` on the module's master version and its version-siblings (e.g. `jedis-1.4/`, `jedis-4.0/` for `jedis-3.0`) before writing tests. This file also shows Java `AbstractInstrumentationTest` examples below; those exist ONLY to illustrate style rules (no-banner-comments, error-path coverage) for modules that are ALREADY on the Java/JUnit DSL — they are NOT a license to introduce Java into a Groovy family. Introducing a `src/test/java/**` test into a module whose siblings are `src/test/groovy/**` diverges from master's style, adds review burden, and trips the Java DSL's stricter default-tag matcher (`TagsMatcher.defaultTags()` currently omits some tags the Groovy DSL tolerates, e.g. `_dd.svc_src`), producing spurious CI failures that are NOT instrumentation defects. Adding new `.groovy` files to a PR will trigger the `Enforce Groovy Migration` bot — add the `tag: override groovy enforcement` label to bypass it. - Groovy/Spock test class in `src/test/groovy/datadog/trace/instrumentation//` - Verify: spans created, tags set, errors propagated, resource names correct @@ -129,7 +129,7 @@ Common libraries where this split matters: Reactor, Netty, gRPC, Kafka clients ( Do NOT insert banner-style separator comments (e.g. `// --------- Successful completion ---------`) inside test files to group related test methods. Banner comments have unclear scope, don't render usefully in IDEs, and add review burden without a benefit that justifies the noise. -**If a group of related tests warrants its own heading**, extract them into a separate test class with a focused class-level Javadoc: +**If a group of related tests warrants its own heading**, extract them into a separate test class with a focused class-level Javadoc. (The examples below are in Java purely because that's a compact way to show the no-banner/focused-class pattern — the same rule applies to Groovy/Spock. Per the DSL rule at the top of this file, do NOT read these snippets as license to write Java tests for a Groovy-family module.) ```java // ❌ Banner comments From f4f909d671e999d945165fc3644041f156c95d92 Mon Sep 17 00:00:00 2001 From: Jordan Wong Date: Tue, 1 Sep 2026 08:35:32 -0400 Subject: [PATCH 2/2] skill(apm-integrations): trim redundant prose in test-setup guidance MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - SKILL.md Step 4: collapsed the 2-line source-layout bullet into the directory-creation line; test dir is already governed by Step 9.1, no need to name it twice. - tests.md: cut the TagsMatcher.defaultTags()/_dd.svc_src mechanism detail from the DSL rule — the consequence ("diverges from master's style, trips the stricter tag matcher") is enough for the rule; the exact internals aren't needed to follow it. - tests.md banner-comments section: replaced a full caveat sentence with a one-line comment inside the code fence itself, next to the code it's caveating. Co-Authored-By: Claude Opus 4.8 (1M context) --- .agents/skills/apm-integrations/SKILL.md | 11 ++++------- .agents/skills/apm-integrations/references/tests.md | 5 +++-- 2 files changed, 7 insertions(+), 9 deletions(-) diff --git a/.agents/skills/apm-integrations/SKILL.md b/.agents/skills/apm-integrations/SKILL.md index af16044f499..0d980a95715 100644 --- a/.agents/skills/apm-integrations/SKILL.md +++ b/.agents/skills/apm-integrations/SKILL.md @@ -43,16 +43,13 @@ pattern before writing new code. Use it as a template. ## Step 4 – Set up the module -1. Create directory: `dd-java-agent/instrumentation/$framework/$framework-$minVersion/` -2. Under it, create the standard Maven source layout: - - `src/main/java/` — instrumentation code - - `src/test/groovy/` — Groovy/Spock instrumentation tests (see Step 9.1) -3. Create `build.gradle` with: +1. Create directory: `dd-java-agent/instrumentation/$framework/$framework-$minVersion/`, with the standard Maven source layout under it (`src/main/java/` for instrumentation code; test source dir per Step 9.1) +2. Create `build.gradle` with: - `compileOnly` dependencies for the target framework - `testImplementation` dependencies for tests - `muzzle { pass { } }` directives (see Step 9.2) -4. Register the new module in `settings.gradle.kts` in **alphabetical order** -5. Register all integration names in `metadata/supported-configurations.json` — **read [Supported Configurations](references/supported-configurations.md)** for the exact key shapes and CI checks involved. Declaring several names (`super("a", "b")`) means one entry each. +3. Register the new module in `settings.gradle.kts` in **alphabetical order** +4. Register all integration names in `metadata/supported-configurations.json` — **read [Supported Configurations](references/supported-configurations.md)** for the exact key shapes and CI checks involved. Declaring several names (`super("a", "b")`) means one entry each. **See [Naming Conventions](references/naming-conventions.md) — module directory name must end with a version or an allowed suffix (`-common`, `-stubs`, `-iast`). Java filename and `public class` name MUST match character-for-character including acronym casing (CRITICAL — see § "Java naming consistency").** diff --git a/.agents/skills/apm-integrations/references/tests.md b/.agents/skills/apm-integrations/references/tests.md index 49f7904e280..566fe15ab4e 100644 --- a/.agents/skills/apm-integrations/references/tests.md +++ b/.agents/skills/apm-integrations/references/tests.md @@ -4,7 +4,7 @@ ## 1. Instrumentation test (mandatory) -**Write Groovy/Spock tests for instrumentation tests** (per `AGENTS.md`: "Only use Groovy / Spock tests for instrumentation and smoke tests"). This is unconditional — including for modules whose existing siblings happen to use Java/JUnit — an existing Java-DSL sibling is NOT license to add more Java tests; do not migrate a Groovy family to Java either. Confirm what the family is on with `ls src/test/` on the module's master version and its version-siblings (e.g. `jedis-1.4/`, `jedis-4.0/` for `jedis-3.0`) before writing tests. This file also shows Java `AbstractInstrumentationTest` examples below; those exist ONLY to illustrate style rules (no-banner-comments, error-path coverage) for modules that are ALREADY on the Java/JUnit DSL — they are NOT a license to introduce Java into a Groovy family. Introducing a `src/test/java/**` test into a module whose siblings are `src/test/groovy/**` diverges from master's style, adds review burden, and trips the Java DSL's stricter default-tag matcher (`TagsMatcher.defaultTags()` currently omits some tags the Groovy DSL tolerates, e.g. `_dd.svc_src`), producing spurious CI failures that are NOT instrumentation defects. Adding new `.groovy` files to a PR will trigger the `Enforce Groovy Migration` bot — add the `tag: override groovy enforcement` label to bypass it. +**Write Groovy/Spock tests for instrumentation tests** (per `AGENTS.md`: "Only use Groovy / Spock tests for instrumentation and smoke tests"). This is unconditional — including for modules whose existing siblings happen to use Java/JUnit — an existing Java-DSL sibling is NOT license to add more Java tests; do not migrate a Groovy family to Java either. Confirm what the family is on with `ls src/test/` on the module's master version and its version-siblings (e.g. `jedis-1.4/`, `jedis-4.0/` for `jedis-3.0`) before writing tests. This file also shows Java `AbstractInstrumentationTest` examples below; those exist ONLY to illustrate style rules (no-banner-comments, error-path coverage) for modules that are ALREADY on the Java/JUnit DSL — they are NOT a license to introduce Java into a Groovy family. Introducing a `src/test/java/**` test into a module whose siblings are `src/test/groovy/**` diverges from master's style and can trip the Java DSL's stricter tag matcher, producing spurious CI failures that are NOT instrumentation defects. Adding new `.groovy` files to a PR will trigger the `Enforce Groovy Migration` bot — add the `tag: override groovy enforcement` label to bypass it. - Groovy/Spock test class in `src/test/groovy/datadog/trace/instrumentation//` - Verify: spans created, tags set, errors propagated, resource names correct @@ -129,9 +129,10 @@ Common libraries where this split matters: Reactor, Netty, gRPC, Kafka clients ( Do NOT insert banner-style separator comments (e.g. `// --------- Successful completion ---------`) inside test files to group related test methods. Banner comments have unclear scope, don't render usefully in IDEs, and add review burden without a benefit that justifies the noise. -**If a group of related tests warrants its own heading**, extract them into a separate test class with a focused class-level Javadoc. (The examples below are in Java purely because that's a compact way to show the no-banner/focused-class pattern — the same rule applies to Groovy/Spock. Per the DSL rule at the top of this file, do NOT read these snippets as license to write Java tests for a Groovy-family module.) +**If a group of related tests warrants its own heading**, extract them into a separate test class with a focused class-level Javadoc. ```java +// Java example, style only — same rule applies to Groovy/Spock (see DSL rule above) // ❌ Banner comments class RxJava3ResultExtensionTest extends AbstractInstrumentationTest { // ---------------------------------------------------------------------------