From bc8fb152590f25b953620a1c620873ffcda21252 Mon Sep 17 00:00:00 2001 From: Pieter Noordhuis Date: Wed, 3 Jun 2026 13:29:46 +0200 Subject: [PATCH 1/4] acceptance: document LOG.* files, gron.py over jq, --keep gotcha, and EnvMatrix scoping MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Four idioms surfaced in PR #5414 review that weren't captured in the acceptance-test guide: - Limit `EnvMatrix.DATABRICKS_BUNDLE_ENGINE` when the test inspects engine-specific state shape (resources.json vs terraform.tfstate). - Prefer `gron.py | grep ` over inline `jq` for single-value lookups — the gron output line includes the JSON path, so the test log is self-explanatory. - Don't pass `--keep` to print_requests.py if a later call follows; the second call re-reads the buffer and double-prints. - Use `LOG.` for cleanup-noise stderr that should show up only under `go test -v` (not dropped with `2>/dev/null`). Each rule has a GOOD / BAD example and points at existing tests that already follow the pattern. Co-authored-by: Isaac --- .agent/rules/testing.md | 34 ++++++++++++++++++++++++++++++++++ 1 file changed, 34 insertions(+) diff --git a/.agent/rules/testing.md b/.agent/rules/testing.md index 67f41cd2ea2..4328c8daa16 100644 --- a/.agent/rules/testing.md +++ b/.agent/rules/testing.md @@ -87,6 +87,8 @@ acceptance/cmd/fs/cp/file-to-dir/ If the only reason for divergence is a server-side default that one engine sets and the other doesn't, set the field explicitly in `databricks.yml` so both engines produce identical output. Don't paper over it with per-engine files. +**RULE: Limit `EnvMatrix.DATABRICKS_BUNDLE_ENGINE` when the test exercises engine-specific behavior.** Direct's `PrepareState` / `resources.json` shape and terraform's drift loop / `.tfstate` shape are not shared. If a test inspects either, set the matrix to just the relevant engine and leave a one-line `test.toml` comment explaining why. Cross-engine parity of the contract (request/response shape, destroy semantics) belongs in a sibling test that exercises both. + ### Reference - Tests live in `acceptance/` with a nested directory structure. @@ -132,6 +134,38 @@ Available on `PATH` during test execution (from `acceptance/bin/`): - `gron.py`: flatten JSON into greppable discrete assignments (simpler than `jq` for searching JSON). - `jq` is also available for JSON processing. +**RULE: Prefer `gron.py | grep ` over inline `jq` paths for single-value lookups.** The gron form prints the full assignment (e.g. `json.state["resources.foo.bar"].state.purge_on_delete = true;`), so the path is self-evident in the test output and reviewers don't have to mentally walk a jq expression. + +GOOD: + +```bash +gron.py < .databricks/bundle/default/resources.json | grep purge_on_delete +``` + +BAD: + +```bash +jq -r '.state["resources.postgres_projects.proj"].state.purge_on_delete' .databricks/bundle/default/resources.json +``` + +**RULE: Don't pass `--keep` to `print_requests.py` if you make a later `print_requests.py` call in the same test.** `--keep` retains `out.requests.txt`, so the next call re-reads the same accumulated buffer and double-prints earlier requests. Use `--keep` only when this is the last invocation AND something else (e.g. a separate `jq`) needs to read the file afterward. + +### Diagnostic output + +**RULE: For cleanup-noise or other stderr a developer might want to see under `go test -v`, append to a `LOG.` file instead of dropping it with `2>/dev/null`.** Files named `LOG` or `LOG.` are surfaced by the test runner only in verbose mode (see `acceptance/selftest/log/`), and they aren't part of the diffed output so they don't trigger spurious failures. + +GOOD: + +```bash +$CLI postgres delete-project --purge "projects/$NAME" 2>>LOG.delete-project || true +``` + +BAD: + +```bash +$CLI postgres delete-project --purge "projects/$NAME" 2>/dev/null || true +``` + ### Update workflow **RULE: Run `./task test-update` to regenerate outputs, then `./task fmt` and `./task lint`.** If fmt or lint modify files in `acceptance/`, there's an issue in the source files. Fix the source, regenerate, and verify fmt/lint pass cleanly. From ddf3ea793d7e437eda3757df87465edcf9c1700f Mon Sep 17 00:00:00 2001 From: Pieter Noordhuis Date: Mon, 8 Jun 2026 09:26:07 +0200 Subject: [PATCH 2/4] acceptance: condense testing rules and drop EnvMatrix engine-scoping rule Tighten the gron-over-jq, print_requests --keep, and LOG. rules from multi-block GOOD/BAD examples to one-liners. Drop the EnvMatrix engine-scoping rule per maintainer feedback. Co-authored-by: Isaac --- .agent/rules/testing.md | 34 +++------------------------------- 1 file changed, 3 insertions(+), 31 deletions(-) diff --git a/.agent/rules/testing.md b/.agent/rules/testing.md index 4328c8daa16..8ccb513f167 100644 --- a/.agent/rules/testing.md +++ b/.agent/rules/testing.md @@ -87,8 +87,6 @@ acceptance/cmd/fs/cp/file-to-dir/ If the only reason for divergence is a server-side default that one engine sets and the other doesn't, set the field explicitly in `databricks.yml` so both engines produce identical output. Don't paper over it with per-engine files. -**RULE: Limit `EnvMatrix.DATABRICKS_BUNDLE_ENGINE` when the test exercises engine-specific behavior.** Direct's `PrepareState` / `resources.json` shape and terraform's drift loop / `.tfstate` shape are not shared. If a test inspects either, set the matrix to just the relevant engine and leave a one-line `test.toml` comment explaining why. Cross-engine parity of the contract (request/response shape, destroy semantics) belongs in a sibling test that exercises both. - ### Reference - Tests live in `acceptance/` with a nested directory structure. @@ -134,37 +132,11 @@ Available on `PATH` during test execution (from `acceptance/bin/`): - `gron.py`: flatten JSON into greppable discrete assignments (simpler than `jq` for searching JSON). - `jq` is also available for JSON processing. -**RULE: Prefer `gron.py | grep ` over inline `jq` paths for single-value lookups.** The gron form prints the full assignment (e.g. `json.state["resources.foo.bar"].state.purge_on_delete = true;`), so the path is self-evident in the test output and reviewers don't have to mentally walk a jq expression. - -GOOD: - -```bash -gron.py < .databricks/bundle/default/resources.json | grep purge_on_delete -``` - -BAD: - -```bash -jq -r '.state["resources.postgres_projects.proj"].state.purge_on_delete' .databricks/bundle/default/resources.json -``` - -**RULE: Don't pass `--keep` to `print_requests.py` if you make a later `print_requests.py` call in the same test.** `--keep` retains `out.requests.txt`, so the next call re-reads the same accumulated buffer and double-prints earlier requests. Use `--keep` only when this is the last invocation AND something else (e.g. a separate `jq`) needs to read the file afterward. - -### Diagnostic output - -**RULE: For cleanup-noise or other stderr a developer might want to see under `go test -v`, append to a `LOG.` file instead of dropping it with `2>/dev/null`.** Files named `LOG` or `LOG.` are surfaced by the test runner only in verbose mode (see `acceptance/selftest/log/`), and they aren't part of the diffed output so they don't trigger spurious failures. +**RULE: Prefer `gron.py | grep ` over inline `jq` paths for single-value lookups.** The gron output prints the JSON path inline, so the test log explains itself. -GOOD: +**RULE: Don't pass `--keep` to `print_requests.py` if a later `print_requests.py` call follows.** The buffer accumulates, so the second call double-prints the earlier requests. -```bash -$CLI postgres delete-project --purge "projects/$NAME" 2>>LOG.delete-project || true -``` - -BAD: - -```bash -$CLI postgres delete-project --purge "projects/$NAME" 2>/dev/null || true -``` +**RULE: Send cleanup-noise stderr to `LOG.` instead of `/dev/null`.** `LOG.*` files surface under `go test -v` but stay out of the diffed output — see `acceptance/selftest/log/`. Example: `$CLI postgres delete-project --purge ... 2>>LOG.delete-project || true`. ### Update workflow From da6d1d4008e470eb479a2c3535c31e47540836da Mon Sep 17 00:00:00 2001 From: Pieter Noordhuis Date: Mon, 8 Jun 2026 09:26:16 +0200 Subject: [PATCH 3/4] acceptance: scope testing rules to test files via paths frontmatter Adds path globs so the rules auto-load when Claude reads test files, matching how .agent/rules/dresources.md auto-loads under bundle/direct/dresources. Co-authored-by: Isaac --- .agent/rules/testing.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/.agent/rules/testing.md b/.agent/rules/testing.md index 8ccb513f167..b37fab72857 100644 --- a/.agent/rules/testing.md +++ b/.agent/rules/testing.md @@ -1,5 +1,9 @@ --- description: Rules for the testing strategy of this repo +paths: + - "**/*_test.go" + - "acceptance/**" + - "integration/**" --- # Rules for the testing strategy of this repo From f777c3d9fd265aa8e4a7a5550532597c0fb7e024 Mon Sep 17 00:00:00 2001 From: Pieter Noordhuis Date: Mon, 8 Jun 2026 09:42:14 +0200 Subject: [PATCH 4/4] acceptance: broaden LOG. rule to cover both deploy-style capture and cleanup stderr The dominant real-world pattern in acceptance/bundle/invariant/* is `&> LOG.deploy` paired with `contains.py '!panic'` to keep noisy non-deterministic output out of `output.txt` while still asserting invariants. Cover that as the primary case, with `2>>LOG.` for cleanup-step stderr as the secondary case. Co-authored-by: Isaac --- .agent/rules/testing.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.agent/rules/testing.md b/.agent/rules/testing.md index b37fab72857..77173ff6506 100644 --- a/.agent/rules/testing.md +++ b/.agent/rules/testing.md @@ -140,7 +140,7 @@ Available on `PATH` during test execution (from `acceptance/bin/`): **RULE: Don't pass `--keep` to `print_requests.py` if a later `print_requests.py` call follows.** The buffer accumulates, so the second call double-prints the earlier requests. -**RULE: Send cleanup-noise stderr to `LOG.` instead of `/dev/null`.** `LOG.*` files surface under `go test -v` but stay out of the diffed output — see `acceptance/selftest/log/`. Example: `$CLI postgres delete-project --purge ... 2>>LOG.delete-project || true`. +**RULE: Route noisy or non-deterministic command output to `LOG.` instead of `output.txt` or `/dev/null`.** `LOG.*` files are visible under `go test -v` but excluded from the diff — see `acceptance/selftest/log/`. Use `&> LOG.` to capture both streams (then `contains.py` to assert invariants like `'!panic' '!internal error'`), or `2>>LOG.` for cleanup-step stderr you'd otherwise drop to `/dev/null`. ### Update workflow