Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 16 additions & 0 deletions .agent/rules/testing.md
Original file line number Diff line number Diff line change
Expand Up @@ -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 (`<resource>_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:
Expand Down
9 changes: 9 additions & 0 deletions .github/workflows/push.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
71 changes: 71 additions & 0 deletions acceptance/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,77 @@ 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** (`<resource>_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).
Add a comment explaining why the file drifts:

```toml
# out.template.txt is a dump of the remotely-fetched mlops-stacks template
SoftFailFiles = ["out.template.txt"]
```

- **`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 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`)

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
# 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. 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
`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.
Expand Down
29 changes: 25 additions & 4 deletions acceptance/acceptance_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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, &printedRepls)
doComparison(t, repls, dir, tmpDir, relPath, softFailFiles, softFailAll, &printedRepls)
}

// Make sure there are not unaccounted for new files
Expand Down Expand Up @@ -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, &printedRepls)
doComparison(t, repls, dir, tmpDir, relPath, softFailFiles, softFailAll, &printedRepls)
}
}

Expand Down Expand Up @@ -1085,7 +1095,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, softFailAll bool, printedRepls *bool) {
pathRef := filepath.Join(dirRef, relPath)
pathNew := filepath.Join(dirNew, relPath)
bufRef, okRef := tryReading(t, pathRef)
Expand Down Expand Up @@ -1145,6 +1155,17 @@ func doComparison(t *testing.T, repls testdiff.ReplacementsContext, dirRef, dirN
return
}

// 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
}

// Compare the reference and new values.
equal := testdiff.AssertEqualTexts(t, pathRef, pathNew, valueRef, valueNew)

Expand Down Expand Up @@ -1441,7 +1462,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) {
Expand Down
1 change: 1 addition & 0 deletions acceptance/bundle/deploy/mlops-stacks/out.test.toml

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

7 changes: 7 additions & 0 deletions acceptance/bundle/deploy/mlops-stacks/test.toml
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,13 @@ RequiresUnityCatalog = true

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.
SoftFail = true

Ignore = [
"config.json"
]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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"
}
}
}
Expand Down
Original file line number Diff line number Diff line change
@@ -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]"
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
{
"resources.dashboards.file_reference": {
"remote_state": null,
"changes": {}
}
}

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,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
Expand Down
10 changes: 9 additions & 1 deletion acceptance/bundle/resources/dashboards/detect-change/script
Original file line number Diff line number Diff line change
Expand Up @@ -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).
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
trace errcode $CLI bundle deploy

title "Redeploy the bundle with the --force flag and confirm that the out of band modification is ignored:"
Expand Down
12 changes: 12 additions & 0 deletions acceptance/bundle/resources/dashboards/detect-change/test.toml
Original file line number Diff line number Diff line change
@@ -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 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",
]

Ignore = [
"databricks.yml",
]
Expand Down
Loading
Loading