Repository navigation
chore(docs): add the test-audit skill for authoring and auditing tests - #475
Conversation
Adds an agent skill under .claude/skills/test-audit that gates new tests at write time and audits existing ones for low-value, duplicated, or implementation-coupled tests and the test-only production seams they keep alive. The skill is adapted from openclaw/openclaw (MIT) at commit eb1b48c58b61dec3cc273587d44e4f9a7eaa29f9; VENDOR.md records the origin. The openclaw-specific parts are replaced with this repository's test tiers and CI routing (TIERS.md), snapshot rules, the contracts worth retaining, and Nx/jest validation commands. CLAUDE.md recommends the skill when adding or changing tests, no longer points at a constitution path that is not in the repository, and lists the PR title types and scopes that pr-lint.yml allows. CONTRIBUTING.md gains the perf, test and revert types and drops a `pnpm test:watch` script that does not exist. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…it skill SKILL.md and CAMPAIGN.md are adapted from MIT-licensed OpenClaw material, and the MIT license requires its copyright and permission notice to accompany copies or substantial portions. Adds the upstream LICENSE, verbatim from the pinned commit, next to the skill and references it from VENDOR.md. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Non-blocking content notes from comparing
My rubric, very briefly: test observable behavior and credible regressions at the cheapest durable boundary; ensure tests run in normal validation; question skips, static scans, duplicate coverage, and test-only seams; require evidence and remaining keeper coverage before deleting anything. |
- Snapshots: say to update only the intended snapshot file, and that the repository-wide update commands are for deliberate migrations where every changed entry is still reviewed, instead of forbidding them. - Campaign reconcile: re-evaluate and port what main added to a retired file before keeping the deletion. - Tiers: a contract has one primary owning tier, matching the authoring gate, which allows another tier for a distinct risk. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Thanks, all three taken in 20543f6.
|
jsteinich
left a comment
There was a problem hiding this comment.
Non-blocking review. I checked the skill's factual claims against the workflows and tooling; most hold (Nx tags, the pr-unit.yml matrix, the integration retry, the Testing.app future-flag default, the hcl2cdk CI gate, the test:update targets). Inline notes below on the ones that don't.
One more outside the diff: this PR aligns CLAUDE.md and CONTRIBUTING.md with pr-lint's allowed types, but subjectPatternError in .github/workflows/pr-lint.yml still says the supported types are only feat, fix, chore, refactor and revert. A contributor with a malformed title is told test and perf are unsupported. Worth fixing at the source alongside the docs.
🤖 Generated with Claude Code
- TIERS.md: CI packages before unit tests only when @cdktn/hcl2cdk or @cdktn/cli-core is affected (and only for hcl2cdk on main), so a dist-gated suite such as cdktn-cli's init test can skip in CI. SKILL.md no longer says CI covers the rest. - TIERS.md: compute the affected base from the open-constructs remote with merge-base, so the command is right from a fork. - TIERS.md: the provider matrix builder checks no pins; only the integration and example builders do. - TIERS.md: say what runs on main for the unit tier, which leaves out @cdktn/hcl-tools. - SKILL.md: add @cdktn/hcl-tools to the discovery lanes. - CLAUDE.md: show revert with a scope, as CONTRIBUTING.md does. - pr-lint.yml: list test and perf in the title error message. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Thanks for checking the claims against the workflows. All six inline notes are addressed in 80f134c, with a reply on each thread. The note outside the diff is fixed in the same commit: Two things the review surfaced are documented here but not fixed, since they are workflow changes and not docs:
|
### Description Depot runners set `NX_SELF_HOSTED_REMOTE_CACHE_SERVER`, so Nx tasks on `depot-*` runners read and write the Depot remote cache. The five `*-documentation` examples run there, and their `build` target (`cdktn get`) started getting `[remote cache]` hits: ``` nx run @examples/go-documentation:build [remote cache] ... nx run @examples/go-documentation:synth ERROR: synthesis failed, run "cdktn get" to generate providers in generated ``` Two things are wrong with caching example builds at all: - The example projects have no Nx dependency edge to `cdktn`, so their task hash covers only the example's own files, not the dist bundle the job installs. A cache hit replays bindings generated against whatever build was under test when the entry was stored. - The generated bindings directory (`generated/`, `.gen/`, `imports/`) is not a declared output, so nothing is restored and `synth` fails. Declaring outputs would fix the second point but keep the first, so this passes `--skip-nx-cache` to the single `nx run-many` call in `tools/build-example.mjs`. It covers every example and every target the runner invokes. The core packages keep their cache; #474 made their outputs complete. Seen on the Release runs for #467, #445 and #475 (a rotating set of documentation examples depending on which keys were warm). Draft until #476 lands; will rebase then. ### Checklist - [x] I have updated the PR title to match [CDKTN's style guide](https://github.com/open-constructs/cdk-terrain/blob/main/CONTRIBUTING.md#pull-requests-1) - [x] I have performed a self-review of my code - [ ] I have made corresponding changes to the documentation if applicable - [x] My changes generate no new warnings 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_01RvtnNBvNuTHU4dG3NFuBeb Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com>
#478) ### Description The `pip` variant of `python/provider-add-command` and `python/provider-upgrade-command` installs `cdktn` in `beforeAll` without awaiting the call, while the `pipenv` block above does await its equivalent. `TestDriver.exec` returns a promise, so the first test can run `cdktn debug` before pip has finished: ``` External Error: Could not determine cdktf/cdktn version. Please make sure you are in a directory containing a cdktn project and have all dependencies installed. at TestDriver.exec (test-helper.ts:171:17) at Object.<anonymous> (python/provider-add-command/test.ts:89:21) ✕ detects correct cdktn version (1364 ms) Tests: 1 failed, 3 passed, 4 total ``` Later tests in the block pass once pip completes, which is why exactly one test fails. The slower Terraform 1.5.7 matrix entry loses the race consistently; seen on the Release runs for #445 and #475. #188 added `--no-compile` to the same line but left it un-awaited, and upstream terraform-cdk has the same call. Adds the missing `await` in both files. Draft until #476 lands; will rebase then. ### Checklist - [x] I have updated the PR title to match [CDKTN's style guide](https://github.com/open-constructs/cdk-terrain/blob/main/CONTRIBUTING.md#pull-requests-1) - [x] I have performed a self-review of my code - [ ] I have made corresponding changes to the documentation if applicable - [x] My changes generate no new warnings 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_01RvtnNBvNuTHU4dG3NFuBeb Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com>
🤖 Release PR — merge to cut a new release. Kept open and rebased as commits land on `main`. --- <details><summary>0.25.0</summary> ## [0.25.0](v0.24.0...v0.25.0) (2026-10-27) ### ⚠ BREAKING CHANGES * **lib:** `IAssetPackaging` gains `acceptsDirectorySource` as a required member. Custom implementations of the interface must add it. * **lib:** ValidateBinaryVersion and ValidateTerraformVersion are removed (they were only reachable through deep imports, not the cdktn package root). Synth no longer executes the terraform/tofu binary for feature validation; set validateInstalledBinary: true in cdktf.json to verify the installed binary before commands that run it. * **lib:** resource moves (moved blocks) are now validated at synth against the project's declared targetVersions (default terraform >=1.5.7, opentofu >=1.6.0) instead of the locally installed binary. Projects that declare terraform targets below 1.5.0 now fail synth; projects whose installed binary is older than their declared targets should set validateInstalledBinary: true in cdktf.json. * **lib:** import blocks (importFrom / ImportableResource) are now validated at synth against the project's declared targetVersions (default terraform >=1.5.7, opentofu >=1.6.0) instead of the locally installed binary. Projects that declare terraform targets below 1.5.0 now fail synth; projects whose installed binary is older than their declared targets should set validateInstalledBinary: true in cdktf.json. * **lib:** terraform_data resources are now validated at synth against the project's declared targetVersions (default terraform >=1.5.7, opentofu >=1.6.0) instead of the locally installed binary. Projects that declare terraform targets below 1.4.0 now fail synth; projects whose installed binary is older than their declared targets should set * **lib:** cloud backends (the cloud block) are now validated at synth against the project's declared targetVersions (default terraform >=1.5.7, opentofu >=1.6.0) instead of the locally installed binary. Projects that declare terraform targets below 1.1.0 now fail synth; projects whose installed binary is older than their declared targets should set validateInstalledBinary: true in cdktf.json. * **lib:** IAssetPackaging gains a required member omitsDirectoryEntries; external implementations must add it. Custom TerraformAsset assetHash values are now validated against `/^[A-Za-z0-9_.-]+$/` and throw if they contain other characters (e.g. "feature/my-branch"). * **lib:** add asset pipeline foundation types and hash utilities ([#371](#371)) ### Features * **cli:** add a typescript-pnpm init template and deprecate Yarn ([#422](#422)) ([81da164](81da164)) * **cli:** move the TypeScript template to tsx and TypeScript 6 ([#396](#396)) ([b50349d](b50349d)) * **cli:** point AI coding agents at the CDKTN docs MCP server ([#518](#518)) ([f257cd9](f257cd9)) * **cli:** rename `--cdktf-version` to `--cdktn-version` in `init` ([#228](#228)) ([dd3818e](dd3818e)) * **cli:** replace HashiCorp checkpoint telemetry with Sentry usage metrics and consent ([#411](#411)) ([e17046a](e17046a)) * **cli:** report aggregated stack, provider and command usage metrics ([#413](#413)) ([ed9b1e5](ed9b1e5)) * **cli:** report the installed binary, target versions, platform and package manager in usage metrics ([#412](#412)) ([bfe26a8](bfe26a8)) * **cli:** resolve providers from the registry the project targets ([#445](#445)) ([78898ae](78898ae)) * **deps:** adopt jsii 6.0 and TypeScript 6 ([#395](#395)) ([26fa8fa](26fa8fa)) * **gha:** Revert "run the affected unit tests against OpenTofu ([#439](#439))" ([#446](#446)) ([ae015ef](ae015ef)) * **gha:** run the affected unit tests against OpenTofu ([#439](#439)) ([823682d](823682d)) * **gha:** run the affected unit tests against OpenTofu ([#447](#447)) ([c0c4c1c](c0c4c1c)) * **lib:** add asset bundling extension interface ([#442](#442)) ([7839625](7839625)) * **lib:** add asset pipeline foundation types and hash utilities ([#371](#371)) ([a008d12](a008d12)) * **lib:** add asset staging core with no bundling ([#433](#433)) ([44d8bd1](44d8bd1)) * **lib:** add support for single-file bundler outputs ([#464](#464)) ([3592db0](3592db0)) * **lib:** pass ignore strategy through terraform asset ([#472](#472)) ([705e5b9](705e5b9)) * **tests:** add per-test runtime pinning to the integration matrix ([#405](#405)) ([17c2278](17c2278)) ### Bug Fixes * **cli:** catch failures of the background environment collection ([#531](#531)) ([c5feed8](c5feed8)) * **cli:** correct the constructs range comment in the TypeScript template hook ([#408](#408)) ([9fc0e43](9fc0e43)) * **cli:** don't crash after a successful deploy when an output is missing ([#375](#375)) ([0bbd64a](0bbd64a)) * **cli:** fail cleanly when the --outputs-file write fails ([#376](#376)) ([8f9d03a](8f9d03a)) * **cli:** make `cdktn get` remove stale files for resources removed from a provider ([#421](#421)) ([d6e28dc](d6e28dc)) * **cli:** match prebuilt providers regardless of the project's registry ([#493](#493)) ([ea82124](ea82124)) * **cli:** probe with `version` instead of `-v` ([#471](#471)) ([b87ee22](b87ee22)) * **cli:** require patched Node 22.23.4 and bump undici for security fixes ([#516](#516)) ([76436f5](76436f5)) * **cli:** resolve bundled handlers next to the entrypoint ([#414](#414)) ([49600c1](49600c1)) * **cli:** resolve upgraded providers from the CLI's own registry ([#489](#489)) ([10672ee](10672ee)) * **cli:** stop the top-level error handler racing yargs ([#410](#410)) ([5d0c2ef](5d0c2ef)) * **deps:** move jsii-rosetta to 6.0.16 for stream-json 3 ([#463](#463)) ([f1ec16f](f1ec16f)) * **gha:** authenticate to public.ecr.aws in the docker image build ([#519](#519)) ([c65bf2f](c65bf2f)) * **gha:** don't fail the pnpm upgrade jobs on ignored dep builds ([#399](#399)) ([5b66342](5b66342)) * **gha:** move the jsii package cache out of the container HOME ([#514](#514)) ([df5b490](df5b490)) * **gha:** pass -R to gh pr edit in release_github relabel step ([#372](#372)) ([358edc7](358edc7)) * **gha:** push only the prerelease tag from prepare-next ([#476](#476)) ([848f2a6](848f2a6)) * **lib:** avoid terraform init plugin-cache race in test matchers ([#467](#467)) ([26847f9](26847f9)) * **lib:** retry the staging install without --prefer-offline outside CI ([#407](#407)) ([7bb26e7](7bb26e7)) * **provider-generator:** pass the default provider configuration a module requires on get ([#466](#466)) ([a5225ab](a5225ab)) * **provider-generator:** pick the docs registry from targetVersions ([#443](#443)) ([00f2884](00f2884)) * **provider-generator:** pick the module docs registry from targetVersions ([#484](#484)) ([8200330](8200330)) * **provider-generator:** regenerate bindings when the target registry changes ([#490](#490)) ([813e022](813e022)) * **provider-generator:** satisfy module provider configuration aliases on get ([#383](#383)) ([54a8217](54a8217)) * **provider-generator:** stop a provider_functions resource clobbering the functions submodule ([#403](#403)) ([cf19de9](cf19de9)) * **tests:** await the pip install in the python provider command tests ([#478](#478)) ([1c83a36](1c83a36)) * **tests:** bound and retry the dist publish to the local registry ([#425](#425)) ([cc9f9e2](cc9f9e2)) * **tests:** build the edge provider bindings in their own cached Nx target ([#482](#482)) ([f480cc9](f480cc9)) * **tests:** drop unused ts-jest transform from shared jest preset ([#382](#382)) ([03f6e16](03f6e16)) * **tests:** mark the terraform-cloud target Terraform-only in the matrix ([#430](#430)) ([b0deefa](b0deefa)) * **tests:** run example builds with the Nx cache skipped ([#477](#477)) ([3e25e13](3e25e13)) * **tools:** declare the cdktn-cli bundle as a cached Nx output with its real inputs ([#485](#485)) ([10dd118](10dd118)) * **tools:** make the workspace build, package, lint and test on Windows ([#364](#364)) ([8dcddbe](8dcddbe)) ### Code Refactoring * **lib:** Remove the ValidateBinaryVersion synth-time binary probe ([#457](#457)) ([e60142e](e60142e)) * **lib:** Use declared targetVersions for synth-time validation of import blocks ([#454](#454)) ([c0097f4](c0097f4)) * **lib:** Use declared targetVersions for synth-time validation of resource moves ([#453](#453)) ([43edc1e](43edc1e)) * **lib:** Use declared targetVersions for synth-time validation of terraform_data ([#455](#455)) ([a1c125b](a1c125b)) * **lib:** Use declared targetVersions for synth-time validation of the cloud backend ([#456](#456)) ([4a83507](4a83507)) ### Miscellaneous Chores * add OpenTofu and current Terraform to the jsii-terraform image ([#394](#394)) ([2f29d5a](2f29d5a)) * declare complete Nx build outputs so cache hits restore them ([#474](#474)) ([794cf77](794cf77)) * **deps:** move the remaining packages to TypeScript 6 ([#406](#406)) ([c55d360](c55d360)) * **deps:** move the root TypeScript to 6 ([#417](#417)) ([3cae1e2](3cae1e2)) * **deps:** Update xstate to v5.32.4 ([#333](#333)) ([5ebf74f](5ebf74f)) * **deps:** upgrade @sentry/node to 10.x ([#409](#409)) ([2714fff](2714fff)) * **deps:** use caret ranges for unbundled runtime dependencies ([#503](#503)) ([e722357](e722357)) * **docs:** add the test-audit skill for authoring and auditing tests ([#475](#475)) ([909c612](909c612)) * **examples:** move the TypeScript examples to tsx and TypeScript 6 ([#419](#419)) ([857e182](857e182)) * Flexible python version ([#420](#420)) ([955f62a](955f62a)) * **gha:** adopt the new jsii-terraform image and test Terraform 1.5.7 + 1.16.1 ([#398](#398)) ([3ad760b](3ad760b)) * **gha:** gate dependency upgrades on release age ([#418](#418)) ([b611a87](b611a87)) * **gha:** retire Docker Hub publishing of jsii-terraform ([#388](#388)) ([e53cddf](e53cddf)) * **gha:** select the CLI by binary name instead of concatenating a version ([#438](#438)) ([914da56](914da56)) * **gha:** skip the remote Nx cache in the release workflow ([#487](#487)) ([bce91da](bce91da)) * **lib:** stop publishing build state in the cdktn package ([#397](#397)) ([51e4a95](51e4a95)) * npm-check-updates && pnpm update ([#326](#326)) ([7095f6b](7095f6b)) * **readme:** point to the cdktn-io/action GitHub Action ([#504](#504)) ([e387852](e387852)), closes [#58](#58) * **tests:** add timeout handling to spawned commands ([#432](#432)) ([ddb4a8a](ddb4a8a)) * **tests:** run the function-binding scripts with tsx ([#431](#431)) ([fec67bf](fec67bf)) ### Tests * **lib:** ensure TerraformAsset stage within stack own directory ([#434](#434)) ([9a80366](9a80366)) * **lib:** move matcher Docker fixture to a provider version OpenTofu accepts ([#429](#429)) ([7b0da12](7b0da12)) * **tests:** move the python providers fixture to Docker provider 3.9.0 ([#440](#440)) ([a678473](a678473)) * **tests:** move the remaining kreuzwerker/docker pins off unsigned versions ([#459](#459)) ([d855cc1](d855cc1)) * **tests:** move the testing-matchers fixtures to Docker provider 3.x ([#436](#436)) ([8e2c60e](8e2c60e)) * **tests:** re-enable the Python edge integration test ([#404](#404)) ([0a80626](0a80626)) </details> --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please). Co-authored-by: open-constructs-cdktn[bot] <291052431+open-constructs-cdktn[bot]@users.noreply.github.com>
|
I'm going to lock this pull request because it has been closed for 30 days. This helps our maintainers find and focus on the active issues. If you've found a problem that seems related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further. |
Related issue
None. Docs and agent-tooling only; no package code is touched.
Description
Adds a
test-auditagent skill under.claude/skills/test-audit/. It has two everyday uses and one occasional one:CAMPAIGN.md): prune one package's or one owner area's whole test surface, one lane per PR.Origin
The skill is adapted from the
test-auditskill in openclaw/openclaw (MIT), taken ateb1b48c.VENDOR.mdrecords this.The value bar, junk patterns, candidate evidence and campaign steps are upstream's. What is specific to this repository replaces the openclaw parts (Vitest, plugins, their PR tooling):
TIERS.md: the test tiers (static gates, unit, unit with the Terraform/OpenTofu matrix, integration, provider integration, examples), how CI routes each, what a green run does and does not prove, and the commands.TestingAPI).Doc alignment
CLAUDE.mdrecommends the skill when adding or changing tests. It no longer points at a constitution path that is not in the repository (the principles are stated inline), and it lists the PR title types and scopespr-lint.ymlactually allows.CONTRIBUTING.mdgains theperf,testandreverttypes, and apnpm test:watchscript that does not exist is replaced with the Nx watch command.Findings from trialling it
The skill was run read-only over the diff of #473 (
feat(lib): support a pluggable asset hash algorithm) as a first trial. The result is posted there as a non-blocking review. In short, of the seven tests that PR adds:It also found production paths the diff changes with no test: the per-App hash cache key, OUTPUT hashing with a bundler, the new option on the legacy (flag-off) scheme, a literal pin for default hash values on either scheme, and a missing guard for an out-of-range algorithm.
The trial fed back into the skill before this PR: one statement that turned out to be wrong was removed, the commands were corrected and verified, and the tier and command reference was split out into
TIERS.mdto keepSKILL.mdclose to the upstream's size.Checklist
prettier --checkonCLAUDE.mdandCONTRIBUTING.md;.claude/is prettier-ignored)🤖 Generated with Claude Code