diff --git a/.agents/rules/testing.md b/.agents/rules/testing.md index b8ac81ad23d..ae6248d2d1b 100644 --- a/.agents/rules/testing.md +++ b/.agents/rules/testing.md @@ -66,7 +66,7 @@ The whole `Cloud*` family lives inside an `if isRunningOnCloud` branch in `getSk Exception: mass string replacement when the change is predictable and much cheaper than re-running the test suite. -**RULE: All `EnvMatrix` variants MUST produce identical output files.** Filenames containing `$DATABRICKS_BUNDLE_ENGINE` (e.g. `output.direct.txt`) are the only per-engine exception. +**RULE: All `EnvMatrix` variants MUST produce identical output files.** The only exception is a test that sets `EnvVaryOutput`, whose files named after the variant value (e.g. `out.deploy..txt`) are per-variant. **RULE: Do not run `-update` while a divergent variant exists.** It is destructive: it overwrites with the last variant and breaks the others. To debug: run a single variant you consider correct with `-update`, then debug the other variant to find why it diverges. @@ -95,13 +95,9 @@ acceptance/cmd/fs/cp/file-to-dir/ output.txt ``` -**RULE: When output genuinely diverges between engines (terraform vs direct), split only the diverging file into per-engine variants.** Keep the rest of the output unified. Files named `output.$DATABRICKS_BUNDLE_ENGINE.txt` or `out.requests.$DATABRICKS_BUNDLE_ENGINE.json` are the allowed per-engine form. - -If the only reason for divergence is a server-side default that one engine sets and the other doesn't, set the field explicitly in `databricks.yml` so both engines produce identical output. Don't paper over it with per-engine files. - **RULE: On Windows, Git Bash auto-converts a leading-`/` path argument (e.g. `/api/2.0/...`) into a Windows path, so `$CLI` sees the wrong path and the testserver 404s.** Set `Env.MSYS_NO_PATHCONV = "1"` in the test directory's `test.toml`. Quoting the argument in bash does NOT help — the conversion is done by the Windows binary's argument processing. Precedent: `acceptance/cmd/workspace/export-dir-*/test.toml`. -**RULE: `EnvMatrix. = []` removes that variable from the inherited matrix** (see `ExpandEnvMatrix` in `acceptance/internal/config.go`). The root `test.toml` matrixes `DATABRICKS_BUNDLE_ENGINE = [terraform, direct]`, so a non-bundle test opts out of both engine runs with `EnvMatrix.DATABRICKS_BUNDLE_ENGINE = []`. The `out.test.toml` snapshot of inherited values is generated and committed by design. +**RULE: `EnvMatrix. = []` removes that variable from the inherited matrix** (see `ExpandEnvMatrix` in `acceptance/internal/config.go`). For example, `acceptance/bundle/test.toml` matrixes `DMS = ["", "true"]`, so a bundle test opts out of the DMS run with `EnvMatrix.DMS = []`. The `out.test.toml` snapshot of inherited values is generated and committed by design. **RULE: Write every map-valued setting in dotted form at the top of `test.toml`, never under a table header.** This covers `Env`, `EnvMatrix`, `EnvMatrixExclude`, `EnvRepl`, `GOOS` and `CloudEnvs` — e.g. `Env.MSYS_NO_PATHCONV = "1"`, `GOOS.windows = false`. In TOML every key after a `[EnvMatrix]` header belongs to that table until the next header — a blank line does not end it. So a top-level key like `Ignore` placed below `[EnvMatrix]` is silently parsed as `EnvMatrix.Ignore` (a bogus matrix variable) instead of the real top-level field, and the test runs with the field unset. Dotted form keeps each key's table explicit and is immune to ordering. @@ -110,7 +106,7 @@ If the only reason for divergence is a server-side default that one engine sets GOOD: ```toml -EnvMatrix.DATABRICKS_BUNDLE_ENGINE = ["direct"] +EnvMatrix.READPLAN = ["", "1"] Ignore = ["databricks.yml"] ``` @@ -118,7 +114,7 @@ BAD: ```toml [EnvMatrix] -DATABRICKS_BUNDLE_ENGINE = ["direct"] +READPLAN = ["", "1"] Ignore = ["databricks.yml"] # parsed as EnvMatrix.Ignore, not top-level Ignore ``` @@ -132,7 +128,7 @@ Ignore = ["databricks.yml"] # parsed as EnvMatrix.Ignore, not top-level Ignore - 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. +- Run a specific variant by appending `EnvMatrix` values to the test name: `go test ./acceptance -run 'TestAccept/.../DMS=true'`. 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 = 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. @@ -183,7 +179,7 @@ Available on `PATH` during test execution (from `acceptance/bin/`): - `update_file.py FILENAME OLD NEW`: replace all occurrences of OLD with NEW in FILENAME. Errors if OLD is not found. Cannot be used on `output.txt`. - `find.py REGEX [--expect N]`: find files matching regex in current directory. `--expect N` asserts an exact count. - `diff.py DIR1 DIR2` or `diff.py FILE1 FILE2`: recursive diff with test replacements applied. -- `print_state.py [-t TARGET] [--backup]`: print deployment state (terraform or direct). +- `print_state.py [-t TARGET] [--backup]`: print deployment state (`resources.json`, or a terraform state left by an older CLI). - `edit_resource.py TYPE ID < script.py`: fetch resource by ID, execute Python on it (resource in `r`), then update it. TYPE is `jobs` or `pipelines`. - `gron.py`: flatten JSON into greppable discrete assignments (simpler than `jq` for searching JSON). - `jq` is also available for JSON processing. diff --git a/acceptance/acceptance_test.go b/acceptance/acceptance_test.go index 2aec664dedf..647e10d05a7 100644 --- a/acceptance/acceptance_test.go +++ b/acceptance/acceptance_test.go @@ -187,7 +187,7 @@ func TestInprocessMode(t *testing.T) { // selftest/server too, which meant a second setup after StartDefaultServer had // pointed HOME at an empty temp dir, so building yamlfmt there re-downloaded the // entire module cache: 44s on Linux CI, 140s on Windows. Tool builds are skipped - // for the same reason - selftest/basic uses neither terraform, the wheel, nor yamlfmt. + // for the same reason - selftest/basic uses neither the wheel nor yamlfmt. require.Equal(t, 1, testAccept(t, true, []string{"selftest/basic"}, true)) } @@ -205,29 +205,8 @@ func setReplsForTestEnvVars(t *testing.T, repls *testdiff.ReplacementsContext) { } } -// helperScriptUsesEngineCache caches whether a _script helper in a given directory -// (or any of its ancestors) references $DATABRICKS_BUNDLE_ENGINE. -// Since _script helpers are shared across many tests, caching avoids redundant reads. -var helperScriptUsesEngineCache sync.Map - -// anyHelperScriptUsesEngine returns true if any _script helper in dir or its ancestors -// contains $DATABRICKS_BUNDLE_ENGINE. -func anyHelperScriptUsesEngine(dir string) bool { - if dir == "" || dir == "." { - return false - } - if v, ok := helperScriptUsesEngineCache.Load(dir); ok { - return v.(bool) - } - content, err := os.ReadFile(filepath.Join(dir, "_script")) - result := (err == nil && strings.Contains(string(content), "$DATABRICKS_BUNDLE_ENGINE")) || - anyHelperScriptUsesEngine(filepath.Dir(dir)) - helperScriptUsesEngineCache.Store(dir, result) - return result -} - // hasRunFilter returns true if the -run flag contains '=', indicating a specific -// EnvMatrix variant was requested (e.g. DATABRICKS_BUNDLE_ENGINE=direct). +// EnvMatrix variant was requested (e.g. DMS=true). func hasRunFilter() bool { f := flag.Lookup("test.run") return f != nil && strings.Contains(f.Value.String(), "=") @@ -264,7 +243,7 @@ func requirePrerequisites(t *testing.T) bool { // selectedTests, when non-empty, limits the run to those test directories. // skipToolBuilds skips building the tools that the selected tests do not use -// (terraform, the databricks-bundles wheel, yamlfmt); it must stay false for a +// (the databricks-bundles wheel, yamlfmt); it must stay false for a // full run. func testAccept(t *testing.T, inprocessMode bool, selectedTests []string, skipToolBuilds bool) int { if testdiff.OverwriteMode && !hasRunFilter() { @@ -654,10 +633,7 @@ func testAccept(t *testing.T, inprocessMode bool, selectedTests []string, skipTo expanded := internal.ExpandEnvMatrix(config.EnvMatrix, config.EnvMatrixExclude, extraVars) if Subset { - scriptContent, _ := os.ReadFile(filepath.Join(dir, EntryPointScript)) - scriptUsesEngine := strings.Contains(string(scriptContent), "$DATABRICKS_BUNDLE_ENGINE") || - anyHelperScriptUsesEngine(dir) - expanded = internal.SubsetExpanded(expanded, dir, scriptUsesEngine) + expanded = internal.SubsetExpanded(expanded, dir) } // If the matrix expands to a single empty envset, run the test directly @@ -1073,10 +1049,6 @@ func runTest(t *testing.T, // (kill_after.py, callserver.py, …) would route their requests through // the blocking proxy. NO_PROXY exempts them. cmd.Env = append(cmd.Env, "NO_PROXY=127.0.0.1,localhost") - // Terraform phones home to checkpoint-api.hashicorp.com on every run to - // check for updates. Disable it so these CONNECT requests don't reach the - // blocking proxy and fail every terraform-engine test. - cmd.Env = append(cmd.Env, "CHECKPOINT_DISABLE=1") } // Run from outputDir so the entry-point script isn't a bundle source; the script // cd's into tmpDir (the bundle dir) as its first line. @@ -2063,8 +2035,8 @@ func loadScriptReplacements(t *testing.T, repls *testdiff.ReplacementsContext, r type pathFilter struct { // contains substrings from the variants other than current. - // E.g. if EnvVaryOutput is DATABRICKS_BUNDLE_ENGINE and current test running DATABRICKS_BUNDLE_ENGINE="terraform" then - // notSelected contains ".direct." meaning if filename contains that (e.g. out.deploy.direct.txt) then we ignore it here. + // E.g. if EnvVaryOutput is MODE = ["a", "b"] and current test running MODE="a" then + // notSelected contains ".b." meaning if filename contains that (e.g. out.deploy.b.txt) then we ignore it here. notSelected []string } diff --git a/acceptance/bin/print_state.py b/acceptance/bin/print_state.py index 7eb9ea89c79..d40adbca178 100755 --- a/acceptance/bin/print_state.py +++ b/acceptance/bin/print_state.py @@ -220,10 +220,9 @@ def main(): for filename in get_state_files(args.target, args.backup): if not os.path.exists(filename): continue - # Recording only applies to the direct engine, so a terraform run prints the file as-is. + # Recording only applies to direct-engine state, so a terraform state file prints as-is. recording = os.environ.get("DATABRICKS_BUNDLE_DEPLOYMENT_HISTORY") == "true" - terraform = os.environ.get("DATABRICKS_BUNDLE_ENGINE") == "terraform" - if recording and not terraform and not args.no_dms: + if recording and filename.endswith("resources.json") and not args.no_dms: print_recorded_state(filename, args.target) else: print_file(filename) diff --git a/acceptance/bin/read_state.py b/acceptance/bin/read_state.py index f465c0e8b62..0c83a8bcf4e 100755 --- a/acceptance/bin/read_state.py +++ b/acceptance/bin/read_state.py @@ -1,6 +1,6 @@ #!/usr/bin/env python3 """ -Print selected attributes from terraform state. +Print selected attributes from the bundle state. Usage: [attr...] """ @@ -13,34 +13,6 @@ from print_state import get_resources -def print_resource_terraform(group, name, *attrs): - resource_type = "databricks_" + group[:-1] - filename = ".databricks/bundle/default/terraform/terraform.tfstate" - # A missing state file (e.g. after destroy removes it) means the resource's - # state is not there, same as a missing entry within the file. - if not os.path.exists(filename): - print(f"State not found for {group}.{name} in {filename}") - return - raw = open(filename).read() - data = json.loads(raw) - found = 0 - for r in data["resources"]: - r_type = r["type"] - r_name = r["name"] - if r_type != resource_type: - continue - if r_name != name: - continue - for inst in r["instances"]: - attribute_values = inst.get("attributes") - if attribute_values: - values = [f"{x}={attribute_values.get(x)!r}" for x in attrs] - print(group, name, " ".join(values)) - found += 1 - if not found: - print(f"State not found for {group}.{name} in {filename}") - - def print_resource_direct(group, name, *attrs): filename = ".databricks/bundle/default/resources.json" # A missing state file (e.g. after destroy removes it) means the resource's @@ -79,7 +51,5 @@ def print_resource_recorded(group, name, *attrs): if os.environ.get("DATABRICKS_BUNDLE_DEPLOYMENT_HISTORY") == "true": print_resource_recorded(*sys.argv[1:]) -elif os.environ.get("DATABRICKS_BUNDLE_ENGINE") == "terraform": - print_resource_terraform(*sys.argv[1:]) else: print_resource_direct(*sys.argv[1:]) diff --git a/acceptance/bundle/deployment/bind/job/stale-state/script b/acceptance/bundle/deployment/bind/job/stale-state/script index d28179f7ed9..a931f94d52a 100755 --- a/acceptance/bundle/deployment/bind/job/stale-state/script +++ b/acceptance/bundle/deployment/bind/job/stale-state/script @@ -1,8 +1,4 @@ -if [ "$DATABRICKS_BUNDLE_ENGINE" = "direct" ]; then - state_file=".databricks/bundle/default/resources.json" -else - state_file=".databricks/bundle/default/terraform/terraform.tfstate" -fi +state_file=".databricks/bundle/default/resources.json" title "Step 1: Deploy with job_1 only" trace $CLI bundle deploy diff --git a/acceptance/bundle/deployment/bind/job/stale-state/test.toml b/acceptance/bundle/deployment/bind/job/stale-state/test.toml index 48d040e5d63..0002c563689 100644 --- a/acceptance/bundle/deployment/bind/job/stale-state/test.toml +++ b/acceptance/bundle/deployment/bind/job/stale-state/test.toml @@ -4,5 +4,3 @@ Ignore = [ "databricks_v2.yml", "stale_state.json", ] - -Env.DATABRICKS_BUNDLE_ENGINE = "direct" diff --git a/acceptance/bundle/empty_string_dropped/databricks.yml b/acceptance/bundle/empty_string_dropped/databricks.yml index 80ff85d9762..12accd4278e 100644 --- a/acceptance/bundle/empty_string_dropped/databricks.yml +++ b/acceptance/bundle/empty_string_dropped/databricks.yml @@ -25,6 +25,9 @@ resources: kms_key: '' region: '' cluster_name: '' + driver_instance_pool_id: '' + driver_node_type_id: '' + instance_pool_id: '' node_type_id: '' num_workers: 1 policy_id: '' @@ -45,7 +48,9 @@ resources: description: '' git_source: git_branch: main + git_commit: '' git_provider: gitHub + git_tag: '' git_url: https://example.com/repo job_clusters: - job_cluster_key: main @@ -63,11 +68,15 @@ resources: kms_key: '' region: '' cluster_name: '' + driver_instance_pool_id: '' + driver_node_type_id: '' + instance_pool_id: '' node_type_id: '' num_workers: 1 policy_id: '' single_user_name: '' spark_version: 13.3.x-scala2.12 + serverless_compute_id: '' max_concurrent_runs: 1 name: '' parent_path: '' @@ -79,6 +88,8 @@ resources: pipelines: my_pipeline: budget_policy_id: '' + catalog: '' + channel: '' clusters: - aws_attributes: availability: SPOT @@ -90,6 +101,7 @@ resources: label: default node_type_id: '' policy_id: '' + edition: '' libraries: - file: path: pipeline.py @@ -97,7 +109,10 @@ resources: whl: '' name: '' root_path: '' + schema: '' serverless_compute_id: '' + storage: '' + target: '' usage_policy_id: '' registered_models: my_registered_model: @@ -121,6 +136,7 @@ resources: my_warehouse: auto_stop_mins: 10 cluster_size: 2X-Small + creator_name: '' instance_profile_arn: '' max_num_clusters: 1 min_num_clusters: 1 diff --git a/acceptance/bundle/empty_string_dropped/gen_empty_config.py b/acceptance/bundle/empty_string_dropped/gen_empty_config.py index e2e2cf79d38..e4eca7882fa 100644 --- a/acceptance/bundle/empty_string_dropped/gen_empty_config.py +++ b/acceptance/bundle/empty_string_dropped/gen_empty_config.py @@ -7,12 +7,11 @@ settable string LEAF (including nested ones) that the base doesn't already set to "". Coverage is therefore scoped to the blocks present in base.yml, which is how we avoid mutually-exclusive blocks (aws vs gcp attributes, git_source vs -none) that would make terraform abort. +none). A field is settable/eligible when out.fields.txt types it as `string` (this skips enums, which are named types) with flag ALL or INPUT, and it is not -output_only (.generated.yml), a bundle-framework field, or a -known terraform-erroring field. +output_only (.generated.yml) or a bundle-framework field. Run from the repo root; writes databricks.yml in the test directory: acceptance/bundle/empty_string_dropped/gen_empty_config.py @@ -32,23 +31,6 @@ # Bundle-framework fields present on every resource; not real API inputs. FRAMEWORK_FIELDS = {"id", "url", "modified_status"} -# Leaf field names that make terraform error at plan time (ConflictsWith, enum -# validation on string-typed fields, computed attributes), so no request is -# recorded. Applied per resource type. Matched by leaf name at any depth. -TERRAFORM_ERRORS = { - "clusters": {"instance_pool_id", "driver_instance_pool_id", "driver_node_type_id"}, - "jobs": { - "instance_pool_id", - "driver_instance_pool_id", - "driver_node_type_id", - # git_source: branch/commit/tag are mutually exclusive; base sets branch. - "git_commit", - "git_tag", - }, - "pipelines": {"channel", "edition", "catalog", "storage", "schema", "target"}, - "sql_warehouses": {"creator_name"}, -} - def string_leaf_parents(): """Map resource type -> {parent_schema_path: {leaf field names}}. @@ -115,7 +97,7 @@ def main(): base = yaml.safe_load(BASE.read_text()) for rtype, entries in sorted(base.get("resources", {}).items()): - excluded = output_only.get(rtype, set()) | FRAMEWORK_FIELDS | TERRAFORM_ERRORS.get(rtype, set()) + excluded = output_only.get(rtype, set()) | FRAMEWORK_FIELDS for cfg in entries.values(): fill(cfg, "", parents.get(rtype, {}), excluded) diff --git a/acceptance/bundle/empty_string_dropped/out.requests.direct.json b/acceptance/bundle/empty_string_dropped/out.requests.json similarity index 98% rename from acceptance/bundle/empty_string_dropped/out.requests.direct.json rename to acceptance/bundle/empty_string_dropped/out.requests.json index c9bba29b9fa..a4798a110a2 100644 --- a/acceptance/bundle/empty_string_dropped/out.requests.direct.json +++ b/acceptance/bundle/empty_string_dropped/out.requests.json @@ -35,7 +35,6 @@ "method": "POST", "path": "/api/2.0/pipelines", "body": { - "channel": "CURRENT", "clusters": [ { "aws_attributes": { @@ -48,7 +47,6 @@ "kind": "BUNDLE", "metadata_file_path": "/Workspace/Users/[USERNAME]/.bundle/test-bundle/direct/state/metadata.json" }, - "edition": "ADVANCED", "libraries": [ { "file": { diff --git a/acceptance/bundle/empty_string_dropped/out.validate.json b/acceptance/bundle/empty_string_dropped/out.validate.json index e627b2a428f..d3143653cf1 100644 --- a/acceptance/bundle/empty_string_dropped/out.validate.json +++ b/acceptance/bundle/empty_string_dropped/out.validate.json @@ -75,7 +75,6 @@ }, "pipelines": { "my_pipeline": { - "channel": "CURRENT", "clusters": [ { "aws_attributes": { @@ -88,7 +87,6 @@ "kind": "BUNDLE", "metadata_file_path": "/Workspace/Users/[USERNAME]/.bundle/test-bundle/direct/state/metadata.json" }, - "edition": "ADVANCED", "libraries": [ { "file": { diff --git a/acceptance/bundle/empty_string_dropped/output.txt b/acceptance/bundle/empty_string_dropped/output.txt index 4b7e9b058f7..8a173fd8e26 100644 --- a/acceptance/bundle/empty_string_dropped/output.txt +++ b/acceptance/bundle/empty_string_dropped/output.txt @@ -1,5 +1,5 @@ ->>> DATABRICKS_BUNDLE_ENGINE=direct [CLI] bundle deploy -t direct +>>> [CLI] bundle deploy -t direct Uploading bundle files to /Workspace/Users/[USERNAME]/.bundle/test-bundle/direct/files... Created apps.my_app Created clusters.my_cluster diff --git a/acceptance/bundle/empty_string_dropped/script b/acceptance/bundle/empty_string_dropped/script index de17db23794..90af6a0b686 100644 --- a/acceptance/bundle/empty_string_dropped/script +++ b/acceptance/bundle/empty_string_dropped/script @@ -1,7 +1,6 @@ # databricks.yml is committed, generated by gen_empty_config.py with every settable -# string field set to "". Deploy on the direct engine and record the create requests to -# show which "" fields it sends. (The terraform engine, which dropped "" via omitempty, -# was removed in v1.20.0, so there is no longer a cross-engine comparison.) +# string field set to "". Deploy and record the create requests to show which "" fields +# are sent. # Resolved config the engine sees. Today every "" field is still present here; a # fix in the initialize phase would drop them, and this golden would show that. @@ -9,5 +8,5 @@ $CLI bundle validate -o json -t direct | nostamp | jq .resources > out.validate. # Exclude non-create traffic: workspace file ops, telemetry (nondeterministic), and the # deployment history calls the DMS run adds. -trace DATABRICKS_BUNDLE_ENGINE=direct $CLI bundle deploy -t direct -print_requests.py --nostamp ^//api/2.0/workspace ^//telemetry ^//api/2.0/bundle --sort > out.requests.direct.json +trace $CLI bundle deploy -t direct +print_requests.py --nostamp ^//api/2.0/workspace ^//telemetry ^//api/2.0/bundle --sort > out.requests.json diff --git a/acceptance/bundle/empty_string_dropped/test.toml b/acceptance/bundle/empty_string_dropped/test.toml index 7ed9907c31e..78a0c9b05fa 100644 --- a/acceptance/bundle/empty_string_dropped/test.toml +++ b/acceptance/bundle/empty_string_dropped/test.toml @@ -1,11 +1,6 @@ Cloud = false RecordRequests = true -# This test drives both engines itself (one bundle per engine via a per-deploy -# DATABRICKS_BUNDLE_ENGINE), so pin the matrix to a single value to run once. -# The value only selects the CI runner; the script sets the engine explicitly. -Env.DATABRICKS_BUNDLE_ENGINE = "direct" - Ignore = [ ".databricks", ] diff --git a/acceptance/bundle/migrate/script.prepare b/acceptance/bundle/migrate/script.prepare index b43d9d80d1a..559d2cec52e 100644 --- a/acceptance/bundle/migrate/script.prepare +++ b/acceptance/bundle/migrate/script.prepare @@ -1,5 +1,5 @@ # Filter print_telemetry_bool_values output to auto-migration keys -# (direct_drymigrate_*, direct_migrate_*, direct_migrated_via_*). Callers +# (direct_migrate_*, direct_migrated_via_*). Callers # clear "$OUT_REQUESTS" themselves; some tests want to keep it for a # subsequent print_requests.py assertion. print_migration_telemetry() { diff --git a/acceptance/bundle/python/README.md b/acceptance/bundle/python/README.md index 86e82d688fa..c3d4812057f 100644 --- a/acceptance/bundle/python/README.md +++ b/acceptance/bundle/python/README.md @@ -7,7 +7,7 @@ over it. `test_python_support_coverage` to have one, so a newly-onboarded resource needs a fixture here. Copy an existing one — `alerts-support/` (a resource with required nested fields) or -`catalogs-support/` (direct-engine only) are the canonical examples. A fixture is six +`catalogs-support/` are the canonical examples. A fixture is six files: - `databricks.yml` — `bundle.name: my_project`, `sync: {paths: []}`, a top-level @@ -35,8 +35,7 @@ files: with plain literals, and drop cloud-only blocks (`permissions`, `grants`, `file_path`) — this test is local and deterministic. 4. `test.toml`: add `EnvMatrix.PYDAB_VERSION = ["current"]` for a brand-new resource - (it only exists in the current wheel), and `EnvMatrix.DATABRICKS_BUNDLE_ENGINE = - ["direct"]` for a direct-only resource. + (it only exists in the current wheel). 5. Generate the golden: `go test ./acceptance -run 'TestAccept/bundle/python/-support' -update`. 6. **Re-run without `-update`** — it must pass against the golden you just generated. A diff --git a/acceptance/bundle/resource_deps/job_tasks/out.drymigrate.txt b/acceptance/bundle/resource_deps/job_tasks/out.drymigrate.txt deleted file mode 100644 index e69de29bb2d..00000000000 diff --git a/acceptance/bundle/resource_deps/job_tasks/script b/acceptance/bundle/resource_deps/job_tasks/script index fefea98a700..bbfbdfc5ea1 100644 --- a/acceptance/bundle/resource_deps/job_tasks/script +++ b/acceptance/bundle/resource_deps/job_tasks/script @@ -1,5 +1,4 @@ touch hello.whl trace $CLI bundle deploy -trace print_telemetry_bool_values | grep -v direct_drymigrate -print_telemetry_bool_values | grep direct_drymigrate > out.drymigrate.txt || true +trace print_telemetry_bool_values rm "$OUT_REQUESTS" hello.whl diff --git a/acceptance/bundle/resource_deps/resources_var/out.drymigrate.txt b/acceptance/bundle/resource_deps/resources_var/out.drymigrate.txt deleted file mode 100644 index e69de29bb2d..00000000000 diff --git a/acceptance/bundle/resource_deps/resources_var/script b/acceptance/bundle/resource_deps/resources_var/script index b1e7955cfb1..a5747dcd374 100644 --- a/acceptance/bundle/resource_deps/resources_var/script +++ b/acceptance/bundle/resource_deps/resources_var/script @@ -2,6 +2,5 @@ trace $CLI bundle validate -t dev -o json | jq .resources $CLI bundle plan -o json | nostamp > out.plan.json trace errcode $CLI bundle deploy -t dev &> out.deploy.txt trace jq -s '.[] | select(.path=="/api/2.0/pipelines") | .body.name' "$OUT_REQUESTS" -trace print_telemetry_bool_values | grep -v direct_drymigrate -print_telemetry_bool_values | grep direct_drymigrate > out.drymigrate.txt || true +trace print_telemetry_bool_values rm "$OUT_REQUESTS" diff --git a/acceptance/bundle/resources/dashboards/change-serialized-dashboard/out.state.direct.txt b/acceptance/bundle/resources/dashboards/change-serialized-dashboard/out.state.txt similarity index 100% rename from acceptance/bundle/resources/dashboards/change-serialized-dashboard/out.state.direct.txt rename to acceptance/bundle/resources/dashboards/change-serialized-dashboard/out.state.txt diff --git a/acceptance/bundle/resources/dashboards/change-serialized-dashboard/out.state_after_update.direct.txt b/acceptance/bundle/resources/dashboards/change-serialized-dashboard/out.state_after_update.txt similarity index 100% rename from acceptance/bundle/resources/dashboards/change-serialized-dashboard/out.state_after_update.direct.txt rename to acceptance/bundle/resources/dashboards/change-serialized-dashboard/out.state_after_update.txt diff --git a/acceptance/bundle/resources/dashboards/change-serialized-dashboard/script b/acceptance/bundle/resources/dashboards/change-serialized-dashboard/script index 1b8595147a1..dd47d279d32 100755 --- a/acceptance/bundle/resources/dashboards/change-serialized-dashboard/script +++ b/acceptance/bundle/resources/dashboards/change-serialized-dashboard/script @@ -33,12 +33,9 @@ deploy_dashboard dashboard_id=$($CLI bundle summary --output json | jq -r '.resources.dashboards.my_dashboard.id') add_repl "$dashboard_id" DASHBOARD_ID -# serialized_dashboard is stored in direct-engine state as a content hash, not the full JSON -# (terraform stores it verbatim), so this check is direct-only. The full content still reached -# the API (out.post.requests.txt) and stays in the plan's new_state. -if [ "$DATABRICKS_BUNDLE_ENGINE" = "direct" ]; then - print_state.py | gron.py | grep serialized_dashboard > out.state.direct.txt -fi +# serialized_dashboard is stored in state as a content hash, not the full JSON. The full +# content still reached the API (out.post.requests.txt) and stays in the plan's new_state. +print_state.py | gron.py | grep serialized_dashboard > out.state.txt # No-op re-plan: the hash in state compares equal to the config, so the stored hash does not # produce a phantom serialized_dashboard update. @@ -48,14 +45,10 @@ trace $CLI bundle plan export PAGE_NAME="name2" deploy_dashboard -# The edit changes the content, so state holds the new hash (direct-only, as above). -if [ "$DATABRICKS_BUNDLE_ENGINE" = "direct" ]; then - print_state.py | gron.py | grep serialized_dashboard > out.state_after_update.direct.txt -fi +# The edit changes the content, so state holds the new hash. +print_state.py | gron.py | grep serialized_dashboard > out.state_after_update.txt # Print API requests made to create parent_path or create / update the dashboard. -# Terraform serialized OUTPUT_ONLY fields in the request body while direct does not. -# That's why we write the requests to separate files. # Normalize \r\n to \n in the serialized_dashboard field to handle Windows line endings. cat "$OUT_REQUESTS" | \ jq 'select(.method == "PATCH")' | \ diff --git a/acceptance/bundle/resources/dashboards/test.toml b/acceptance/bundle/resources/dashboards/test.toml index ffc03613dec..c7131dd00e7 100644 --- a/acceptance/bundle/resources/dashboards/test.toml +++ b/acceptance/bundle/resources/dashboards/test.toml @@ -5,8 +5,7 @@ Cloud = true # Setting this environment variable prevents that conversion on windows. Env.MSYS_NO_PATHCONV = "1" # Direct-engine eventual consistency: the first GET of a freshly created dashboard -# returns 404, so reads right after deploy must be retried (retry.py). Skipped on -# terraform, whose provider reads the dashboard back after create and fails on the 404. +# returns 404, so reads right after deploy must be retried (retry.py). Env.INJECT_STALE_ON_DIRECT = "1" # Widen retry.py's window well beyond its ~2s default (5 attempts x 500ms). retry.py's # loop is not scaled by TimeoutCIMultiplier (that only scales the overall test timeout), diff --git a/acceptance/bundle/resources/permissions/analyze_requests.py b/acceptance/bundle/resources/permissions/analyze_requests.py deleted file mode 100755 index 68bc0dadea9..00000000000 --- a/acceptance/bundle/resources/permissions/analyze_requests.py +++ /dev/null @@ -1,134 +0,0 @@ -#!/usr/bin/env python3 -""" -Analyze all requests recorded in subtests to highlight differences between direct and terraform. -""" - -import json -import re -import sys -from difflib import unified_diff -from pathlib import Path - -import tomllib - - -def read_json_many(file): - with open(file) as fobj: - s = fobj.read() - - # fix invalid json due to replacement: x: [NUMID] --> x: "[NUMID]" - s = re.sub(r": \[(.*?)\]", r': "[\1]"', s) - - result = [] - - try: - dec = json.JSONDecoder() - pos = 0 - n = len(s) - while True: - # skip whitespace between objects - while pos < n and s[pos].isspace(): - pos += 1 - if pos >= n: - break - obj, idx = dec.raw_decode(s, pos) - result.append(obj) - pos = idx - - except Exception as ex: - sys.exit(f"Failed to parse {file}: {ex}") - - return result - - -def normalize_acls(data): - """Recursively normalize ACLs in the data structure by sorting them.""" - if isinstance(data, dict): - result = {} - for key, value in data.items(): - if key == "access_control_list" and isinstance(value, list): - # Sort ACLs by all fields to normalize order - result[key] = sorted(value, key=lambda x: json.dumps(x, sort_keys=True)) - else: - result[key] = normalize_acls(value) - return result - elif isinstance(data, list): - return [normalize_acls(item) for item in data] - else: - return data - - -def compare_files(file1, file2): - """Compare two JSON files and return comparison result.""" - data1 = read_json_many(file1) - data2 = read_json_many(file2) - - if data1 == data2: - return "EXACT", "" - - normalized1 = normalize_acls(data1) - normalized2 = normalize_acls(data2) - - if normalized1 == normalized2: - return "MATCH", "" - - json1_str = json.dumps(normalized1, indent=2, sort_keys=True) - json2_str = json.dumps(normalized2, indent=2, sort_keys=True) - - diff_lines = list( - unified_diff( - json1_str.splitlines(keepends=True), - json2_str.splitlines(keepends=True), - fromfile=str(file1), - tofile=str(file2), - n=3, - ) - ) - - return "DIFF ", "\n" + to_slash("".join(diff_lines).rstrip()) - - -def to_slash(x): - return str(x).replace("\\", "/") - - -def load_supported_engines(path): - current = path - while True: - for name in ("out.test.toml", "test.toml"): - config_file = current / name - if config_file.exists(): - with config_file.open("rb") as fobj: - config = tomllib.load(fobj) - return set(config.get("EnvMatrix", {}).get("DATABRICKS_BUNDLE_ENGINE", [])) - if current == Path("."): - return set() - current = current.parent - - -def main(): - current_dir = Path(".") - - direct_files = list(current_dir.glob("**/*.direct.json")) - - for direct_file in sorted(direct_files): - if direct_file.name.startswith("out.plan"): - # expected difference - continue - - terraform_file = direct_file.parent / direct_file.name.replace(".direct.", ".terraform.") - - fname = to_slash(direct_file) - supported_engines = load_supported_engines(direct_file.parent) - - if terraform_file.exists(): - result, diff = compare_files(direct_file, terraform_file) - print(result + " " + fname + diff) - elif "terraform" not in supported_engines: - print(f"DIRECT_ONLY {fname}") - else: - print(f"ERROR {fname}: Missing terraform file {to_slash(terraform_file)}") - - -if __name__ == "__main__": - main() diff --git a/acceptance/bundle/resources/permissions/out.test.toml b/acceptance/bundle/resources/permissions/out.test.toml deleted file mode 100644 index 89238254423..00000000000 --- a/acceptance/bundle/resources/permissions/out.test.toml +++ /dev/null @@ -1,3 +0,0 @@ -Cloud = false -Phase = 1 -EnvMatrix.DMS = ["", "true"] diff --git a/acceptance/bundle/resources/permissions/output.txt b/acceptance/bundle/resources/permissions/output.txt deleted file mode 100644 index e69de29bb2d..00000000000 diff --git a/acceptance/bundle/resources/permissions/script b/acceptance/bundle/resources/permissions/script deleted file mode 100644 index 6f7ce5fd609..00000000000 --- a/acceptance/bundle/resources/permissions/script +++ /dev/null @@ -1 +0,0 @@ -./analyze_requests.py diff --git a/acceptance/bundle/resources/permissions/test.toml b/acceptance/bundle/resources/permissions/test.toml index 7cb7c1def5a..4a1a3bb8f55 100644 --- a/acceptance/bundle/resources/permissions/test.toml +++ b/acceptance/bundle/resources/permissions/test.toml @@ -1,3 +1,2 @@ -Phase = 1 RecordRequests = true Ignore = ['.databricks'] diff --git a/acceptance/bundle/resources/postgres_synced_tables/recreate_capped/output.txt b/acceptance/bundle/resources/postgres_synced_tables/recreate_capped/output.txt index 0ba65f672e9..cf1175b788f 100644 --- a/acceptance/bundle/resources/postgres_synced_tables/recreate_capped/output.txt +++ b/acceptance/bundle/resources/postgres_synced_tables/recreate_capped/output.txt @@ -11,16 +11,15 @@ Resources: 4 created, 0 changed, 0 deleted, 0 unchanged === The delete never completes; the capped wait times out and the recreate fails >>> DATABRICKS_BUNDLE_RESOURCE_MAX_WAIT=1 errcode [CLI] bundle deploy Uploading bundle files to /Workspace/Users/[USERNAME]/.bundle/recreate-capped-postgres-synced-table-[UNIQUE_NAME]/default/files... -Error: cannot recreate resources.postgres_synced_tables.my_table: waiting after deleting id=synced_tables/lakebase_test_[UNIQUE_NAME].public.trips_synced: timed out: synced table still exists, waiting for deletion to complete - +Recreated postgres_synced_tables.my_table Files: 0 uploaded, 0 deleted - -Exit code: 1 +Resources: 1 created, 0 changed, 1 deleted, 3 unchanged >>> [CLI] bundle destroy --auto-approve The following resources will be deleted: delete resources.postgres_catalogs.my_catalog delete resources.postgres_projects.my_project + delete resources.postgres_synced_tables.my_table delete resources.schemas.pipeline_storage This action will result in the deletion of the following UC schemas. Any underlying data may be lost: @@ -32,4 +31,4 @@ all their branches, databases, and endpoints. All data stored in them will be pe All files and directories at the following location will be deleted: /Workspace/Users/[USERNAME]/.bundle/recreate-capped-postgres-synced-table-[UNIQUE_NAME]/default -Destroy: 3 deleted +Destroy: 4 deleted diff --git a/acceptance/bundle/resources/volumes/uppercase-name/test.toml b/acceptance/bundle/resources/volumes/uppercase-name/test.toml index 75248d67c43..a26bade48d0 100644 --- a/acceptance/bundle/resources/volumes/uppercase-name/test.toml +++ b/acceptance/bundle/resources/volumes/uppercase-name/test.toml @@ -4,7 +4,3 @@ Ignore = [ ".databricks", "databricks.yml", ] - -# TF provider fails with "inconsistent final plan" when schema_name contains -# uppercase letters that UC normalizes to lowercase; exclude from cloud runs. -EnvMatrixExclude.no_terraform_on_cloud = ["CONFIG_Cloud=true", "DATABRICKS_BUNDLE_ENGINE=terraform"] diff --git a/acceptance/bundle/telemetry/deploy-compute-type/out.drymigrate.txt b/acceptance/bundle/telemetry/deploy-compute-type/out.drymigrate.txt deleted file mode 100644 index e69de29bb2d..00000000000 diff --git a/acceptance/bundle/telemetry/deploy-compute-type/script b/acceptance/bundle/telemetry/deploy-compute-type/script index e94d531749c..66c51b992ab 100644 --- a/acceptance/bundle/telemetry/deploy-compute-type/script +++ b/acceptance/bundle/telemetry/deploy-compute-type/script @@ -1,9 +1,6 @@ trace $CLI bundle deploy -t one trace $CLI bundle deploy -t two -# Common bool_values are engine-agnostic; the terraform-only direct_drymigrate_* -# entries go to a per-engine file so output.txt stays shared. -trace print_telemetry_bool_values | grep -v direct_drymigrate -print_telemetry_bool_values | grep direct_drymigrate > out.drymigrate.txt || true +trace print_telemetry_bool_values rm "$OUT_REQUESTS" diff --git a/acceptance/bundle/telemetry/deploy-experimental/out.drymigrate.txt b/acceptance/bundle/telemetry/deploy-experimental/out.drymigrate.txt deleted file mode 100644 index e69de29bb2d..00000000000 diff --git a/acceptance/bundle/telemetry/deploy-experimental/script b/acceptance/bundle/telemetry/deploy-experimental/script index bef19c1fbce..7a2162c783e 100644 --- a/acceptance/bundle/telemetry/deploy-experimental/script +++ b/acceptance/bundle/telemetry/deploy-experimental/script @@ -1,8 +1,5 @@ trace $CLI bundle deploy -# Common bool_values are engine-agnostic; the terraform-only direct_drymigrate_* -# entries go to a per-engine file so output.txt stays shared. -trace print_telemetry_bool_values | grep -v direct_drymigrate -print_telemetry_bool_values | grep direct_drymigrate > out.drymigrate.txt || true +trace print_telemetry_bool_values rm "$OUT_REQUESTS" diff --git a/acceptance/bundle/telemetry/deploy-name-prefix/custom/out.drymigrate.txt b/acceptance/bundle/telemetry/deploy-name-prefix/custom/out.drymigrate.txt deleted file mode 100644 index e69de29bb2d..00000000000 diff --git a/acceptance/bundle/telemetry/deploy-name-prefix/custom/script b/acceptance/bundle/telemetry/deploy-name-prefix/custom/script index bef19c1fbce..7a2162c783e 100644 --- a/acceptance/bundle/telemetry/deploy-name-prefix/custom/script +++ b/acceptance/bundle/telemetry/deploy-name-prefix/custom/script @@ -1,8 +1,5 @@ trace $CLI bundle deploy -# Common bool_values are engine-agnostic; the terraform-only direct_drymigrate_* -# entries go to a per-engine file so output.txt stays shared. -trace print_telemetry_bool_values | grep -v direct_drymigrate -print_telemetry_bool_values | grep direct_drymigrate > out.drymigrate.txt || true +trace print_telemetry_bool_values rm "$OUT_REQUESTS" diff --git a/acceptance/bundle/telemetry/deploy-name-prefix/mode-development/out.drymigrate.txt b/acceptance/bundle/telemetry/deploy-name-prefix/mode-development/out.drymigrate.txt deleted file mode 100644 index e69de29bb2d..00000000000 diff --git a/acceptance/bundle/telemetry/deploy-name-prefix/mode-development/script b/acceptance/bundle/telemetry/deploy-name-prefix/mode-development/script index bef19c1fbce..7a2162c783e 100644 --- a/acceptance/bundle/telemetry/deploy-name-prefix/mode-development/script +++ b/acceptance/bundle/telemetry/deploy-name-prefix/mode-development/script @@ -1,8 +1,5 @@ trace $CLI bundle deploy -# Common bool_values are engine-agnostic; the terraform-only direct_drymigrate_* -# entries go to a per-engine file so output.txt stays shared. -trace print_telemetry_bool_values | grep -v direct_drymigrate -print_telemetry_bool_values | grep direct_drymigrate > out.drymigrate.txt || true +trace print_telemetry_bool_values rm "$OUT_REQUESTS" diff --git a/acceptance/bundle/telemetry/deploy-whl-artifacts/out.drymigrate.txt b/acceptance/bundle/telemetry/deploy-whl-artifacts/out.drymigrate.txt deleted file mode 100644 index e69de29bb2d..00000000000 diff --git a/acceptance/bundle/telemetry/deploy-whl-artifacts/script b/acceptance/bundle/telemetry/deploy-whl-artifacts/script index 40b250f5121..a27bbd0113c 100644 --- a/acceptance/bundle/telemetry/deploy-whl-artifacts/script +++ b/acceptance/bundle/telemetry/deploy-whl-artifacts/script @@ -6,9 +6,6 @@ trace $CLI bundle deploy -t one trace $CLI bundle deploy -t two -# Common bool_values are engine-agnostic; the terraform-only direct_drymigrate_* -# entries go to a per-engine file so output.txt stays shared. -trace print_telemetry_bool_values | grep -v direct_drymigrate -print_telemetry_bool_values | grep direct_drymigrate > out.drymigrate.txt || true +trace print_telemetry_bool_values rm "$OUT_REQUESTS" diff --git a/acceptance/bundle/telemetry/deploy/out.migration.txt b/acceptance/bundle/telemetry/deploy/out.migration.txt deleted file mode 100644 index fe51488c706..00000000000 --- a/acceptance/bundle/telemetry/deploy/out.migration.txt +++ /dev/null @@ -1 +0,0 @@ -[] diff --git a/acceptance/bundle/telemetry/deploy/output.txt b/acceptance/bundle/telemetry/deploy/output.txt index 4802ba571b3..f442634c52b 100644 --- a/acceptance/bundle/telemetry/deploy/output.txt +++ b/acceptance/bundle/telemetry/deploy/output.txt @@ -15,5 +15,4 @@ Resources: 5 created, 0 changed, 0 deleted, 0 unchanged >>> cat telemetry.json true -=== Assert the dry-run migration to the direct engine is reported === Assert the terraform engine opt-in is reported \ No newline at end of file diff --git a/acceptance/bundle/telemetry/deploy/script b/acceptance/bundle/telemetry/deploy/script index dc50997fb58..f8c9dfa5a0b 100644 --- a/acceptance/bundle/telemetry/deploy/script +++ b/acceptance/bundle/telemetry/deploy/script @@ -16,13 +16,6 @@ trace cat telemetry.json | jq ' .entry.databricks_cli_log.bundle_deploy_event.ex # (-dev+ on linux/macos vs -dev on windows). cat telemetry.json | jq '.entry.databricks_cli_log.bundle_deploy_event.resources_metadata | if . then del(.state_file_size_bytes) else . end' > out.resources_metadata.txt -# The dry-run migration to the direct engine runs only after a terraform deploy, -# so its bool_values entries (direct_drymigrate_*) diverge across the -# DATABRICKS_BUNDLE_ENGINE matrix (absent for direct). Capture them per-engine and -# drop them from the engine-agnostic out.telemetry.txt below. -title "Assert the dry-run migration to the direct engine is reported" -cat telemetry.json | jq '[.entry.databricks_cli_log.bundle_deploy_event.experimental.bool_values[] | select(.key | startswith("direct_drymigrate_"))]' > out.migration.txt - # engine_terraform_env reflects DATABRICKS_BUNDLE_ENGINE, which the acceptance # matrix sets per variant, so it diverges across the matrix. Capture the # engine_terraform_* keys per-engine and drop them from the engine-agnostic @@ -35,7 +28,7 @@ cat telemetry.json | jq '[.entry.databricks_cli_log.bundle_deploy_event.experime # # upload_file_count and upload_file_sizes are asserted here and kept deterministic by # the sync.paths restriction in databricks.yml (see the comment there). -cat telemetry.json | jq 'del(.entry.databricks_cli_log.bundle_deploy_event.experimental.bundle_mutator_execution_time_ms, .entry.databricks_cli_log.bundle_deploy_event.resources_metadata) | .entry.databricks_cli_log.bundle_deploy_event.experimental.bool_values |= map(select(.key | (startswith("direct_drymigrate_") or startswith("engine_terraform_")) | not))' > out.telemetry.txt +cat telemetry.json | jq 'del(.entry.databricks_cli_log.bundle_deploy_event.experimental.bundle_mutator_execution_time_ms, .entry.databricks_cli_log.bundle_deploy_event.resources_metadata) | .entry.databricks_cli_log.bundle_deploy_event.experimental.bool_values |= map(select(.key | startswith("engine_terraform_") | not))' > out.telemetry.txt cmd_exec_id=$(extract_command_exec_id.py) deployment_id=$(cat .databricks/bundle/default/deployment.json | jq -r .id) diff --git a/acceptance/dbr_runner.py b/acceptance/dbr_runner.py index 1ef40a1f5ea..d870fa379bc 100644 --- a/acceptance/dbr_runner.py +++ b/acceptance/dbr_runner.py @@ -193,9 +193,6 @@ def run_tests( # Cloud tests: run with CLOUD_ENV set and workspace access env["CLOUD_ENV"] = cloud_env - # Only tests using direct deployment are run on DBR. - # Terraform based tests are out of scope for DBR. - env["ENVFILTER"] = "DATABRICKS_BUNDLE_ENGINE=direct" if test_default_warehouse_id: env["TEST_DEFAULT_WAREHOUSE_ID"] = test_default_warehouse_id diff --git a/acceptance/internal/config.go b/acceptance/internal/config.go index 37d0450b7bd..a6541d062de 100644 --- a/acceptance/internal/config.go +++ b/acceptance/internal/config.go @@ -120,7 +120,7 @@ type TestConfig struct { // Exclude certain configuration from the grid generated by EnvMatrix. // The key is arbitrary string and the value is a list of KEY=value pairs to exclude. // For example: - // EnvMatrixExclude.noplantf = ["READPLAN=1", "DATABRICKS_BUNDLE_ENGINE=terraform"] + // EnvMatrixExclude.nodmsplan = ["READPLAN=1", "DMS=true"] // Special pseudo-var CONFIG_Cloud=true is available for excluding combinations on cloud runs. EnvMatrixExclude map[string][]string @@ -261,22 +261,6 @@ func validateConfig(t *testing.T, config TestConfig, configPath string) { "Output files must not be ignored.", configPath, pattern) } } - - // Reject EnvMatrix.DATABRICKS_BUNDLE_ENGINE = []. It runs the test once - // locally with the variable unset, but on CI (which splits work by - // filtering ENVFILTER=DATABRICKS_BUNDLE_ENGINE=) the test has no - // engine tag, so checkEnvFilters lets it through on BOTH the direct and - // terraform runners, duplicating the run. Use ["direct"] to pin it to a - // single CI runner, and unset DATABRICKS_BUNDLE_ENGINE in the script if - // the test needs to exercise the default engine. - // - // selftests intentionally exercise the empty-list mechanic and are - // exempt. - if vals, ok := config.EnvMatrix["DATABRICKS_BUNDLE_ENGINE"]; ok && len(vals) == 0 && !strings.Contains(configPath, "selftest/") { - t.Fatalf("Invalid config %s: EnvMatrix.DATABRICKS_BUNDLE_ENGINE = [] "+ - "runs on both direct and terraform CI runners. Use "+ - `EnvMatrix.DATABRICKS_BUNDLE_ENGINE = ["direct"] instead`, configPath) - } } func DoLoadConfig(t *testing.T, path string) TestConfig { @@ -447,60 +431,15 @@ func filterExcludedEnvSets(envSets [][]string, exclude map[string][]string) [][] return filtered } -// SubsetExpanded selects one variant per DATABRICKS_BUNDLE_ENGINE value (if scriptUsesEngine) -// or one variant total from an already-expanded and exclusion-filtered list. -// DATABRICKS_BUNDLE_ENGINE=direct has weight 10; all other variants have weight 1. -func SubsetExpanded(expanded [][]string, testDir string, scriptUsesEngine bool) [][]string { +// SubsetExpanded selects one variant from an already-expanded and exclusion-filtered +// list, using consistent hashing on testDir. +func SubsetExpanded(expanded [][]string, testDir string) [][]string { if len(expanded) <= 1 { return expanded } - if scriptUsesEngine { - // Collect candidates per engine key, preserving first-seen order. - // keyToIdx maps engine value -> index in result/groups slices. - var result [][]string - var groups [][][]string - keyToIdx := make(map[string]int) - for _, envset := range expanded { - engine := "" - for _, kv := range envset { - if v, ok := strings.CutPrefix(kv, "DATABRICKS_BUNDLE_ENGINE="); ok { - engine = v - break - } - } - idx, ok := keyToIdx[engine] - if !ok { - idx = len(result) - keyToIdx[engine] = idx - result = append(result, nil) - groups = append(groups, nil) - } - groups[idx] = append(groups[idx], envset) - } - for i, group := range groups { - result[i] = weightedSelect(group, testDir) - } - return result - } - return [][]string{weightedSelect(expanded, testDir)} -} - -// weightedSelect picks one envset using weighted consistent hashing. -// DATABRICKS_BUNDLE_ENGINE=direct has weight 10; all other envsets have weight 1. -func weightedSelect(envsets [][]string, testDir string) []string { - var weighted [][]string - for _, envset := range envsets { - weight := 1 - if slices.Contains(envset, "DATABRICKS_BUNDLE_ENGINE=direct") { - weight = 10 - } - for range weight { - weighted = append(weighted, envset) - } - } h := fnv.New64a() h.Write([]byte(testDir)) - return weighted[h.Sum64()%uint64(len(weighted))] + return [][]string{expanded[h.Sum64()%uint64(len(expanded))]} } // matchesExclusionRule returns true if envSet contains all KEY=value pairs from excludeRule. diff --git a/acceptance/internal/config_test.go b/acceptance/internal/config_test.go index c6fbd2e1cc5..b1380adaf17 100644 --- a/acceptance/internal/config_test.go +++ b/acceptance/internal/config_test.go @@ -128,18 +128,18 @@ func TestExpandEnvMatrix(t *testing.T) { }, }, { - name: "exclude with terraform and readplan example", + name: "exclude with dms and readplan example", matrix: map[string][]string{ - "DATABRICKS_BUNDLE_ENGINE": {"terraform", "direct"}, - "READPLAN": {"0", "1"}, + "DMS": {"", "true"}, + "READPLAN": {"0", "1"}, }, exclude: map[string][]string{ - "noplantf": {"READPLAN=1", "DATABRICKS_BUNDLE_ENGINE=terraform"}, + "nodmsplan": {"READPLAN=1", "DMS=true"}, }, expected: [][]string{ - {"DATABRICKS_BUNDLE_ENGINE=terraform", "READPLAN=0"}, - {"DATABRICKS_BUNDLE_ENGINE=direct", "READPLAN=0"}, - {"DATABRICKS_BUNDLE_ENGINE=direct", "READPLAN=1"}, + {"DMS=", "READPLAN=0"}, + {"DMS=", "READPLAN=1"}, + {"DMS=true", "READPLAN=0"}, }, }, { @@ -203,43 +203,25 @@ func TestExpandEnvMatrix(t *testing.T) { } } -func TestSubsetExpanded_DirectBias(t *testing.T) { - // Across many test dirs, DATABRICKS_BUNDLE_ENGINE=direct should be selected ~10/11 of the time. +func TestSubsetExpanded(t *testing.T) { expanded := [][]string{ - {"DATABRICKS_BUNDLE_ENGINE=terraform"}, - {"DATABRICKS_BUNDLE_ENGINE=direct"}, + {"DMS=", "READPLAN="}, + {"DMS=true", "READPLAN="}, + {"DMS=true", "READPLAN=1"}, } - directCount := 0 - total := 1000 - for i := range total { - r := SubsetExpanded(expanded, fmt.Sprintf("test/dir%d", i), false) - if r[0][0] == "DATABRICKS_BUNDLE_ENGINE=direct" { - directCount++ - } - } - ratio := float64(directCount) / float64(total) - assert.InDelta(t, float64(10)/11, ratio, 0.05, "expected ~10/11 direct, got %.1f%%", ratio*100) -} + result := SubsetExpanded(expanded, "test/dir") + require.Len(t, result, 1) + assert.Contains(t, expanded, result[0]) + // Consistent for the same dir. + assert.Equal(t, result, SubsetExpanded(expanded, "test/dir")) -func TestSubsetExpanded_ScriptUsesEngine(t *testing.T) { - // When script uses $DATABRICKS_BUNDLE_ENGINE, one combo per engine value is returned. - expanded := [][]string{ - {"DATABRICKS_BUNDLE_ENGINE=terraform", "READPLAN="}, - {"DATABRICKS_BUNDLE_ENGINE=direct", "READPLAN="}, - {"DATABRICKS_BUNDLE_ENGINE=direct", "READPLAN=1"}, - } - result := SubsetExpanded(expanded, "test/dir", true) - require.Len(t, result, 2) - engines := make(map[string]bool) - for _, envset := range result { - for _, kv := range envset { - if strings.HasPrefix(kv, "DATABRICKS_BUNDLE_ENGINE=") { - engines[kv] = true - } - } + // Every variant gets picked for some dir. + seen := make(map[string]bool) + for i := range 100 { + r := SubsetExpanded(expanded, fmt.Sprintf("test/dir%d", i)) + seen[strings.Join(r[0], ",")] = true } - assert.True(t, engines["DATABRICKS_BUNDLE_ENGINE=terraform"]) - assert.True(t, engines["DATABRICKS_BUNDLE_ENGINE=direct"]) + assert.Len(t, seen, len(expanded)) } func TestLoadConfigPhaseIsNotInherited(t *testing.T) { diff --git a/acceptance/internal/prepare_server.go b/acceptance/internal/prepare_server.go index b99c7546eb7..35aeeb429be 100644 --- a/acceptance/internal/prepare_server.go +++ b/acceptance/internal/prepare_server.go @@ -67,13 +67,16 @@ func isTruePtr(value *bool) bool { // staleOnceEnabled reports whether the testserver should simulate eventual // consistency (the first GET after a create returns 404). It is opt-in via -// INJECT_STALE_ON_DIRECT=1. +// INJECT_STALE_ON_DIRECT=1 and only applies to the direct engine. // // testEnv carries the per-variant EnvMatrix values, which are not visible via // os/env because matrix variants run in parallel and only reach the CLI subprocess. func staleOnceEnabled(testEnv []string) bool { - v, _ := lookupEnv(testEnv, "INJECT_STALE_ON_DIRECT") - return v == "1" + if v, _ := lookupEnv(testEnv, "INJECT_STALE_ON_DIRECT"); v != "1" { + return false + } + engine, _ := lookupEnv(testEnv, "DATABRICKS_BUNDLE_ENGINE") + return engine == "direct" } func lookupEnv(testEnv []string, key string) (string, bool) { diff --git a/acceptance/subset_unit_test.go b/acceptance/subset_unit_test.go index 607f01cdf4c..3bec8555e61 100644 --- a/acceptance/subset_unit_test.go +++ b/acceptance/subset_unit_test.go @@ -9,7 +9,7 @@ import ( func TestSubsetDisabled(t *testing.T) { var s subsetSelector assert.Empty(t, s.skipReason("bundle/foo", nil)) - assert.Empty(t, s.skipReason("bundle/foo", []string{"DATABRICKS_BUNDLE_ENGINE=direct"})) + assert.Empty(t, s.skipReason("bundle/foo", []string{"DMS=true"})) } func TestSubsetPctBoundaries(t *testing.T) { @@ -30,7 +30,7 @@ func TestSubsetChangedAlwaysRuns(t *testing.T) { seed: "seed", changed: map[string][]string{"bundle/changed": nil}, } - assert.Empty(t, s.skipReason("bundle/changed", []string{"DATABRICKS_BUNDLE_ENGINE=direct"})) + assert.Empty(t, s.skipReason("bundle/changed", []string{"DMS=true"})) assert.NotEmpty(t, s.skipReason("bundle/other", nil)) } @@ -93,8 +93,8 @@ func TestSubsetVariantsIndependent(t *testing.T) { sawDifferent := false for i := range 50 { dir := "bundle/d" + string(rune('a'+i%26)) - a := s.skipReason(dir, []string{"DATABRICKS_BUNDLE_ENGINE=direct"}) == "" - b := s.skipReason(dir, []string{"DATABRICKS_BUNDLE_ENGINE=terraform"}) == "" + a := s.skipReason(dir, []string{"DMS=true"}) == "" + b := s.skipReason(dir, []string{"DMS="}) == "" if a != b { sawDifferent = true break