skill(apm-integrations): split tests.md into mandatory rules + situational style guide

.agents/skills/apm-integrations/SKILL.md

@@ -93,15 +93,15 @@ 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). Situational rules (version-mismatch comments, mutual-exclusion deps, etc.) are in [tests-style.md](references/tests-style.md) — check it if your module hits one of those cases.
 
 ### 2. Muzzle directives (mandatory)
 
 **Read [Muzzle Directives](references/muzzle.md)** — it covers all three valid patterns and their `assertInverse` rules. Search adjacent module `build.gradle` files for `skipVersions` before declaring a new version-bounded module's muzzle directives. **If a prior-major-version sibling module already exists in the repo** (e.g. you're writing `rxjava-3.0` and `rxjava-2.0` exists), add the "Namespace-isolation `fail` block" that section describes — it's not optional, it's how CI catches accidental cross-version advice matching.
 
 ### 3. Latest dependency test (mandatory)
 
-If the library's API surface changes across minor versions (deprecated/removed methods, changed signatures), see [Writing Tests](references/tests.md)'s "Version-sensitive tests belong in a separate `latestDepTest` source set" section for which tests belong in `src/test/` vs `src/latestDepTest/`.
+If the library's API surface changes across minor versions (deprecated/removed methods, changed signatures), see [tests-style.md](references/tests-style.md)'s "Version-sensitive tests belong in a separate `latestDepTest` source set" section for which tests belong in `src/test/` vs `src/latestDepTest/`.
 
 Use `latestDepTestImplementation` in `build.gradle` to pin the latest available version. Run with:
 ```bash

.agents/skills/apm-integrations/references/tests-style.md

@@ -0,0 +1,148 @@
+# Test Style & Situational Rules
+
+> Referenced from [tests.md](tests.md). These rules are situational — read the specific section you need, not the whole file top to bottom.
+
+## Error test example
+
+```groovy
+// Example error test (Groovy/Spock)
+def "exception sets error tags"() {
+    when:
+    client.execute(badRequest)
+
+    then:
+    thrown(SomeException)
+    assertTraces(1) {
+        trace(1) {
+            span {
+                errored true
+                errorTags(SomeException, "expected error message")
+            }
+        }
+    }
+}
+```
+
+## compileOnly and testImplementation may use different versions — explain why
+
+When `compileOnly` and `testImplementation` use different versions of the same library, add a
+comment that explains the specific API or class that requires the higher version, and why.
+Do not just state the fact — state the reason.
+
+```groovy
+// WRONG — states the fact without explaining why
+// compileOnly=2.3, testImplementation=2.4
+compileOnly group: 'com.sparkjava', name: 'spark-core', version: '2.3'
+testImplementation group: 'com.sparkjava', name: 'spark-core', version: '2.4'
+
+// CORRECT — explains the specific class and why it requires the higher version
+// compileOnly=2.3 (module targets this version) but testImplementation=2.4:
+// JettyHandler, which Spark uses internally to dispatch HTTP requests to route handlers,
+// is not accessible as a public class in 2.3 — it was exposed starting in 2.4.
+// The instrumentation hooks into JettyHandler via Jetty's existing instrumentation,
+// so tests require 2.4 at minimum to exercise the code path.
+compileOnly group: 'com.sparkjava', name: 'spark-core', version: '2.3'
+testImplementation group: 'com.sparkjava', name: 'spark-core', version: '2.4'
+```
+
+**How to discover this during development**: install the library at the `compileOnly` version and run
+your instrumentation test. If a specific class raises `ClassNotFoundException` or is inaccessible, that
+class is the reason — check when it became public and use that version for `testImplementation`.
+Name the class in the comment.
+
+## Include sibling version modules in testImplementation for mutual exclusion
+
+When two modules instrument the same library at non-overlapping version ranges, each module should include the other as a `testImplementation` dependency to confirm they don't double-instrument. The rule is symmetric — both the older and the newer module should carry this dependency:
+
+```groovy
+// jedis-3.0/build.gradle
+dependencies {
+    testImplementation project(':dd-java-agent:instrumentation:jedis:jedis-1.4')
+}
+
+// jedis-1.4/build.gradle
+dependencies {
+    testImplementation project(':dd-java-agent:instrumentation:jedis:jedis-3.0')
+}
+```
+
+This ensures `:test` in each module validates that only the correct module fires for its version range.
+
+## Embedded servers use a static field — do not recreate per test
+
+For tests that start an embedded server (Jetty, Netty, Undertow, Spark, etc.), initialize the server once as a `@Shared` or `static` field and reuse it across test methods. Do NOT construct a new server in each `setup:` / `@Before` unless you have a concrete reason (e.g. per-test configuration). Recreating the server per test multiplies test wall-time and adds a startup-race surface for no benefit. Follow the pattern of existing server-instrumentation tests in the same framework family.
+
+## Factor shared test scaffolding into a base class
+
+If two sibling test classes (e.g. `FooTest` and `FooForkedTest`) need the same setup, request builder, or assertion helpers, extract them into a shared abstract base — do NOT copy-paste between the two files. Duplicated helper code across a handful of test classes is how bespoke JUnit scaffolding metastasizes across the codebase.
+
+## ForkedTest variants must have a concrete isolation reason
+
+The `ForkedTest` suffix runs a test in its own JVM via the `forkedTest` task. Only add a `ForkedTest` variant when the test genuinely needs JVM isolation — e.g. a system property that must be set before class-loading, an agent-level configuration that cannot be reset between tests, or a class-loader state that leaks. Do NOT mechanically add a `ForkedTest` alongside every `Test` class; each fork adds JVM startup cost to CI.
+
+State the isolation reason in a comment on the `ForkedTest` class.
+
+## Version-sensitive tests belong in a separate latestDepTest source set
+
+For libraries whose API surface changes across minor versions (Reactor deprecates and removes APIs; Netty changes handler signatures; gRPC's generated code evolves), route each test to the source set whose classpath actually satisfies its imports. First check how the module wires `latestDepTest` in `build.gradle`:
+
+- **`addTestSuite('latestDepTest')`** — `latestDepTest` has its own sources at `src/latestDepTest/`, separate from `src/test/`. In this shape: put latest-only APIs (added after your pinned min) in `src/latestDepTest/`; put removed-in-latest APIs (e.g. Reactor's `Schedulers.elastic()`, removed in 3.5+) in `src/test/`, testing the replacement API (`Schedulers.boundedElastic()`) in `latestDepTest/` instead.
+- **`addTestSuiteForDir('latestDepTest', 'test')`** — `latestDepTest` reuses `src/test/`'s sources and compiles them against the latest classpath too. In this shape, `src/test/` is NOT a safe place for a removed-in-latest-API test — it still gets compiled against `latestDepTestImplementation` and will fail the same way. A test that exercises a removed API needs the module to declare a real separate `latestDepTest` source set instead (switch to `addTestSuite('latestDepTest')`), or the test needs to avoid the removed API entirely (e.g. call the replacement API and assert equivalent behavior).
+
+Common libraries where this split matters: Reactor, Netty, gRPC, Kafka clients (consumer API changed at 3.0), Cassandra driver (3.x vs 4.x largely incompatible).
+
+**Editing an existing module:** check for `src/latestDepTest/` and the exact `addTestSuite(...)`/`addTestSuiteForDir(...)` declaration in `build.gradle` before touching tests, and preserve whichever shape master uses.
+
+## No banner/separator comments in test files
+
+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:
+
+```java
+// Java example, style only — the same DSL rule in tests.md applies regardless of language
+// ❌ Banner comments
+class RxJava3ResultExtensionTest extends AbstractInstrumentationTest {
+  // ---------------------------------------------------------------------------
+  // Successful async completion: span finishes when reactive type completes
+  // ---------------------------------------------------------------------------
+  @ParameterizedTest
+  void successfulCompletion(...) { ... }
+
+  // ---------------------------------------------------------------------------
+  // Error paths: span records error and finishes
+  // ---------------------------------------------------------------------------
+  @ParameterizedTest
+  void errorPath(...) { ... }
+}
+
+// ✅ Either omit the banner
+class RxJava3ResultExtensionTest extends AbstractInstrumentationTest {
+  @ParameterizedTest
+  void successfulCompletion(...) { ... }
+
+  @ParameterizedTest
+  void errorPath(...) { ... }
+}
+
+// OR extract into focused classes with class Javadoc
+/**
+ * Successful async completion — verifies the extension finishes the span
+ * when the reactive type emits a terminal signal.
+ */
+class RxJava3ResultExtensionSuccessTest extends AbstractInstrumentationTest {
+  @ParameterizedTest
+  void successfulCompletion(...) { ... }
+}
+
+/**
+ * Error paths — verifies the extension records the error and finishes the span
+ * when the reactive type emits an onError signal.
+ */
+class RxJava3ResultExtensionErrorTest extends AbstractInstrumentationTest {
+  @ParameterizedTest
+  void errorPath(...) { ... }
+}
+```
+
+Source: @ygree review on PR #11939.