skill(apm-integrations): split tests.md into mandatory rules + situational style guide - #12410
skill(apm-integrations): split tests.md into mandatory rules + situational style guide#12410jordan-wong wants to merge 1 commit into
Conversation
…ional style guide tests.md mixed always-applicable rules (DSL choice, error-test coverage, no-sleep hygiene) with situational rules that only apply to specific circumstances (version-mismatch comments, mutual-exclusion deps, latestDepTest source-set routing, banner-comment style) and their code examples — 179 lines every reader loaded via SKILL.md's Step 9.1 pointer, regardless of whether their module hit any of those cases. Split into: - tests.md (28 lines) — the mandatory checklist: DSL rule, error-test requirement, ForkedTest naming, integration-name registration, no-Thread.sleep/no-default-jvmArgs hygiene. This is what SKILL.md Step 9.1 points to. - tests-style.md (148 lines) — situational rules + all code examples, linked from tests.md for the specific cases that need them. Also folds in the DSL-rule clarification from #12359 ("Full Java instrumentation test support is not yet available" read as an absolute ban that the repo's own Java-DSL test suites contradict — restated as unconditional-unless-already-Java-DSL). Updated SKILL.md's two references/tests.md pointers (Step 9.1, Step 9.3's latestDepTest pointer) to match the new file split. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
jordan-wong
added
tag: ai generated
|
🎯 Code Coverage (details) 🔗 Commit SHA: 3a927df | Docs | View more details | Give us feedback! |
🟢 Java Benchmark SLOs — All performance SLOs passed
PR vs. master results
Commit: Load and DaCapo benchmarks can be triggered manually in the GitLab pipeline. Results will appear in the Benchmarking Platform UI after completion. |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![datadog-prod-us1-5[bot]](https://avatars.githubusercontent.com/u/365230?s=60&v=4){kind=small}
There was a problem hiding this comment.
More details
The split keeps the mandatory rules in tests.md. It moves the situational rules to tests-style.md and updates all affected links.
🤖 Datadog Autotest · Commit 3a927df · What is Autotest? · @DataDog review to ask questions · Any feedback? Reach out in #autotest
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3a927df5b8
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| ## 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. Java examples in [tests-style.md](tests-style.md) are style-only illustrations for modules ALREADY on the Java/JUnit DSL — NOT a license to introduce Java into a Groovy family. Adding new `.groovy` files to a PR will trigger the `Enforce Groovy Migration` bot — add the `tag: override groovy enforcement` label to bypass it. |
There was a problem hiding this comment.
When the skill edits an instrumentation module already using Java/JUnit, this sentence gives mutually exclusive directions: it declares Groovy/Spock unconditional, but later recognizes modules already on the Java/JUnit DSL. Java instrumentation tests are explicitly supported by docs/how_to_test_with_junit.md:284-287, while .github/workflows/enforce-groovy-migration.yaml:70-102 rejects newly added Groovy files and recommends Java/JUnit unless an exception is justified. Clarify that an existing Java module should remain Java, and reserve Groovy plus the override label for the intended exceptional cases.
AGENTS.md reference: AGENTS.md:L64-L65
Useful? React with 👍 / 👎.
1 participant
🤖 Generated with APM Instrumentation Toolkit
Reference / skill-fix PR. Two
apm-integrationsskill files; docs-only, no instrumentation code changes.Splits
tests.mdso the rules every module needs aren't buried in the ones only some modules hit — restructuring alternative to #12359's inline trim, opened for comparison.Changes
references/tests.mdmixed always-applicable rules (DSL choice, error-test coverage, no-sleep hygiene) with situational rules and code examples that only apply to specific circumstances (version-mismatch comments, mutual-exclusion deps,latestDepTestsource-set routing, banner-comment style) — 179 lines every reader loaded viaSKILL.mdStep 9.1's pointer, regardless of whether their module hit any of those cases.tests.md(28 lines) — the mandatory checklist only: DSL rule, error-test requirement,ForkedTestnaming, integration-name registration, no-Thread.sleep()/no-default-jvmArgshygiene.tests-style.md(148 lines, new) — situational rules + all code examples, linked fromtests.mdfor the specific cases that need them.Also folds in the DSL-rule clarification from #12359 (the old "Full Java instrumentation test support is not yet available" read as an absolute ban the repo's own Java-DSL test suites contradict).
Scope
references/tests.md,references/tests-style.md(new),SKILL.md— docs-only.Alternative to #12359 — pick one; they overlap on the DSL-rule content.