From 1d5ea1e4837b9e342b7e2ec7e81ae62d6b85adc5 Mon Sep 17 00:00:00 2001 From: Jan N Rose Date: Mon, 24 Aug 2026 11:07:51 +0200 Subject: [PATCH 1/2] acc: Remove superfluous vector search test.toml Not needed because parent's `CloudSlow = true` flipped `Cloud` to `true`. Follow-up will stop that behaviour (CloudSlow should only affect `Cloud = true`). However, this test _should_ actually be `Cloud = true` so this config can be removed. --- .../vector_search_indexes/schema_normalization/test.toml | 1 - 1 file changed, 1 deletion(-) delete mode 100644 acceptance/bundle/resources/vector_search_indexes/schema_normalization/test.toml diff --git a/acceptance/bundle/resources/vector_search_indexes/schema_normalization/test.toml b/acceptance/bundle/resources/vector_search_indexes/schema_normalization/test.toml deleted file mode 100644 index 18b1a88417e..00000000000 --- a/acceptance/bundle/resources/vector_search_indexes/schema_normalization/test.toml +++ /dev/null @@ -1 +0,0 @@ -Cloud = false From c37c5b3f7e6353cec13bf4387edf4e8d9777536c Mon Sep 17 00:00:00 2001 From: Jan Rose Date: Mon, 24 Aug 2026 11:20:54 +0200 Subject: [PATCH 2/2] acc: CloudSlow only narrows Cloud = true tests Previously CloudSlow implied Cloud: any test with CloudSlow = true was flipped to Cloud = true during config materialisation and in the runner's skip logic, even when Cloud was explicitly false. This made CloudSlow a second, surprising way to opt a test into the cloud run and let an inherited CloudSlow = true silently override an explicit Cloud = false. Make CloudSlow only narrow an already-enabled cloud run: it is honoured only when Cloud = true, where it skips the cloud run under -short. The runner now gates on Cloud alone and applies the -short skip afterwards. Tests that reached the cloud run solely via the implication now set Cloud = true explicitly to preserve their behaviour. The vector_search_indexes subtests that set CloudSlow = false purely to defeat the old implication drop that workaround, since Cloud = false alone now means local-only. Co-authored-by: Isaac --- .agents/rules/testing.md | 8 +++---- acceptance/README.md | 2 +- acceptance/acceptance_test.go | 23 ++++--------------- acceptance/bundle/integration_whl/test.toml | 1 + .../clusters/run/spark_python_task/test.toml | 1 + .../resources/permissions/factcheck/test.toml | 1 + .../lifecycle-started-edit/test.toml | 4 +++- .../drift/deleted_remotely/out.test.toml | 2 +- .../drift/deleted_remotely/test.toml | 2 -- .../drift/orphaned_endpoint/out.test.toml | 2 +- .../drift/orphaned_endpoint/test.toml | 2 -- .../recreate/pending_deletion/out.test.toml | 2 +- .../recreate/pending_deletion/test.toml | 2 -- .../recreate/with_endpoint/out.test.toml | 2 +- .../recreate/with_endpoint/test.toml | 2 -- acceptance/bundle/run/app-with-job/test.toml | 1 + acceptance/internal/config.go | 4 ++-- acceptance/test.toml | 3 ++- 18 files changed, 25 insertions(+), 39 deletions(-) diff --git a/.agents/rules/testing.md b/.agents/rules/testing.md index bbfd1ce5e50..1142cb0b8dc 100644 --- a/.agents/rules/testing.md +++ b/.agents/rules/testing.md @@ -56,9 +56,9 @@ When writing tests, don't include an explanation in each test case in your respo ## Acceptance Tests -**RULE: `Cloud`/`CloudSlow` add a cloud run; they never remove the local run.** Every test under `acceptance/` runs locally against the fake server in `libs/testserver`. `Cloud = true` in a `test.toml` or `out.test.toml` means "this test *also* runs against a real workspace when `CLOUD_ENV` is set" — it never means "this test does not run locally". Never tell the user a test doesn't run locally because it is `Cloud = true`. +**RULE: `Cloud = true` adds a cloud run; it never removes the local run.** Every test under `acceptance/` runs locally against the fake server in `libs/testserver`. `Cloud = true` in a `test.toml` or `out.test.toml` means "this test *also* runs against a real workspace when `CLOUD_ENV` is set" — it never means "this test does not run locally". Never tell the user a test doesn't run locally because it is `Cloud = true`. `CloudSlow` does *not* enable a cloud run on its own; it only narrows an existing `Cloud = true` run (see below). -The whole `Cloud*` family lives inside an `if isRunningOnCloud` branch in `getSkipReason` (`acceptance/acceptance_test.go`), so it can only ever subtract from the cloud run: `CloudSlow` drops it under `-short`, and `CloudEnvs` / `RequiresUnityCatalog` / `RequiresCluster` / `RequiresWarehouse` narrow it to environments that have the prerequisite. To find what skips a test *locally*, look at a different set: `GOOS`, `RunsOnDbr`, and `DATABRICKS_TEST_SKIPLOCAL` (which cloud CI runs set precisely because those tests already ran locally). +The whole `Cloud*` family lives inside an `if isRunningOnCloud` branch in `getSkipReason` (`acceptance/acceptance_test.go`), so it can only ever subtract from the cloud run: `CloudSlow` (only meaningful when `Cloud = true`) drops it under `-short`, and `CloudEnvs` / `RequiresUnityCatalog` / `RequiresCluster` / `RequiresWarehouse` narrow it to environments that have the prerequisite. To find what skips a test *locally*, look at a different set: `GOOS`, `RunsOnDbr`, and `DATABRICKS_TEST_SKIPLOCAL` (which cloud CI runs set precisely because those tests already ran locally). `Cloud` is inherited, so a parent `test.toml` can opt a whole subtree in; a leaf `test.toml` with no `Cloud` line is not evidence of anything. Read the generated `out.test.toml` for a test's effective settings. @@ -127,14 +127,14 @@ Ignore = ["databricks.yml"] # parsed as EnvMatrix.Ignore, not top-level Ignore ### Reference -- Tests live in `acceptance/` with a nested directory structure. All of them run locally; those with `Cloud`/`CloudSlow` set also run against a real workspace. +- Tests live in `acceptance/` with a nested directory structure. All of them run locally; those with `Cloud = true` set also run against a real workspace. - Each test directory contains `databricks.yml`, `script`, and `output.txt`. - Source files: `test.toml`, `script`, `script.prepare`, `databricks.yml`, etc. - Tests are configured via `test.toml`. Config schema and explanation is in `acceptance/internal/config.go`. Certain options are also dumped to `out.test.toml` so that inherited values are visible on PRs. - Run a single test: `go test ./acceptance -run TestAccept/bundle///` - Run a specific variant by appending `EnvMatrix` values to the test name: `go test ./acceptance -run 'TestAccept/.../DATABRICKS_BUNDLE_ENGINE=direct'`. When there are multiple `EnvMatrix` variables, they appear in alphabetical order. - Useful flags: `-v` for verbose output, `-tail` to follow test output (requires `-v`), `-logrequests` to log all HTTP requests/responses (requires `-v`). -- Run tests on cloud: `deco env run -i -n aws-prod-ucws -- ` (requires `deco` tool and access to test env). This is an *additional* pass over the same test directories, restricted to those with `Cloud`/`CloudSlow` set; it does not replace the local run. +- Run tests on cloud: `deco env run -i -n aws-prod-ucws -- ` (requires `deco` tool and access to test env). This is an *additional* pass over the same test directories, restricted to those with `Cloud = true` set; it does not replace the local run. - `script.prepare` files from parent directories are concatenated into the test script. Use them for shared bash helpers. ### Built-in shell helpers diff --git a/acceptance/README.md b/acceptance/README.md index bf38fd1c025..d3288382f97 100644 --- a/acceptance/README.md +++ b/acceptance/README.md @@ -6,7 +6,7 @@ Acceptance tests are blackbox tests that are run against compiled binary. A test can *additionally* opt into running against a real workspace by setting `Cloud = true` in its `test.toml`. That is an extra run, not a different one: `Cloud = true` never means "this test does not run locally". Related settings: -- `CloudSlow = true` implies `Cloud = true`, but the cloud run is skipped when `-short` is passed. +- `CloudSlow = true` only narrows a `Cloud = true` test: its cloud run is skipped when `-short` is passed. On its own (without `Cloud = true`) it does not enable the cloud run. - `CloudEnvs` and `RequiresUnityCatalog` / `RequiresCluster` / `RequiresWarehouse` only narrow the cloud run further. They are ignored locally. - `Cloud` is inherited from parent `test.toml` files, so a whole subtree can be opted in at once. The root `acceptance/test.toml` defaults it to `Cloud = false`, i.e. local only. - Each test's effective settings are visible in its generated `out.test.toml`. diff --git a/acceptance/acceptance_test.go b/acceptance/acceptance_test.go index 957cacac65c..69fd5a02588 100644 --- a/acceptance/acceptance_test.go +++ b/acceptance/acceptance_test.go @@ -499,12 +499,6 @@ func testAccept(t *testing.T, inprocessMode bool, singleTest string) int { t.Fatalf("Invalid config %s: %s", configPath, err) } - // Apply default: CloudSlow implies Cloud. Do this before generating - // the materialized config so the implication is visible in out.test.toml. - if isTruePtr(config.CloudSlow) { - config.Cloud = config.CloudSlow - } - // Generate materialized config for this test. // We do this before skipping the test, so the configs are generated for all tests. materializedConfig := internal.GenerateMaterializedConfig(&config) @@ -686,20 +680,13 @@ func getSkipReason(config *internal.TestConfig, configPath, dir, skipLocalMode s return fmt.Sprintf("Disabled via CloudEnvs.%s setting in %s (CLOUD_ENV=%s)", cloudEnvBase, configPath, cloudEnv) } - if isTruePtr(config.CloudSlow) { - if testing.Short() { - return fmt.Sprintf("Disabled via CloudSlow setting in %s (CLOUD_ENV=%s, Short=%v)", configPath, cloudEnv, testing.Short()) - } + if !isTruePtr(config.Cloud) { + return fmt.Sprintf("Disabled via Cloud setting in %s (CLOUD_ENV=%s)", configPath, cloudEnv) } - isCloudEnabled := isTruePtr(config.Cloud) || isTruePtr(config.CloudSlow) - if !isCloudEnabled { - return fmt.Sprintf("Disabled via Cloud/CloudSlow setting in %s (CLOUD_ENV=%s, Cloud=%v, CloudSlow=%v)", - configPath, - cloudEnv, - isTruePtr(config.Cloud), - isTruePtr(config.CloudSlow), - ) + // CloudSlow only narrows an already-enabled cloud run: skip it under -short. + if isTruePtr(config.CloudSlow) && testing.Short() { + return fmt.Sprintf("Disabled via CloudSlow setting in %s (CLOUD_ENV=%s, Short=%v)", configPath, cloudEnv, testing.Short()) } if isTruePtr(config.RequiresUnityCatalog) && os.Getenv("TEST_METASTORE_ID") == "" { diff --git a/acceptance/bundle/integration_whl/test.toml b/acceptance/bundle/integration_whl/test.toml index bfb7b4105e1..6b36b680b92 100644 --- a/acceptance/bundle/integration_whl/test.toml +++ b/acceptance/bundle/integration_whl/test.toml @@ -1,3 +1,4 @@ +Cloud = true CloudSlow = true # Workspace file system does not allow initializing python envs on it. diff --git a/acceptance/bundle/resources/clusters/run/spark_python_task/test.toml b/acceptance/bundle/resources/clusters/run/spark_python_task/test.toml index ba4690ab1ae..5172de730ad 100644 --- a/acceptance/bundle/resources/clusters/run/spark_python_task/test.toml +++ b/acceptance/bundle/resources/clusters/run/spark_python_task/test.toml @@ -1,3 +1,4 @@ +Cloud = true CloudSlow = true RecordRequests = false RunsOnDbr = true diff --git a/acceptance/bundle/resources/permissions/factcheck/test.toml b/acceptance/bundle/resources/permissions/factcheck/test.toml index a2dd24a5fc0..f194bca8601 100644 --- a/acceptance/bundle/resources/permissions/factcheck/test.toml +++ b/acceptance/bundle/resources/permissions/factcheck/test.toml @@ -1,3 +1,4 @@ +Cloud = true CloudSlow = true RecordRequests = false RunsOnDbr = true diff --git a/acceptance/bundle/resources/sql_warehouses/lifecycle-started-edit/test.toml b/acceptance/bundle/resources/sql_warehouses/lifecycle-started-edit/test.toml index 97922e069d7..d418e49465a 100644 --- a/acceptance/bundle/resources/sql_warehouses/lifecycle-started-edit/test.toml +++ b/acceptance/bundle/resources/sql_warehouses/lifecycle-started-edit/test.toml @@ -1,7 +1,9 @@ RecordRequests = true # Starting warehouses is slow, so run on cloud nightly (CloudSlow) instead of every PR -# to confirm the real Edit-restarts behaviour without per-PR timeouts. +# to confirm the real Edit-restarts behaviour without per-PR timeouts. The parent +# disables Cloud, so re-enable it here alongside CloudSlow. +Cloud = true CloudSlow = true Ignore = [".databricks", "databricks.yml"] diff --git a/acceptance/bundle/resources/vector_search_indexes/drift/deleted_remotely/out.test.toml b/acceptance/bundle/resources/vector_search_indexes/drift/deleted_remotely/out.test.toml index 20b5ce022d7..7d05ff4fd32 100644 --- a/acceptance/bundle/resources/vector_search_indexes/drift/deleted_remotely/out.test.toml +++ b/acceptance/bundle/resources/vector_search_indexes/drift/deleted_remotely/out.test.toml @@ -1,4 +1,4 @@ Cloud = false -CloudSlow = false +CloudSlow = true RequiresUnityCatalog = true EnvMatrix.DATABRICKS_BUNDLE_ENGINE = ["direct"] diff --git a/acceptance/bundle/resources/vector_search_indexes/drift/deleted_remotely/test.toml b/acceptance/bundle/resources/vector_search_indexes/drift/deleted_remotely/test.toml index 0fff24e5ff0..18b1a88417e 100644 --- a/acceptance/bundle/resources/vector_search_indexes/drift/deleted_remotely/test.toml +++ b/acceptance/bundle/resources/vector_search_indexes/drift/deleted_remotely/test.toml @@ -1,3 +1 @@ Cloud = false -# The parent sets CloudSlow=true; disable it here too, otherwise CloudSlow would imply Cloud=true. -CloudSlow = false diff --git a/acceptance/bundle/resources/vector_search_indexes/drift/orphaned_endpoint/out.test.toml b/acceptance/bundle/resources/vector_search_indexes/drift/orphaned_endpoint/out.test.toml index 20b5ce022d7..7d05ff4fd32 100644 --- a/acceptance/bundle/resources/vector_search_indexes/drift/orphaned_endpoint/out.test.toml +++ b/acceptance/bundle/resources/vector_search_indexes/drift/orphaned_endpoint/out.test.toml @@ -1,4 +1,4 @@ Cloud = false -CloudSlow = false +CloudSlow = true RequiresUnityCatalog = true EnvMatrix.DATABRICKS_BUNDLE_ENGINE = ["direct"] diff --git a/acceptance/bundle/resources/vector_search_indexes/drift/orphaned_endpoint/test.toml b/acceptance/bundle/resources/vector_search_indexes/drift/orphaned_endpoint/test.toml index 0fff24e5ff0..18b1a88417e 100644 --- a/acceptance/bundle/resources/vector_search_indexes/drift/orphaned_endpoint/test.toml +++ b/acceptance/bundle/resources/vector_search_indexes/drift/orphaned_endpoint/test.toml @@ -1,3 +1 @@ Cloud = false -# The parent sets CloudSlow=true; disable it here too, otherwise CloudSlow would imply Cloud=true. -CloudSlow = false diff --git a/acceptance/bundle/resources/vector_search_indexes/recreate/pending_deletion/out.test.toml b/acceptance/bundle/resources/vector_search_indexes/recreate/pending_deletion/out.test.toml index 20b5ce022d7..7d05ff4fd32 100644 --- a/acceptance/bundle/resources/vector_search_indexes/recreate/pending_deletion/out.test.toml +++ b/acceptance/bundle/resources/vector_search_indexes/recreate/pending_deletion/out.test.toml @@ -1,4 +1,4 @@ Cloud = false -CloudSlow = false +CloudSlow = true RequiresUnityCatalog = true EnvMatrix.DATABRICKS_BUNDLE_ENGINE = ["direct"] diff --git a/acceptance/bundle/resources/vector_search_indexes/recreate/pending_deletion/test.toml b/acceptance/bundle/resources/vector_search_indexes/recreate/pending_deletion/test.toml index 2b33ed3650a..3ed9d70679f 100644 --- a/acceptance/bundle/resources/vector_search_indexes/recreate/pending_deletion/test.toml +++ b/acceptance/bundle/resources/vector_search_indexes/recreate/pending_deletion/test.toml @@ -1,4 +1,2 @@ # The two-phase deletion race is simulated by the testserver, so this only runs locally. Cloud = false -# The parent sets CloudSlow=true; disable it here too, otherwise CloudSlow would imply Cloud=true. -CloudSlow = false diff --git a/acceptance/bundle/resources/vector_search_indexes/recreate/with_endpoint/out.test.toml b/acceptance/bundle/resources/vector_search_indexes/recreate/with_endpoint/out.test.toml index 20b5ce022d7..7d05ff4fd32 100644 --- a/acceptance/bundle/resources/vector_search_indexes/recreate/with_endpoint/out.test.toml +++ b/acceptance/bundle/resources/vector_search_indexes/recreate/with_endpoint/out.test.toml @@ -1,4 +1,4 @@ Cloud = false -CloudSlow = false +CloudSlow = true RequiresUnityCatalog = true EnvMatrix.DATABRICKS_BUNDLE_ENGINE = ["direct"] diff --git a/acceptance/bundle/resources/vector_search_indexes/recreate/with_endpoint/test.toml b/acceptance/bundle/resources/vector_search_indexes/recreate/with_endpoint/test.toml index c36f7b6033c..63009a307fe 100644 --- a/acceptance/bundle/resources/vector_search_indexes/recreate/with_endpoint/test.toml +++ b/acceptance/bundle/resources/vector_search_indexes/recreate/with_endpoint/test.toml @@ -1,5 +1,3 @@ Cloud = false -# The parent sets CloudSlow=true; disable it here too, otherwise CloudSlow would imply Cloud=true. -CloudSlow = false Badness = "Recreating a vector_search_endpoint does not cascade to its dependent vector_search_indexes. The first deploy recreates the endpoint and leaves the index attached to a stale endpoint UUID; a second plan/deploy is required to reconcile the index. Follow-up will add a generic per-resource recreate cascade rule to the framework." diff --git a/acceptance/bundle/run/app-with-job/test.toml b/acceptance/bundle/run/app-with-job/test.toml index 3c5f533fa51..beead4f5cb0 100644 --- a/acceptance/bundle/run/app-with-job/test.toml +++ b/acceptance/bundle/run/app-with-job/test.toml @@ -5,6 +5,7 @@ # - 10-20 seconds to deploy the bundle # This test includes 2 application starts, and it is taking aroung 420 seconds to complete the entire test # +Cloud = true CloudSlow = true Ignore = [ diff --git a/acceptance/internal/config.go b/acceptance/internal/config.go index 0415b51b776..7e4a2e83fc1 100644 --- a/acceptance/internal/config.go +++ b/acceptance/internal/config.go @@ -52,8 +52,8 @@ type TestConfig struct { // Does not affect the local run, which happens either way. Cloud *bool - // Like Cloud, but the cloud run is skipped when -short is passed. - // This also sets -tail when -v is passed. Implies Cloud. + // Only meaningful alongside Cloud=true: the cloud run is skipped when -short is passed. + // This also sets -tail when -v is passed. It does not enable the cloud run on its own. CloudSlow *bool // If true and Cloud=true, run the cloud part of this test only if unity catalog is available in the cloud environment diff --git a/acceptance/test.toml b/acceptance/test.toml index c2eff20d6bd..5fca2e77234 100644 --- a/acceptance/test.toml +++ b/acceptance/test.toml @@ -1,6 +1,7 @@ # Default settings that apply to all tests unless overriden by test.toml files in inner directories. -# Tests always run locally; set Cloud/CloudSlow to also run on a real workspace. +# Tests always run locally; set Cloud = true to also run on a real workspace. # Cloud = false (the inherited default) therefore means local only. +# CloudSlow only narrows a Cloud = true run (skipped under -short); it does not enable cloud on its own. Cloud = false # default timeouts