From e2e6fdf8652a1e4534fa104f33740938f1814a59 Mon Sep 17 00:00:00 2001 From: Jan Rose Date: Wed, 15 Jul 2026 11:40:31 +0200 Subject: [PATCH 1/9] acc: add per-file SoftFailFiles to downgrade volatile golden drift Some acceptance goldens drift for reasons outside our control: a live backend rewords a message, a response grows a field we don't assert, or a remotely-hosted template is updated upstream. None are regressions, yet each turns PRs and nightlies red. SoftFailFiles downgrades a content diff in a named golden from a hard failure to a recorded, non-blocking SOFTFAIL marker: - Per-file and opt-in, surfaced in out.test.toml so reviewers see the shield on every PR. output.txt can never be shielded (a hard config error), so a regression in our own logic still turns the test red. - Requires Badness, documenting why each file is shielded. - Only a content diff is downgraded; panics, unexpected exit codes, and missing/extra files stay hard failures. - The doComparison branch runs before AssertEqualTexts, whose testify side effect would otherwise mark the test failed. Drift is reported to the GitHub step summary via tools/softfail_report.py (greps the gotestsum JSON for SOFTFAIL markers) so oncall can refresh the golden with ./task test-update on a cadence without the build turning red. Co-authored-by: Isaac --- .github/workflows/push.yml | 9 ++ acceptance/README.md | 50 ++++++++++ acceptance/acceptance_test.go | 18 +++- acceptance/internal/config.go | 35 +++++++ acceptance/internal/config_test.go | 45 +++++++++ acceptance/internal/materialized_config.go | 4 + acceptance/softfail_test.go | 89 ++++++++++++++++++ tools/softfail_report.py | 103 +++++++++++++++++++++ 8 files changed, 349 insertions(+), 4 deletions(-) create mode 100644 acceptance/softfail_test.go create mode 100644 tools/softfail_report.py diff --git a/.github/workflows/push.yml b/.github/workflows/push.yml index 8f5f981e6b8..36ebf1fd462 100644 --- a/.github/workflows/push.yml +++ b/.github/workflows/push.yml @@ -152,6 +152,15 @@ jobs: if-no-files-found: warn retention-days: 7 + - name: Report soft-failed acceptance goldens + # Non-blocking: surfaces goldens that drifted but did not fail the build, + # so oncall can refresh them on a cadence. Runs even when tests failed. + if: ${{ always() }} + run: | + if [ -f test-output.json ]; then + python3 tools/softfail_report.py test-output.json >> "$GITHUB_STEP_SUMMARY" + fi + - name: Check no files changed or appeared after running tests run: | # Register untracked files with intent-to-add so `git diff` reports them diff --git a/acceptance/README.md b/acceptance/README.md index 7277481327b..ac2196a0c18 100644 --- a/acceptance/README.md +++ b/acceptance/README.md @@ -26,6 +26,56 @@ Any file starting with "LOG" will be logged to test log (visible with go test -v See [selftest](./selftest) for more examples. +## Soft-failing volatile goldens + +Some goldens drift for reasons outside our control: a live backend rewords a message, a +response grows a field we don't assert, or a remotely-hosted template is updated upstream. +None of these are regressions, yet each turns PRs and nightlies red. `SoftFailFiles` +downgrades a *content diff* in a named golden from a hard failure to a recorded, +non-blocking `SOFTFAIL` marker. It is the last resort — reach for these tools in order: + +1. **`Repls` regex mask** — for *describable* volatility (UUIDs, timestamps, versions). Keeps the rest of the file strict. +2. **Field projection** (`_fields()`, see `.agent/rules/testing.md`) — for "the backend added a field I don't assert". Projecting a response to the fields you assert ignores new fields with zero drift. This fully handles the extra-field case; it should never reach soft-fail. +3. **`SoftFailFiles`** — only for *undescribable* drift you can't regex ahead of time: a reworded message whose new text is unknown, or a wholesale remote-artifact dump we don't control. + +### How it works + +Add the golden's name to `SoftFailFiles` in `test.toml` (inherited from a parent like any +other config, and surfaced in `out.test.toml` so reviewers see the shield on every PR): + +```toml +Badness = "mlops-stacks template is fetched remotely and updated out of band" +SoftFailFiles = ["out.template.txt"] +``` + +- **`Badness` is required** — every shield carries an in-tree justification. +- **`output.txt` can never be shielded** (a hard config error), and entries must start with `out`. `output.txt` carries the CLI behavior a local regression would corrupt, so a regression in our own logic still turns the test red. +- Only a *content diff* is downgraded. A panic, an unexpected exit code, a missing golden, or an unexpected new file stays a hard failure. + +Because the shield is per-file, **route volatile output to its own `out.*` file** and keep +the assertable part strict. Split along a *volatility* seam, not a *convenience* seam: + +```bash +# strict — our logic, stays red on any drift +trace $CLI bundle plan | contains.py "Plan: 1 to add" + +# volatile — reworded backend output, isolated into its own soft-failed file +trace $CLI some-command-with-drifty-output &> out.drifty.txt +``` + +A pure local testserver test is deterministic and cannot drift, so soft-fail is only ever +appropriate for cloud tests and tests that consume remotely-fetched artifacts. + +### Refresh cadence (oncall) + +Soft-failed goldens are reported to the GitHub step summary on every run (via +`tools/softfail_report.py`, which greps the gotestsum JSON for `SOFTFAIL` markers) so drift +is visible even though the build is green. On a cadence, oncall: + +1. runs `./task test-update` (cloud tests under the cloud runner), +2. eyeballs `git diff` — mundane drift → commit; anything resembling a real behavior change → investigate, +3. commits the refreshed goldens. + ## Benchmarks Benchmarks are regular acceptance test that log measurements in certain format. The output can be fed to `tools/bench_parse.py` to print a summary table. diff --git a/acceptance/acceptance_test.go b/acceptance/acceptance_test.go index e5781f88f43..c814bdedf3a 100644 --- a/acceptance/acceptance_test.go +++ b/acceptance/acceptance_test.go @@ -978,7 +978,7 @@ func runTest(t *testing.T, continue } - doComparison(t, repls, dir, tmpDir, relPath, &printedRepls) + doComparison(t, repls, dir, tmpDir, relPath, config.SoftFailFiles, &printedRepls) } // Make sure there are not unaccounted for new files @@ -1012,7 +1012,7 @@ func runTest(t *testing.T, if strings.HasPrefix(relPath, "out") { // We have a new file starting with "out" // Show the contents & support overwrite mode for it: - doComparison(t, repls, dir, tmpDir, relPath, &printedRepls) + doComparison(t, repls, dir, tmpDir, relPath, config.SoftFailFiles, &printedRepls) } } @@ -1085,7 +1085,7 @@ func addEnvVar(t *testing.T, env []string, repls *testdiff.ReplacementsContext, return append(env, key+"="+newValue) } -func doComparison(t *testing.T, repls testdiff.ReplacementsContext, dirRef, dirNew, relPath string, printedRepls *bool) { +func doComparison(t testutil.TestingT, repls testdiff.ReplacementsContext, dirRef, dirNew, relPath string, softFailFiles []string, printedRepls *bool) { pathRef := filepath.Join(dirRef, relPath) pathNew := filepath.Join(dirNew, relPath) bufRef, okRef := tryReading(t, pathRef) @@ -1145,6 +1145,16 @@ func doComparison(t *testing.T, repls testdiff.ReplacementsContext, dirRef, dirN return } + // Soft-fail: a content diff in a listed golden is downgraded to a recorded, + // non-blocking marker instead of failing the test. This must run before + // AssertEqualTexts, which calls testify's assert.Equal and marks the test + // failed as a side effect that its bool return cannot undo. + if valueRef != valueNew && slices.Contains(softFailFiles, relPath) { + diff := testdiff.UnifiedDiff(pathRef, pathNew, valueRef, valueNew) + t.Logf("SOFTFAIL %s\n%s", relPath, diff) + return + } + // Compare the reference and new values. equal := testdiff.AssertEqualTexts(t, pathRef, pathNew, valueRef, valueNew) @@ -1441,7 +1451,7 @@ func formatOutput(w io.Writer, err error) { } } -func tryReading(t *testing.T, path string) (string, bool) { +func tryReading(t testutil.TestingT, path string) (string, bool) { info, err := os.Stat(path) if err != nil { if !errors.Is(err, os.ErrNotExist) { diff --git a/acceptance/internal/config.go b/acceptance/internal/config.go index b2d1a9f578f..09b229d187a 100644 --- a/acceptance/internal/config.go +++ b/acceptance/internal/config.go @@ -2,6 +2,7 @@ package internal import ( "errors" + "fmt" "hash/fnv" "io/fs" "maps" @@ -95,6 +96,14 @@ type TestConfig struct { // List of gitignore patterns to ignore when checking output files Ignore []string + // Golden files whose content diffs are downgraded from a hard failure to a + // recorded, non-blocking SOFTFAIL. Use ONLY for files that capture volatility + // outside our control that a Repls mask or field-projection can't express + // (reworded backend messages, remotely-updated template dumps). "output.txt" + // is rejected: it carries the CLI behavior a local regression would corrupt. + // Requires Badness to be set, documenting why each file is shielded. + SoftFailFiles []string + CompiledIgnoreObject *ignore.GitIgnore // Environment variables @@ -257,6 +266,32 @@ func validateConfig(t *testing.T, config TestConfig, configPath string) { "Output files must not be ignored.", configPath, pattern) } } + + if err := validateSoftFailFiles(config); err != nil { + t.Fatalf("Invalid config %s: %s", configPath, err) + } +} + +// validateSoftFailFiles enforces the SoftFailFiles invariants: every shield needs +// a Badness justification, output.txt can never be shielded (it carries the CLI +// behavior a local regression would corrupt), and only generated files (out*) are +// eligible. +func validateSoftFailFiles(config TestConfig) error { + if len(config.SoftFailFiles) == 0 { + return nil + } + if config.Badness == nil { + return errors.New("SoftFailFiles requires Badness to be set, documenting why each file is shielded") + } + for _, name := range config.SoftFailFiles { + if name == "output.txt" { + return fmt.Errorf("SoftFailFiles must not contain %q; it carries CLI behavior a local regression would corrupt", name) + } + if !strings.HasPrefix(name, "out") { + return fmt.Errorf("SoftFailFiles entry %q must start with \"out\"; only generated output files can be soft-failed", name) + } + } + return nil } func DoLoadConfig(t *testing.T, path string) TestConfig { diff --git a/acceptance/internal/config_test.go b/acceptance/internal/config_test.go index a81345ed571..455a0205ecb 100644 --- a/acceptance/internal/config_test.go +++ b/acceptance/internal/config_test.go @@ -242,6 +242,51 @@ func TestSubsetExpanded_ScriptUsesEngine(t *testing.T) { assert.True(t, engines["DATABRICKS_BUNDLE_ENGINE=direct"]) } +func TestValidateSoftFailFiles(t *testing.T) { + badness := "backend rewords this message out of our control" + tests := []struct { + name string + config TestConfig + wantErr string + }{ + { + name: "empty is allowed without badness", + config: TestConfig{}, + }, + { + name: "valid out file with badness", + config: TestConfig{Badness: &badness, SoftFailFiles: []string{"out.drifty.txt"}}, + }, + { + name: "requires badness", + config: TestConfig{SoftFailFiles: []string{"out.drifty.txt"}}, + wantErr: "requires Badness", + }, + { + name: "output.txt is rejected", + config: TestConfig{Badness: &badness, SoftFailFiles: []string{"output.txt"}}, + wantErr: "must not contain", + }, + { + name: "non-out file is rejected", + config: TestConfig{Badness: &badness, SoftFailFiles: []string{"databricks.yml"}}, + wantErr: "must start with", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + err := validateSoftFailFiles(tt.config) + if tt.wantErr == "" { + assert.NoError(t, err) + } else { + require.Error(t, err) + assert.Contains(t, err.Error(), tt.wantErr) + } + }) + } +} + func TestLoadConfigPhaseIsNotInherited(t *testing.T) { tests := []struct { name string diff --git a/acceptance/internal/materialized_config.go b/acceptance/internal/materialized_config.go index cd6d34739d5..8febaf4d2a4 100644 --- a/acceptance/internal/materialized_config.go +++ b/acceptance/internal/materialized_config.go @@ -65,6 +65,10 @@ func GenerateMaterializedConfig(config *TestConfig) string { writeTomlStringArray(&buf, "EnvMatrix."+k, envMatrix[k]) } + if len(config.SoftFailFiles) > 0 { + writeTomlStringArray(&buf, "SoftFailFiles", config.SoftFailFiles) + } + return buf.String() } diff --git a/acceptance/softfail_test.go b/acceptance/softfail_test.go new file mode 100644 index 00000000000..7cc25a1e6b9 --- /dev/null +++ b/acceptance/softfail_test.go @@ -0,0 +1,89 @@ +package acceptance_test + +import ( + "context" + "fmt" + "path/filepath" + "strings" + "testing" + + "github.com/databricks/cli/internal/testutil" + "github.com/databricks/cli/libs/testdiff" + "github.com/stretchr/testify/assert" +) + +// recordingT records log output and whether a failure was reported, so we can +// drive doComparison without failing the enclosing test. +type recordingT struct { + testutil.TestingT + ctx context.Context + logs strings.Builder + failed bool +} + +func (r *recordingT) Helper() {} + +func (r *recordingT) Log(args ...any) { fmt.Fprintln(&r.logs, args...) } + +func (r *recordingT) Logf(format string, args ...any) { + fmt.Fprintf(&r.logs, format, args...) +} + +func (r *recordingT) Error(args ...any) { r.failed = true } + +func (r *recordingT) Errorf(format string, args ...any) { r.failed = true } + +func (r *recordingT) Context() context.Context { return r.ctx } + +// TestSoftFailComparison exercises the soft-fail branch in doComparison directly. +// A committed golden mismatch can't drive an acceptance selftest for this: -update +// would heal it, so the SOFTFAIL path would stop firing. Driving the comparison +// with a recording T lets us assert both the green-on-listed-file path and that an +// unlisted file (including output.txt) still fails. +func TestSoftFailComparison(t *testing.T) { + tests := []struct { + name string + relPath string + softFailFiles []string + wantFailed bool + wantSoftfail bool + }{ + { + name: "listed file drifts green with marker", + relPath: "out.drifty.txt", + softFailFiles: []string{"out.drifty.txt"}, + wantFailed: false, + wantSoftfail: true, + }, + { + name: "unlisted file drift still fails", + relPath: "out.other.txt", + softFailFiles: []string{"out.drifty.txt"}, + wantFailed: true, + wantSoftfail: false, + }, + { + name: "output.txt drift still fails", + relPath: "output.txt", + softFailFiles: []string{"out.drifty.txt"}, + wantFailed: true, + wantSoftfail: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + dirRef := t.TempDir() + dirNew := t.TempDir() + testutil.WriteFile(t, filepath.Join(dirRef, tt.relPath), "reference\n") + testutil.WriteFile(t, filepath.Join(dirNew, tt.relPath), "drifted\n") + + rt := &recordingT{ctx: t.Context()} + doComparison(rt, testdiff.ReplacementsContext{}, dirRef, dirNew, tt.relPath, tt.softFailFiles, nil) + + assert.Equal(t, tt.wantFailed, rt.failed) + gotSoftfail := strings.Contains(rt.logs.String(), "SOFTFAIL "+tt.relPath) + assert.Equal(t, tt.wantSoftfail, gotSoftfail) + }) + } +} diff --git a/tools/softfail_report.py b/tools/softfail_report.py new file mode 100644 index 00000000000..b81fbb235d5 --- /dev/null +++ b/tools/softfail_report.py @@ -0,0 +1,103 @@ +#!/usr/bin/env python3 +""" +Scan a gotestsum/`go test -json` file for SOFTFAIL markers and print a markdown +report. Acceptance tests emit `SOFTFAIL ` followed by a unified diff via +t.Logf when a soft-failed golden drifts (see doComparison in acceptance_test.go). + +Those tests stay green, so the drift is invisible unless we surface it. This report +is written to the GitHub step summary on every run so oncall can refresh the golden +on a cadence without the build ever turning red. + +Usage: softfail_report.py test-output.json [test-output.json ...] +Exits 0 whether or not markers are found (soft-fail is non-blocking by design). +""" + +import json +import sys +from collections import defaultdict + + +def collect_softfails(events): + """Group SOFTFAIL diff blocks by test from `go test -json` output events. + + Each SOFTFAIL marker line opens a block that runs until the next test-scoped + control event, so the unified diff that follows the marker is captured with it. + + >>> evs = [ + ... {"Action": "output", "Test": "TestAccept/x", "Output": " foo.go:1: SOFTFAIL out.drifty.txt\\n"}, + ... {"Action": "output", "Test": "TestAccept/x", "Output": " --- a\\n"}, + ... {"Action": "output", "Test": "TestAccept/x", "Output": " +++ b\\n"}, + ... {"Action": "pass", "Test": "TestAccept/x"}, + ... ] + >>> result = collect_softfails(evs) + >>> result["TestAccept/x"][0][0] + 'out.drifty.txt' + """ + blocks = defaultdict(list) + open_block = {} + for ev in events: + if ev.get("Action") != "output": + open_block.pop(ev.get("Test"), None) + continue + test = ev.get("Test") + out = ev.get("Output", "") + marker = "SOFTFAIL " + idx = out.find(marker) + if idx != -1: + relpath = out[idx + len(marker) :].strip() + block = [relpath, []] + blocks[test].append(block) + open_block[test] = block + elif test in open_block: + open_block[test][1].append(out) + return blocks + + +def render(blocks): + """Render collected SOFTFAIL blocks as a markdown report.""" + if not blocks: + return "No soft-failed acceptance goldens drifted in this run.\n" + + total = sum(len(v) for v in blocks.values()) + lines = [ + f"## Soft-failed acceptance goldens ({total})", + "", + "These goldens drifted but did not fail the build. Refresh with `./task test-update`.", + "", + ] + for test in sorted(blocks): + for relpath, diff in blocks[test]: + lines.append(f"
{relpath} — {test}") + lines.append("") + lines.append("```diff") + lines.append("".join(diff).rstrip("\n")) + lines.append("```") + lines.append("
") + lines.append("") + return "\n".join(lines) + + +def read_events(paths): + for path in paths: + with open(path) as f: + for line in f: + line = line.strip() + if not line: + continue + try: + yield json.loads(line) + except json.JSONDecodeError: + continue + + +def main(argv): + if len(argv) < 2: + sys.stderr.write(f"usage: {argv[0]} test-output.json [...]\n") + return 2 + blocks = collect_softfails(read_events(argv[1:])) + sys.stdout.write(render(blocks)) + return 0 + + +if __name__ == "__main__": + sys.exit(main(sys.argv)) From 230a580c1803462707b8a871994ed14180051a1e Mon Sep 17 00:00:00 2001 From: Jan Rose Date: Wed, 15 Jul 2026 13:39:13 +0200 Subject: [PATCH 2/9] acc: add whole-test SoftFail flag Some tests depend end-to-end on a runtime-fetched artifact outside our control, so an upstream change breaks not just captured output but the run itself. For those there is no volatility seam to split into a per-file SoftFailFiles shield: output.txt is the drift. Add a whole-test SoftFail flag that downgrades a content diff in ANY of the test's goldens (including output.txt) to a non-blocking SOFTFAIL marker. It is a blunt instrument, gated on Badness and intended only for tests whose CLI behavior is also covered by a hermetic local test, so shielding the e2e test can't hide a real regression. Structural failures (panics, unexpected/missing files) stay hard. Co-authored-by: Isaac --- acceptance/README.md | 19 +++++++++++++++++++ acceptance/acceptance_test.go | 17 +++++++++-------- acceptance/internal/config.go | 21 +++++++++++++++++---- acceptance/internal/config_test.go | 10 ++++++++++ acceptance/internal/materialized_config.go | 1 + acceptance/softfail_test.go | 17 ++++++++++++++++- 6 files changed, 72 insertions(+), 13 deletions(-) diff --git a/acceptance/README.md b/acceptance/README.md index ac2196a0c18..9adaf35cb47 100644 --- a/acceptance/README.md +++ b/acceptance/README.md @@ -66,6 +66,25 @@ trace $CLI some-command-with-drifty-output &> out.drifty.txt A pure local testserver test is deterministic and cannot drift, so soft-fail is only ever appropriate for cloud tests and tests that consume remotely-fetched artifacts. +### Whole-test shield (`SoftFail`) + +When a test's behavior *end-to-end* depends on a runtime-fetched artifact outside our +control — so an upstream change breaks not just captured output but the run itself (e.g. a +remotely-hosted template whose new resources fail to deploy) — no volatility seam exists to +split, because `output.txt` itself is the drift. `SoftFail = true` shields **every** golden +for that test, including `output.txt`: + +```toml +Badness = "mlops-stacks template is fetched remotely; upstream changes break this e2e test" +SoftFail = true +``` + +This is a blunt instrument. Only use it when the CLI behavior under test is **also covered +by a hermetic local test**, so shielding the whole e2e test can't hide a real regression in +that behavior (it would still turn the local test red). Prefer `SoftFailFiles` whenever a +seam can be isolated. `Badness` is still required, and structural failures (panics, +unexpected/missing files) stay hard even under `SoftFail`. + ### Refresh cadence (oncall) Soft-failed goldens are reported to the GitHub step summary on every run (via diff --git a/acceptance/acceptance_test.go b/acceptance/acceptance_test.go index c814bdedf3a..dc944c89114 100644 --- a/acceptance/acceptance_test.go +++ b/acceptance/acceptance_test.go @@ -978,7 +978,7 @@ func runTest(t *testing.T, continue } - doComparison(t, repls, dir, tmpDir, relPath, config.SoftFailFiles, &printedRepls) + doComparison(t, repls, dir, tmpDir, relPath, config.SoftFailFiles, isTruePtr(config.SoftFail), &printedRepls) } // Make sure there are not unaccounted for new files @@ -1012,7 +1012,7 @@ func runTest(t *testing.T, if strings.HasPrefix(relPath, "out") { // We have a new file starting with "out" // Show the contents & support overwrite mode for it: - doComparison(t, repls, dir, tmpDir, relPath, config.SoftFailFiles, &printedRepls) + doComparison(t, repls, dir, tmpDir, relPath, config.SoftFailFiles, isTruePtr(config.SoftFail), &printedRepls) } } @@ -1085,7 +1085,7 @@ func addEnvVar(t *testing.T, env []string, repls *testdiff.ReplacementsContext, return append(env, key+"="+newValue) } -func doComparison(t testutil.TestingT, repls testdiff.ReplacementsContext, dirRef, dirNew, relPath string, softFailFiles []string, printedRepls *bool) { +func doComparison(t testutil.TestingT, repls testdiff.ReplacementsContext, dirRef, dirNew, relPath string, softFailFiles []string, softFailAll bool, printedRepls *bool) { pathRef := filepath.Join(dirRef, relPath) pathNew := filepath.Join(dirNew, relPath) bufRef, okRef := tryReading(t, pathRef) @@ -1145,11 +1145,12 @@ func doComparison(t testutil.TestingT, repls testdiff.ReplacementsContext, dirRe return } - // Soft-fail: a content diff in a listed golden is downgraded to a recorded, - // non-blocking marker instead of failing the test. This must run before - // AssertEqualTexts, which calls testify's assert.Equal and marks the test - // failed as a side effect that its bool return cannot undo. - if valueRef != valueNew && slices.Contains(softFailFiles, relPath) { + // Soft-fail: a content diff in a shielded golden is downgraded to a recorded, + // non-blocking marker instead of failing the test. softFailAll shields every + // file including output.txt; softFailFiles shields named files only. This must + // run before AssertEqualTexts, which calls testify's assert.Equal and marks the + // test failed as a side effect that its bool return cannot undo. + if valueRef != valueNew && (softFailAll || slices.Contains(softFailFiles, relPath)) { diff := testdiff.UnifiedDiff(pathRef, pathNew, valueRef, valueNew) t.Logf("SOFTFAIL %s\n%s", relPath, diff) return diff --git a/acceptance/internal/config.go b/acceptance/internal/config.go index 09b229d187a..09d2bbf3597 100644 --- a/acceptance/internal/config.go +++ b/acceptance/internal/config.go @@ -104,6 +104,16 @@ type TestConfig struct { // Requires Badness to be set, documenting why each file is shielded. SoftFailFiles []string + // If true, ALL golden files for this test (including output.txt) are soft-failed: + // any content diff is downgraded to a non-blocking SOFTFAIL marker. This is a + // blunt, whole-test shield reserved for tests whose behavior end-to-end depends + // on an artifact fetched at runtime outside our control (e.g. a remotely-hosted + // template), where an upstream change breaks not just captured output but the + // run itself. Only use it when the CLI behavior under test is also covered by a + // hermetic local test. Requires Badness to be set. Prefer the per-file + // SoftFailFiles whenever a volatility seam can be isolated. + SoftFail *bool + CompiledIgnoreObject *ignore.GitIgnore // Environment variables @@ -272,11 +282,14 @@ func validateConfig(t *testing.T, config TestConfig, configPath string) { } } -// validateSoftFailFiles enforces the SoftFailFiles invariants: every shield needs -// a Badness justification, output.txt can never be shielded (it carries the CLI -// behavior a local regression would corrupt), and only generated files (out*) are -// eligible. +// validateSoftFailFiles enforces the soft-fail invariants: every shield needs a +// Badness justification; per-file entries may not target output.txt (it carries the +// CLI behavior a local regression would corrupt) and must be generated files (out*). +// The whole-test SoftFail shield deliberately has no such file restriction. func validateSoftFailFiles(config TestConfig) error { + if isTruePtr(config.SoftFail) && config.Badness == nil { + return errors.New("SoftFail requires Badness to be set, documenting why the test is shielded") + } if len(config.SoftFailFiles) == 0 { return nil } diff --git a/acceptance/internal/config_test.go b/acceptance/internal/config_test.go index 455a0205ecb..90ddfb592d5 100644 --- a/acceptance/internal/config_test.go +++ b/acceptance/internal/config_test.go @@ -244,6 +244,7 @@ func TestSubsetExpanded_ScriptUsesEngine(t *testing.T) { func TestValidateSoftFailFiles(t *testing.T) { badness := "backend rewords this message out of our control" + yes := true tests := []struct { name string config TestConfig @@ -272,6 +273,15 @@ func TestValidateSoftFailFiles(t *testing.T) { config: TestConfig{Badness: &badness, SoftFailFiles: []string{"databricks.yml"}}, wantErr: "must start with", }, + { + name: "whole-test SoftFail with badness", + config: TestConfig{Badness: &badness, SoftFail: &yes}, + }, + { + name: "whole-test SoftFail requires badness", + config: TestConfig{SoftFail: &yes}, + wantErr: "SoftFail requires Badness", + }, } for _, tt := range tests { diff --git a/acceptance/internal/materialized_config.go b/acceptance/internal/materialized_config.go index 8febaf4d2a4..4ac5e8bcad8 100644 --- a/acceptance/internal/materialized_config.go +++ b/acceptance/internal/materialized_config.go @@ -47,6 +47,7 @@ func GenerateMaterializedConfig(config *TestConfig) string { writeBool(&buf, "RequiresCluster", config.RequiresCluster) writeBool(&buf, "RequiresWarehouse", config.RequiresWarehouse) writeBool(&buf, "RunsOnDbr", config.RunsOnDbr) + writeBool(&buf, "SoftFail", config.SoftFail) if config.Phase != 0 { fmt.Fprintf(&buf, "Phase = %d\n", config.Phase) } diff --git a/acceptance/softfail_test.go b/acceptance/softfail_test.go index 7cc25a1e6b9..73fa3928aac 100644 --- a/acceptance/softfail_test.go +++ b/acceptance/softfail_test.go @@ -45,6 +45,7 @@ func TestSoftFailComparison(t *testing.T) { name string relPath string softFailFiles []string + softFailAll bool wantFailed bool wantSoftfail bool }{ @@ -69,6 +70,20 @@ func TestSoftFailComparison(t *testing.T) { wantFailed: true, wantSoftfail: false, }, + { + name: "whole-test shield drifts green with marker", + relPath: "out.other.txt", + softFailAll: true, + wantFailed: false, + wantSoftfail: true, + }, + { + name: "whole-test shield covers output.txt", + relPath: "output.txt", + softFailAll: true, + wantFailed: false, + wantSoftfail: true, + }, } for _, tt := range tests { @@ -79,7 +94,7 @@ func TestSoftFailComparison(t *testing.T) { testutil.WriteFile(t, filepath.Join(dirNew, tt.relPath), "drifted\n") rt := &recordingT{ctx: t.Context()} - doComparison(rt, testdiff.ReplacementsContext{}, dirRef, dirNew, tt.relPath, tt.softFailFiles, nil) + doComparison(rt, testdiff.ReplacementsContext{}, dirRef, dirNew, tt.relPath, tt.softFailFiles, tt.softFailAll, nil) assert.Equal(t, tt.wantFailed, rt.failed) gotSoftfail := strings.Contains(rt.logs.String(), "SOFTFAIL "+tt.relPath) From b262aab601cc66e50743526647bb7e3c1e8e8a91 Mon Sep 17 00:00:00 2001 From: Jan Rose Date: Wed, 15 Jul 2026 13:39:29 +0200 Subject: [PATCH 3/9] acc: opt bundle/deploy/mlops-stacks out with whole-test SoftFail This test initializes and deploys the mlops-stacks template fetched from github at runtime, which is updated out of band. An upstream change breaks not just the captured output but the deploy itself, and there is no volatility seam to isolate: output.txt is the drift. bundle init and bundle deploy are covered by hermetic local tests, so shielding this e2e test cannot hide a real CLI regression. Co-authored-by: Isaac --- acceptance/bundle/deploy/mlops-stacks/out.test.toml | 1 + acceptance/bundle/deploy/mlops-stacks/test.toml | 8 +++++++- 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/acceptance/bundle/deploy/mlops-stacks/out.test.toml b/acceptance/bundle/deploy/mlops-stacks/out.test.toml index 650836edeb3..de0c72b4056 100644 --- a/acceptance/bundle/deploy/mlops-stacks/out.test.toml +++ b/acceptance/bundle/deploy/mlops-stacks/out.test.toml @@ -1,3 +1,4 @@ Local = false Cloud = true +SoftFail = true EnvMatrix.DATABRICKS_BUNDLE_ENGINE = ["terraform", "direct"] diff --git a/acceptance/bundle/deploy/mlops-stacks/test.toml b/acceptance/bundle/deploy/mlops-stacks/test.toml index 98dbfa6435c..3475d745548 100644 --- a/acceptance/bundle/deploy/mlops-stacks/test.toml +++ b/acceptance/bundle/deploy/mlops-stacks/test.toml @@ -1,7 +1,13 @@ Cloud=true Local=false -Badness = "the newly initialized bundle from the 'mlops-stacks' template contains two validation warnings in the configuration" +# This test runs the mlops-stacks template end-to-end. The template is fetched from +# github.com/databricks/mlops-stacks at runtime and updated out of band, so upstream +# changes break not just the captured output but the deploy itself. bundle init and +# bundle deploy are covered by hermetic local tests, so shielding this e2e test cannot +# hide a real CLI regression. Refresh the goldens with `./task test-update` on cloud. +Badness = "mlops-stacks template is fetched remotely; upstream changes break this e2e test" +SoftFail = true Ignore = [ "config.json" From f0d7cc2c8a280dac15642a2f4e9afc16c7c29de0 Mon Sep 17 00:00:00 2001 From: Jan Rose Date: Wed, 15 Jul 2026 13:39:38 +0200 Subject: [PATCH 4/9] acc: split dashboards/detect-change plan into strict + soft-failed remote view The detect-change test dumps the full `bundle plan -o json` into one golden. That plan mixes our planning decisions (action/reason/old/new) with the remote view: remote_state (the full backend read) and each change's `remote` value. On cloud the remote view mirrors the dashboards API response, whose output-only fields (create_time, update_time, etag, lifecycle_state, path, ...) are populated by the backend and change out of our control, so the whole golden drifts red for reasons unrelated to the CLI. Split along that volatility seam: the planning decisions stay strict in out.plan.$engine.json, and the remote view is projected into its own out.plan.remote.$engine.json. Only the direct-engine remote file is listed in SoftFailFiles (terraform exposes no remote view here; its file is structurally empty and stays strict, so a regression that populated it would turn the test red). Co-authored-by: Isaac --- .../detect-change/out.plan.direct.json | 35 ++++--------------- .../detect-change/out.plan.remote.direct.json | 27 ++++++++++++++ .../out.plan.remote.terraform.json | 6 ++++ .../dashboards/detect-change/out.test.toml | 1 + .../dashboards/detect-change/output.txt | 6 ++-- .../resources/dashboards/detect-change/script | 10 +++++- .../dashboards/detect-change/test.toml | 12 +++++++ 7 files changed, 66 insertions(+), 31 deletions(-) create mode 100644 acceptance/bundle/resources/dashboards/detect-change/out.plan.remote.direct.json create mode 100644 acceptance/bundle/resources/dashboards/detect-change/out.plan.remote.terraform.json diff --git a/acceptance/bundle/resources/dashboards/detect-change/out.plan.direct.json b/acceptance/bundle/resources/dashboards/detect-change/out.plan.direct.json index b00e44d4fe8..ee62d3427c6 100644 --- a/acceptance/bundle/resources/dashboards/detect-change/out.plan.direct.json +++ b/acceptance/bundle/resources/dashboards/detect-change/out.plan.direct.json @@ -16,57 +16,36 @@ "warehouse_id": "[TEST_DEFAULT_WAREHOUSE_ID]" } }, - "remote_state": { - "create_time": "[TIMESTAMP]", - "dashboard_id": "[DASHBOARD_ID]", - "display_name": "test-dashboard-[UNIQUE_NAME]", - "embed_credentials": false, - "etag": "[ETAG_2]", - "lifecycle_state": "ACTIVE", - "parent_path": "/Workspace/Users/[USERNAME]/.bundle/[UNIQUE_NAME]/resources", - "path": "/Users/[USERNAME]/.bundle/[UNIQUE_NAME]/resources/test-dashboard-[UNIQUE_NAME].lvdash.json", - "published": true, - "serialized_dashboard": "{}\n", - "update_time": "[TIMESTAMP]", - "warehouse_id": "[TEST_DEFAULT_WAREHOUSE_ID]" - }, "changes": { "create_time": { "action": "skip", - "reason": "spec:output_only", - "remote": "[TIMESTAMP]" + "reason": "spec:output_only" }, "dashboard_id": { "action": "skip", - "reason": "spec:output_only", - "remote": "[DASHBOARD_ID]" + "reason": "spec:output_only" }, "etag": { "action": "update", - "old": "[ETAG_1]", - "remote": "[ETAG_2]" + "old": "[ETAG_1]" }, "lifecycle_state": { "action": "skip", - "reason": "spec:output_only", - "remote": "ACTIVE" + "reason": "spec:output_only" }, "path": { "action": "skip", - "reason": "spec:output_only", - "remote": "/Users/[USERNAME]/.bundle/[UNIQUE_NAME]/resources/test-dashboard-[UNIQUE_NAME].lvdash.json" + "reason": "spec:output_only" }, "serialized_dashboard": { "action": "skip", "reason": "etag_based", "old": "{\n \"pages\": [\n {\n \"displayName\": \"New Page\",\n \"layout\": [\n {\n \"position\": {\n \"height\": 2,\n \"width\": 6,\n \"x\": 0,\n \"y\": 0\n },\n \"widget\": {\n \"name\": \"82eb9107\",\n \"textbox_spec\": \"# I'm a title\"\n }\n },\n {\n \"position\": {\n \"height\": 2,\n \"width\": 6,\n \"x\": 0,\n \"y\": 2\n },\n \"widget\": {\n \"name\": \"ffa6de4f\",\n \"textbox_spec\": \"Text\"\n }\n }\n ],\n \"name\": \"fdd21a3c\"\n }\n ]\n}\n", - "new": "{\n \"pages\": [\n {\n \"displayName\": \"New Page\",\n \"layout\": [\n {\n \"position\": {\n \"height\": 2,\n \"width\": 6,\n \"x\": 0,\n \"y\": 0\n },\n \"widget\": {\n \"name\": \"82eb9107\",\n \"textbox_spec\": \"# I'm a title\"\n }\n },\n {\n \"position\": {\n \"height\": 2,\n \"width\": 6,\n \"x\": 0,\n \"y\": 2\n },\n \"widget\": {\n \"name\": \"ffa6de4f\",\n \"textbox_spec\": \"Text\"\n }\n }\n ],\n \"name\": \"fdd21a3c\"\n }\n ]\n}\n", - "remote": "{}\n" + "new": "{\n \"pages\": [\n {\n \"displayName\": \"New Page\",\n \"layout\": [\n {\n \"position\": {\n \"height\": 2,\n \"width\": 6,\n \"x\": 0,\n \"y\": 0\n },\n \"widget\": {\n \"name\": \"82eb9107\",\n \"textbox_spec\": \"# I'm a title\"\n }\n },\n {\n \"position\": {\n \"height\": 2,\n \"width\": 6,\n \"x\": 0,\n \"y\": 2\n },\n \"widget\": {\n \"name\": \"ffa6de4f\",\n \"textbox_spec\": \"Text\"\n }\n }\n ],\n \"name\": \"fdd21a3c\"\n }\n ]\n}\n" }, "update_time": { "action": "skip", - "reason": "spec:output_only", - "remote": "[TIMESTAMP]" + "reason": "spec:output_only" } } } diff --git a/acceptance/bundle/resources/dashboards/detect-change/out.plan.remote.direct.json b/acceptance/bundle/resources/dashboards/detect-change/out.plan.remote.direct.json new file mode 100644 index 00000000000..3311218de11 --- /dev/null +++ b/acceptance/bundle/resources/dashboards/detect-change/out.plan.remote.direct.json @@ -0,0 +1,27 @@ +{ + "resources.dashboards.file_reference": { + "remote_state": { + "create_time": "[TIMESTAMP]", + "dashboard_id": "[DASHBOARD_ID]", + "display_name": "test-dashboard-[UNIQUE_NAME]", + "embed_credentials": false, + "etag": "[ETAG_2]", + "lifecycle_state": "ACTIVE", + "parent_path": "/Workspace/Users/[USERNAME]/.bundle/[UNIQUE_NAME]/resources", + "path": "/Users/[USERNAME]/.bundle/[UNIQUE_NAME]/resources/test-dashboard-[UNIQUE_NAME].lvdash.json", + "published": true, + "serialized_dashboard": "{}\n", + "update_time": "[TIMESTAMP]", + "warehouse_id": "[TEST_DEFAULT_WAREHOUSE_ID]" + }, + "changes": { + "create_time": "[TIMESTAMP]", + "dashboard_id": "[DASHBOARD_ID]", + "etag": "[ETAG_2]", + "lifecycle_state": "ACTIVE", + "path": "/Users/[USERNAME]/.bundle/[UNIQUE_NAME]/resources/test-dashboard-[UNIQUE_NAME].lvdash.json", + "serialized_dashboard": "{}\n", + "update_time": "[TIMESTAMP]" + } + } +} diff --git a/acceptance/bundle/resources/dashboards/detect-change/out.plan.remote.terraform.json b/acceptance/bundle/resources/dashboards/detect-change/out.plan.remote.terraform.json new file mode 100644 index 00000000000..656f3a3e1f1 --- /dev/null +++ b/acceptance/bundle/resources/dashboards/detect-change/out.plan.remote.terraform.json @@ -0,0 +1,6 @@ +{ + "resources.dashboards.file_reference": { + "remote_state": null, + "changes": {} + } +} diff --git a/acceptance/bundle/resources/dashboards/detect-change/out.test.toml b/acceptance/bundle/resources/dashboards/detect-change/out.test.toml index f489904c6fa..15941ff9b37 100644 --- a/acceptance/bundle/resources/dashboards/detect-change/out.test.toml +++ b/acceptance/bundle/resources/dashboards/detect-change/out.test.toml @@ -4,3 +4,4 @@ RequiresWarehouse = true GOOSOnPR.darwin = false GOOSOnPR.windows = false EnvMatrix.DATABRICKS_BUNDLE_ENGINE = ["terraform", "direct"] +SoftFailFiles = ["out.plan.remote.direct.json"] diff --git a/acceptance/bundle/resources/dashboards/detect-change/output.txt b/acceptance/bundle/resources/dashboards/detect-change/output.txt index fe3f8962ec7..4484171d09b 100644 --- a/acceptance/bundle/resources/dashboards/detect-change/output.txt +++ b/acceptance/bundle/resources/dashboards/detect-change/output.txt @@ -66,8 +66,6 @@ The remote modifications will be lost. update dashboards.file_reference Plan: 0 to add, 1 to change, 0 to delete, 0 unchanged - ->>> [CLI] bundle plan -o json Warning: dashboard "file_reference" has been modified remotely at resources.dashboards.file_reference in databricks.yml:10:7 @@ -82,6 +80,10 @@ To overwrite the remote changes with your local version, use --force. The remote modifications will be lost. +>>> jq del(.plan[].remote_state?) | del(.plan[].changes[]?.remote) plan.json + +>>> jq .plan | map_values({remote_state, changes: (.changes // {} | map_values(.remote))}) plan.json + >>> errcode [CLI] bundle deploy Error: dashboard "file_reference" has been modified remotely at resources.dashboards.file_reference diff --git a/acceptance/bundle/resources/dashboards/detect-change/script b/acceptance/bundle/resources/dashboards/detect-change/script index 98ab4cc93b7..fb71211adcc 100644 --- a/acceptance/bundle/resources/dashboards/detect-change/script +++ b/acceptance/bundle/resources/dashboards/detect-change/script @@ -38,7 +38,15 @@ echo "$(echo "$UPDATE_RESP" | jq -r '.etag'):ETAG_2" >> ACC_REPLS title "Try to redeploy the bundle and confirm that the out of band modification is detected:" trace $CLI bundle plan -trace $CLI bundle plan -o json > out.plan.$DATABRICKS_BUNDLE_ENGINE.json +# Split the plan JSON along a volatility seam. The planning decisions +# (action/reason/old/new) are our logic and stay strict. The remote view -- +# remote_state (the full backend read) and each change's `remote` value -- mirrors +# the dashboards API response, whose output-only fields grow and change out of our +# control on cloud, so it is routed to its own soft-failed file (see test.toml). +$CLI bundle plan -o json > plan.json +trace jq 'del(.plan[].remote_state?) | del(.plan[].changes[]?.remote)' plan.json > out.plan.$DATABRICKS_BUNDLE_ENGINE.json +trace jq '.plan | map_values({remote_state, changes: (.changes // {} | map_values(.remote))})' plan.json > out.plan.remote.$DATABRICKS_BUNDLE_ENGINE.json +rm plan.json trace errcode $CLI bundle deploy title "Redeploy the bundle with the --force flag and confirm that the out of band modification is ignored:" diff --git a/acceptance/bundle/resources/dashboards/detect-change/test.toml b/acceptance/bundle/resources/dashboards/detect-change/test.toml index fbdfbe42eab..0337b9c6a94 100644 --- a/acceptance/bundle/resources/dashboards/detect-change/test.toml +++ b/acceptance/bundle/resources/dashboards/detect-change/test.toml @@ -1,6 +1,18 @@ Local = true Cloud = true +# The plan's remote view (remote_state + each change's `remote` value) mirrors the +# dashboards API response. Its output-only fields (create_time, update_time, etag, +# lifecycle_state, path, ...) are populated by the backend and grow/change out of our +# control on cloud, so the remote-only file is soft-failed. The planning decisions in +# out.plan.$engine.json stay strict. Only the direct engine exposes a remote view; the +# terraform remote file is structurally empty and stays strict (a regression that +# suddenly populated it should turn the test red). +Badness = "plan remote view mirrors the backend dashboards response, whose output-only fields drift on cloud" +SoftFailFiles = [ + "out.plan.remote.direct.json", +] + Ignore = [ "databricks.yml", ] From 76e7ae7acd5d03a169cbedeed5d0e82cb357bc48 Mon Sep 17 00:00:00 2001 From: Jan Rose Date: Wed, 15 Jul 2026 14:26:38 +0200 Subject: [PATCH 5/9] acc: drop the Badness requirement from soft-fail MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Requiring Badness whenever SoftFailFiles/SoftFail is set overloaded an unrelated documentary field and coupled two independent concepts. The shield is already self-documenting: a comment in test.toml explains why the file drifts, and the flag is surfaced in out.test.toml for reviewers. Remove the requirement (no SoftFailReason replacement — it isn't needed). The structural guards stay: output.txt can never be shielded and per-file entries must start with "out". Restore the mlops-stacks Badness to its original text (it documents the template's validation warnings, unrelated to soft-fail). Co-authored-by: Isaac --- acceptance/README.md | 12 ++++----- .../bundle/deploy/mlops-stacks/test.toml | 3 ++- .../dashboards/detect-change/test.toml | 1 - acceptance/internal/config.go | 22 +++++----------- acceptance/internal/config_test.go | 25 ++++++------------- 5 files changed, 21 insertions(+), 42 deletions(-) diff --git a/acceptance/README.md b/acceptance/README.md index 9adaf35cb47..5cbeb942b77 100644 --- a/acceptance/README.md +++ b/acceptance/README.md @@ -41,14 +41,14 @@ non-blocking `SOFTFAIL` marker. It is the last resort — reach for these tools ### How it works Add the golden's name to `SoftFailFiles` in `test.toml` (inherited from a parent like any -other config, and surfaced in `out.test.toml` so reviewers see the shield on every PR): +other config, and surfaced in `out.test.toml` so reviewers see the shield on every PR). +Add a comment explaining why the file drifts: ```toml -Badness = "mlops-stacks template is fetched remotely and updated out of band" +# out.template.txt is a dump of the remotely-fetched mlops-stacks template SoftFailFiles = ["out.template.txt"] ``` -- **`Badness` is required** — every shield carries an in-tree justification. - **`output.txt` can never be shielded** (a hard config error), and entries must start with `out`. `output.txt` carries the CLI behavior a local regression would corrupt, so a regression in our own logic still turns the test red. - Only a *content diff* is downgraded. A panic, an unexpected exit code, a missing golden, or an unexpected new file stays a hard failure. @@ -75,15 +75,15 @@ split, because `output.txt` itself is the drift. `SoftFail = true` shields **eve for that test, including `output.txt`: ```toml -Badness = "mlops-stacks template is fetched remotely; upstream changes break this e2e test" +# mlops-stacks template is fetched remotely; upstream changes break this e2e test SoftFail = true ``` This is a blunt instrument. Only use it when the CLI behavior under test is **also covered by a hermetic local test**, so shielding the whole e2e test can't hide a real regression in that behavior (it would still turn the local test red). Prefer `SoftFailFiles` whenever a -seam can be isolated. `Badness` is still required, and structural failures (panics, -unexpected/missing files) stay hard even under `SoftFail`. +seam can be isolated. Structural failures (panics, unexpected/missing files) stay hard even +under `SoftFail`. ### Refresh cadence (oncall) diff --git a/acceptance/bundle/deploy/mlops-stacks/test.toml b/acceptance/bundle/deploy/mlops-stacks/test.toml index 3475d745548..ddad65c333c 100644 --- a/acceptance/bundle/deploy/mlops-stacks/test.toml +++ b/acceptance/bundle/deploy/mlops-stacks/test.toml @@ -1,12 +1,13 @@ Cloud=true Local=false +Badness = "the newly initialized bundle from the 'mlops-stacks' template contains two validation warnings in the configuration" + # This test runs the mlops-stacks template end-to-end. The template is fetched from # github.com/databricks/mlops-stacks at runtime and updated out of band, so upstream # changes break not just the captured output but the deploy itself. bundle init and # bundle deploy are covered by hermetic local tests, so shielding this e2e test cannot # hide a real CLI regression. Refresh the goldens with `./task test-update` on cloud. -Badness = "mlops-stacks template is fetched remotely; upstream changes break this e2e test" SoftFail = true Ignore = [ diff --git a/acceptance/bundle/resources/dashboards/detect-change/test.toml b/acceptance/bundle/resources/dashboards/detect-change/test.toml index 0337b9c6a94..3d6bef42fd9 100644 --- a/acceptance/bundle/resources/dashboards/detect-change/test.toml +++ b/acceptance/bundle/resources/dashboards/detect-change/test.toml @@ -8,7 +8,6 @@ Cloud = true # out.plan.$engine.json stay strict. Only the direct engine exposes a remote view; the # terraform remote file is structurally empty and stays strict (a regression that # suddenly populated it should turn the test red). -Badness = "plan remote view mirrors the backend dashboards response, whose output-only fields drift on cloud" SoftFailFiles = [ "out.plan.remote.direct.json", ] diff --git a/acceptance/internal/config.go b/acceptance/internal/config.go index 09d2bbf3597..fe33356b497 100644 --- a/acceptance/internal/config.go +++ b/acceptance/internal/config.go @@ -101,7 +101,6 @@ type TestConfig struct { // outside our control that a Repls mask or field-projection can't express // (reworded backend messages, remotely-updated template dumps). "output.txt" // is rejected: it carries the CLI behavior a local regression would corrupt. - // Requires Badness to be set, documenting why each file is shielded. SoftFailFiles []string // If true, ALL golden files for this test (including output.txt) are soft-failed: @@ -110,8 +109,8 @@ type TestConfig struct { // on an artifact fetched at runtime outside our control (e.g. a remotely-hosted // template), where an upstream change breaks not just captured output but the // run itself. Only use it when the CLI behavior under test is also covered by a - // hermetic local test. Requires Badness to be set. Prefer the per-file - // SoftFailFiles whenever a volatility seam can be isolated. + // hermetic local test. Prefer the per-file SoftFailFiles whenever a volatility + // seam can be isolated. SoftFail *bool CompiledIgnoreObject *ignore.GitIgnore @@ -282,20 +281,11 @@ func validateConfig(t *testing.T, config TestConfig, configPath string) { } } -// validateSoftFailFiles enforces the soft-fail invariants: every shield needs a -// Badness justification; per-file entries may not target output.txt (it carries the -// CLI behavior a local regression would corrupt) and must be generated files (out*). -// The whole-test SoftFail shield deliberately has no such file restriction. +// validateSoftFailFiles enforces the per-file soft-fail structural guards: an +// entry may not target output.txt (it carries the CLI behavior a local regression +// would corrupt) and must be a generated file (out*). The whole-test SoftFail +// shield deliberately has no such file restriction. func validateSoftFailFiles(config TestConfig) error { - if isTruePtr(config.SoftFail) && config.Badness == nil { - return errors.New("SoftFail requires Badness to be set, documenting why the test is shielded") - } - if len(config.SoftFailFiles) == 0 { - return nil - } - if config.Badness == nil { - return errors.New("SoftFailFiles requires Badness to be set, documenting why each file is shielded") - } for _, name := range config.SoftFailFiles { if name == "output.txt" { return fmt.Errorf("SoftFailFiles must not contain %q; it carries CLI behavior a local regression would corrupt", name) diff --git a/acceptance/internal/config_test.go b/acceptance/internal/config_test.go index 90ddfb592d5..262b0877944 100644 --- a/acceptance/internal/config_test.go +++ b/acceptance/internal/config_test.go @@ -243,7 +243,6 @@ func TestSubsetExpanded_ScriptUsesEngine(t *testing.T) { } func TestValidateSoftFailFiles(t *testing.T) { - badness := "backend rewords this message out of our control" yes := true tests := []struct { name string @@ -251,36 +250,26 @@ func TestValidateSoftFailFiles(t *testing.T) { wantErr string }{ { - name: "empty is allowed without badness", + name: "empty is allowed", config: TestConfig{}, }, { - name: "valid out file with badness", - config: TestConfig{Badness: &badness, SoftFailFiles: []string{"out.drifty.txt"}}, - }, - { - name: "requires badness", - config: TestConfig{SoftFailFiles: []string{"out.drifty.txt"}}, - wantErr: "requires Badness", + name: "valid out file", + config: TestConfig{SoftFailFiles: []string{"out.drifty.txt"}}, }, { name: "output.txt is rejected", - config: TestConfig{Badness: &badness, SoftFailFiles: []string{"output.txt"}}, + config: TestConfig{SoftFailFiles: []string{"output.txt"}}, wantErr: "must not contain", }, { name: "non-out file is rejected", - config: TestConfig{Badness: &badness, SoftFailFiles: []string{"databricks.yml"}}, + config: TestConfig{SoftFailFiles: []string{"databricks.yml"}}, wantErr: "must start with", }, { - name: "whole-test SoftFail with badness", - config: TestConfig{Badness: &badness, SoftFail: &yes}, - }, - { - name: "whole-test SoftFail requires badness", - config: TestConfig{SoftFail: &yes}, - wantErr: "SoftFail requires Badness", + name: "whole-test SoftFail", + config: TestConfig{SoftFail: &yes}, }, } From fa5619aacffc1dc8641b0e7d92c630611a5ab03a Mon Sep 17 00:00:00 2001 From: Jan Rose Date: Wed, 15 Jul 2026 17:35:38 +0200 Subject: [PATCH 6/9] Fix bad merge of out.test.toml --- acceptance/bundle/deploy/mlops-stacks/out.test.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/acceptance/bundle/deploy/mlops-stacks/out.test.toml b/acceptance/bundle/deploy/mlops-stacks/out.test.toml index bd96afa7585..9cad1bffd3e 100644 --- a/acceptance/bundle/deploy/mlops-stacks/out.test.toml +++ b/acceptance/bundle/deploy/mlops-stacks/out.test.toml @@ -1,5 +1,5 @@ Local = false Cloud = true -SoftFail = true RequiresUnityCatalog = true +SoftFail = true EnvMatrix.DATABRICKS_BUNDLE_ENGINE = ["terraform", "direct"] From ec920469906712c6a96a9a5c20b8d6c13848b91d Mon Sep 17 00:00:00 2001 From: Jan Rose Date: Wed, 15 Jul 2026 17:58:45 +0200 Subject: [PATCH 7/9] acc: gate soft-fail on cloud runs and keep status lines out of the diff report Two review fixes: - Ignore SoftFailFiles/SoftFail on local runs. A local testserver run is deterministic and cannot drift, so a diff there is a real regression. The shield now takes effect only when CLOUD_ENV is set, so a Local+Cloud test (e.g. dashboards/detect-change) keeps its local golden strict while still shielding cloud API drift. - Close a SOFTFAIL diff block on go test status lines (--- PASS:, etc.) in softfail_report.py. go test -json emits the "--- PASS:" status line as an output event before the pass action, so it was landing inside the reported diff block. The unified diff's own "--- " headers lack the colon and are not affected. Co-authored-by: Isaac --- acceptance/README.md | 6 +++-- acceptance/acceptance_test.go | 14 +++++++++-- .../dashboards/detect-change/test.toml | 9 +++---- tools/softfail_report.py | 24 +++++++++++++++---- 4 files changed, 40 insertions(+), 13 deletions(-) diff --git a/acceptance/README.md b/acceptance/README.md index 5cbeb942b77..006a462e5a1 100644 --- a/acceptance/README.md +++ b/acceptance/README.md @@ -63,8 +63,10 @@ trace $CLI bundle plan | contains.py "Plan: 1 to add" trace $CLI some-command-with-drifty-output &> out.drifty.txt ``` -A pure local testserver test is deterministic and cannot drift, so soft-fail is only ever -appropriate for cloud tests and tests that consume remotely-fetched artifacts. +A pure local testserver test is deterministic and cannot drift, so the shield is +**ignored on local runs**: `SoftFailFiles` and `SoftFail` take effect only when +`CLOUD_ENV` is set, and any diff on a local run stays a hard failure. A test that is both +`Local` and `Cloud` therefore keeps its local golden strict while shielding cloud drift. ### Whole-test shield (`SoftFail`) diff --git a/acceptance/acceptance_test.go b/acceptance/acceptance_test.go index dc944c89114..cb18738a801 100644 --- a/acceptance/acceptance_test.go +++ b/acceptance/acceptance_test.go @@ -972,13 +972,23 @@ func runTest(t *testing.T, pathFilter := preparePathFilter(config, customEnv) + // Soft-fail shields drift we don't control (cloud API output, remote artifacts), + // which only occurs on cloud runs. A local testserver run is deterministic, so any + // diff there is a real regression and must stay red regardless of the shield config. + softFailFiles := config.SoftFailFiles + softFailAll := isTruePtr(config.SoftFail) + if !isRunningOnCloud { + softFailFiles = nil + softFailAll = false + } + // Compare expected outputs for relPath := range outputs { if shouldSkip(pathFilter, relPath) { continue } - doComparison(t, repls, dir, tmpDir, relPath, config.SoftFailFiles, isTruePtr(config.SoftFail), &printedRepls) + doComparison(t, repls, dir, tmpDir, relPath, softFailFiles, softFailAll, &printedRepls) } // Make sure there are not unaccounted for new files @@ -1012,7 +1022,7 @@ func runTest(t *testing.T, if strings.HasPrefix(relPath, "out") { // We have a new file starting with "out" // Show the contents & support overwrite mode for it: - doComparison(t, repls, dir, tmpDir, relPath, config.SoftFailFiles, isTruePtr(config.SoftFail), &printedRepls) + doComparison(t, repls, dir, tmpDir, relPath, softFailFiles, softFailAll, &printedRepls) } } diff --git a/acceptance/bundle/resources/dashboards/detect-change/test.toml b/acceptance/bundle/resources/dashboards/detect-change/test.toml index 3d6bef42fd9..4c04aa49986 100644 --- a/acceptance/bundle/resources/dashboards/detect-change/test.toml +++ b/acceptance/bundle/resources/dashboards/detect-change/test.toml @@ -4,10 +4,11 @@ Cloud = true # The plan's remote view (remote_state + each change's `remote` value) mirrors the # dashboards API response. Its output-only fields (create_time, update_time, etag, # lifecycle_state, path, ...) are populated by the backend and grow/change out of our -# control on cloud, so the remote-only file is soft-failed. The planning decisions in -# out.plan.$engine.json stay strict. Only the direct engine exposes a remote view; the -# terraform remote file is structurally empty and stays strict (a regression that -# suddenly populated it should turn the test red). +# control on cloud, so the remote-only file is soft-failed. The shield only takes effect +# on cloud runs, so the deterministic local run keeps this file strict too. The planning +# decisions in out.plan.$engine.json stay strict on both. Only the direct engine exposes +# a remote view; the terraform remote file is structurally empty and stays strict (a +# regression that suddenly populated it should turn the test red). SoftFailFiles = [ "out.plan.remote.direct.json", ] diff --git a/tools/softfail_report.py b/tools/softfail_report.py index b81fbb235d5..4d7b4ebd684 100644 --- a/tools/softfail_report.py +++ b/tools/softfail_report.py @@ -16,22 +16,34 @@ import sys from collections import defaultdict +MARKER = "SOFTFAIL " + +# Prefixes of go test status/control lines that the framework emits between a +# test's own output and its terminating action (e.g. " --- PASS: TestAccept/x"). +# The unified diff's own "--- " / "+++ " headers lack the colon, so +# they are not mistaken for these. +CONTROL_PREFIXES = ("--- PASS:", "--- FAIL:", "--- SKIP:", "--- BENCH:", "=== ") + def collect_softfails(events): """Group SOFTFAIL diff blocks by test from `go test -json` output events. - Each SOFTFAIL marker line opens a block that runs until the next test-scoped - control event, so the unified diff that follows the marker is captured with it. + Each SOFTFAIL marker line opens a block that captures the unified diff that + follows it, closing on the next non-output action or go test status line so the + "--- PASS:" the framework emits for a passing soft-failed test stays out of the diff. >>> evs = [ ... {"Action": "output", "Test": "TestAccept/x", "Output": " foo.go:1: SOFTFAIL out.drifty.txt\\n"}, ... {"Action": "output", "Test": "TestAccept/x", "Output": " --- a\\n"}, ... {"Action": "output", "Test": "TestAccept/x", "Output": " +++ b\\n"}, + ... {"Action": "output", "Test": "TestAccept/x", "Output": " --- PASS: TestAccept/x (0.50s)\\n"}, ... {"Action": "pass", "Test": "TestAccept/x"}, ... ] >>> result = collect_softfails(evs) >>> result["TestAccept/x"][0][0] 'out.drifty.txt' + >>> "PASS" in "".join(result["TestAccept/x"][0][1]) + False """ blocks = defaultdict(list) open_block = {} @@ -41,14 +53,16 @@ def collect_softfails(events): continue test = ev.get("Test") out = ev.get("Output", "") - marker = "SOFTFAIL " - idx = out.find(marker) + idx = out.find(MARKER) if idx != -1: - relpath = out[idx + len(marker) :].strip() + relpath = out[idx + len(MARKER) :].strip() block = [relpath, []] blocks[test].append(block) open_block[test] = block elif test in open_block: + if out.lstrip().startswith(CONTROL_PREFIXES): + open_block.pop(test) + continue open_block[test][1].append(out) return blocks From 393a62133e324d1d340a945506ae1907e1ec978e Mon Sep 17 00:00:00 2001 From: Jan Rose Date: Wed, 15 Jul 2026 18:12:35 +0200 Subject: [PATCH 8/9] Agent rules --- .agent/rules/testing.md | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/.agent/rules/testing.md b/.agent/rules/testing.md index f0bdff1366d..9af7bc9a36b 100644 --- a/.agent/rules/testing.md +++ b/.agent/rules/testing.md @@ -208,6 +208,22 @@ trace $CLI postgres get-branch "$name" # full payload straight into output.txt For a field that appears on some responses but not others (e.g. a provenance field absent on a default resource), append `| with_entries(select(.value != null))` to the projected object so the optional field is dropped rather than emitted as `null`. +### Soft-failing volatile goldens + +Some goldens drift for reasons outside our control: a live backend rewords a message, a response grows a field we don't assert, or a remotely-hosted template is updated upstream. None is a regression, yet each turns cloud PRs and nightlies red. `SoftFailFiles` / `SoftFail` in `test.toml` downgrade a *content diff* from a hard failure to a non-blocking `SOFTFAIL` marker. See `acceptance/README.md` ("Soft-failing volatile goldens") for the full mechanism and oncall refresh cadence. + +**RULE: Soft-failing is the last resort. Reach for the cheaper tools first, in order.** A `Repls` regex mask for *describable* volatility (UUIDs, timestamps, versions); then field projection (`_fields()`, above) for "the backend added a field I don't assert" — projecting to the fields you assert ignores new fields with zero drift and should never reach soft-fail. Only *undescribable* drift you can't regex ahead of time (a reworded message whose new text is unknown, a wholesale remote-artifact dump) justifies `SoftFailFiles`. + +**RULE: Shield per file, and route the volatile output to its own `out.*` file so the assertable part stays strict.** Split along a *volatility* seam, not a *convenience* seam: keep our own logic (e.g. the plan's action/reason) in a strict golden and isolate only the backend-mirrored output into the soft-failed `out.*` file. + +**RULE: `output.txt` can never be soft-failed** (it's a hard config error), and every `SoftFailFiles` entry must start with `out`. `output.txt` carries the CLI behavior a local regression would corrupt, so our own logic still turns the test red. + +**RULE: Only a content diff is downgraded.** A panic, an unexpected exit code, a missing golden, or an unexpected new file stays a hard failure even under `SoftFail`. + +**RULE: The shield takes effect only on cloud runs.** `SoftFailFiles` and `SoftFail` are ignored when `CLOUD_ENV` is unset, because a pure local testserver test is deterministic and cannot drift — any diff on a local run stays a hard failure. A test that is both `Local` and `Cloud` therefore keeps its local golden strict while shielding cloud drift. + +**RULE: Reserve the whole-test shield `SoftFail = true` for e2e tests whose behavior depends on a runtime-fetched artifact, and only when the same behavior is also covered by a hermetic local test.** It shields *every* golden including `output.txt`, so it can only be safe when a real regression would still turn the local test red. Prefer `SoftFailFiles` whenever a volatility seam can be isolated. + ### Test server Acceptance tests run against an in-process fake of the Databricks API in `libs/testserver/` (`FakeWorkspace` and the per-service handler files). The fake keeps real in-memory state and returns the same errors the backend does: 404 on a missing parent, 409 on a duplicate create, 400 on a missing required field, and so on. `test.toml` can also stub a single route with a canned response: From feafbacd5e7dfa720f9c7720270c07396383fd9c Mon Sep 17 00:00:00 2001 From: Jan Rose Date: Wed, 15 Jul 2026 18:12:46 +0200 Subject: [PATCH 9/9] trace json plan --- acceptance/bundle/resources/dashboards/detect-change/output.txt | 2 ++ acceptance/bundle/resources/dashboards/detect-change/script | 2 +- 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/acceptance/bundle/resources/dashboards/detect-change/output.txt b/acceptance/bundle/resources/dashboards/detect-change/output.txt index 4484171d09b..d82af2bc789 100644 --- a/acceptance/bundle/resources/dashboards/detect-change/output.txt +++ b/acceptance/bundle/resources/dashboards/detect-change/output.txt @@ -66,6 +66,8 @@ The remote modifications will be lost. update dashboards.file_reference Plan: 0 to add, 1 to change, 0 to delete, 0 unchanged + +>>> [CLI] bundle plan -o json Warning: dashboard "file_reference" has been modified remotely at resources.dashboards.file_reference in databricks.yml:10:7 diff --git a/acceptance/bundle/resources/dashboards/detect-change/script b/acceptance/bundle/resources/dashboards/detect-change/script index fb71211adcc..dbddd015a67 100644 --- a/acceptance/bundle/resources/dashboards/detect-change/script +++ b/acceptance/bundle/resources/dashboards/detect-change/script @@ -43,7 +43,7 @@ trace $CLI bundle plan # remote_state (the full backend read) and each change's `remote` value -- mirrors # the dashboards API response, whose output-only fields grow and change out of our # control on cloud, so it is routed to its own soft-failed file (see test.toml). -$CLI bundle plan -o json > plan.json +trace $CLI bundle plan -o json > plan.json trace jq 'del(.plan[].remote_state?) | del(.plan[].changes[]?.remote)' plan.json > out.plan.$DATABRICKS_BUNDLE_ENGINE.json trace jq '.plan | map_values({remote_state, changes: (.changes // {} | map_values(.remote))})' plan.json > out.plan.remote.$DATABRICKS_BUNDLE_ENGINE.json rm plan.json