acceptance: use templates for bundle_default_profile scenarios#5270
Open
janniklasrose wants to merge 6 commits into
Open
acceptance: use templates for bundle_default_profile scenarios#5270janniklasrose wants to merge 6 commits into
janniklasrose wants to merge 6 commits into
Conversation
sethome exported HOME (and USERPROFILE) as the relative path the caller passed in (typically "./home"). The SDK expands `~/.databrickscfg` against $HOME at lookup time, so the moment a test cd's into a child directory, the cfg "disappears": helpers like databrickscfg.GetConfiguredDefaultProfile silently return "" because the relative path no longer points at a real file. That made it easy to write tests that look like they exercise default_profile resolution but actually only succeed because the cfg lookup short-circuits. Resolve $1 to an absolute path with `cd && pwd` (portable on macOS/Linux/Windows-MSYS) before exporting. With this fix, the acceptance/auth/bundle_default_profile "workspace.profile pinned" sub-case correctly fails when the Workspace.Profile == "" guard in cmd/root/bundle.go configureProfile is removed; previously it passed either way. Output for the same directory's "Bundle with workspace.host" sub-case also changes: with the cfg now reachable from the bundle subdir, the SDK's host-based lookup finds the matching profile and validate succeeds instead of failing with "default auth: cannot configure default credentials". Co-authored-by: Isaac
81d9297 changed sethome to export an absolute HOME so the SDK can locate ~/.databrickscfg after a test cd's. The SDK now reports paths relative to the absolute HOME, which surfaces in user-visible output as [TEST_TMP_DIR]/home/.databrickscfg (or [HOME]/.zshrc where the test registers a HOME replacement). The four committed outputs predated the fix and still expected the relative ./home form; regenerated with ./task test-update. Co-authored-by: Isaac
The previous fix resolved $home with `cd && pwd` to give HOME and USERPROFILE an absolute path. On Git Bash (MSYS), plain `pwd` returns Unix-style /c/... paths that the native Windows Go binary can't open, so the SDK silently fails to load ~/.databrickscfg and tests like cmd/api/default-profile (no auth resolves) and cmd/api/workspace-id-none (profile not read; SDK back-fills WorkspaceID from host metadata) fail on Windows only. Use `pwd -W` for USERPROFILE on MSYS/Cygwin to emit the mixed C:/... form Windows file APIs accept. HOME keeps the Unix-style path bash itself wants. Co-authored-by: Isaac
The absolute-path resolution (cd && pwd) introduced in 81d9297 works on macOS/Linux but emits Unix-style /c/... paths on Git Bash, which the native Windows Go binary can't open; the follow-up adb452f fixed the CLI path with `pwd -W` but broke tests like cmd/completion that compare the bash $HOME placeholder against the CLI's USERPROFILE-derived output. Revert sethome to the original simple form and restore the four outputs regenerated in f893006 to their pre-fix relative-./home forms. bundle_default_profile keeps its sub-cases but the cd'ing variants short-circuit on a missing cfg again (same expected output as before 81d9297). A separate PR will revisit the test setup to make absolute HOME work cleanly across platforms. Co-authored-by: Isaac
sethome exported HOME (and USERPROFILE) as the relative path the caller passed in (typically "./home"). The SDK expands `~/.databrickscfg` against $HOME at lookup time, so the moment a test cd's into a child directory the cfg "disappears": helpers like databrickscfg.GetConfiguredDefaultProfile silently return "" because the relative path no longer points at a real file. That made it easy to write tests that look like they exercise default_profile resolution but actually only succeed because the cfg lookup short-circuits. Resolve $1 to an absolute path with `cd && pwd` before exporting. On Git Bash (MSYS) plain `pwd` returns Unix-style /c/... paths that the native Windows Go binary can't open, so use `pwd -W` to get the mixed C:/... form. Use the same form for both HOME and USERPROFILE so tests like cmd/completion (which registers `$HOME` as the `[HOME]` replacement) match the CLI's USERPROFILE-derived output on Windows. Regenerate the four affected outputs. Co-authored-by: Isaac
Replace inline heredocs and per-scenario subdirectories
(./bundle-with-host, ./bundle-with-profile) with three committed
template files: databricks.yml.{no-host,with-host,with-profile}.tmpl.
The script copies (or envsubsts, for the host-pinned template) the
active template into databricks.yml before each phase, so the active
bundle is always at the test root.
Follows pietern's suggestion in #5214.
Co-authored-by: Isaac
janniklasrose
temporarily deployed
to
test-trigger-is
GitHub Actions
Inactive
janniklasrose
temporarily deployed
to
test-trigger-is
GitHub Actions
Inactive
Contributor
Waiting for approvalCould not determine reviewers from git history. Eligible reviewers: Suggestions based on git history. See OWNERS for ownership rules. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Follow-up to #5214 (per pietern's review comment).
Replace the inline heredocs in
acceptance/auth/bundle_default_profile/scriptwith three committed templates:databricks.yml.no-host.tmpl— bundle with neitherworkspace.hostnorworkspace.profile. Used for the "default_profile is honored" and "--profileoverrides" phases.databricks.yml.with-host.tmpl— pinsworkspace.host: $DATABRICKS_HOST.envsubstfills in the test server URL before each invocation.databricks.yml.with-profile.tmpl— pinsworkspace.profile: other.Before each phase the script overwrites the active
databricks.ymlfrom the relevant template. The per-scenario subdirectories (./bundle-with-host,./bundle-with-profile) and thecdcalls are gone — the active bundle is always at the test root.Stacked on #5266 (sethome-absolute fix). The base will auto-retarget to
mainonce #5266 merges.output.txtis unchanged across the refactor: both engine variants (terraformanddirect) produce byte-identical output to the pre-refactor run.Test plan
go test ./acceptance -run TestAccept/auth/bundle_default_profile -vpasses for bothDATABRICKS_BUNDLE_ENGINE=terraformand=direct../task fmt-qand./task lint-qclean.This pull request and its description were written by Isaac.