Skip to content
Merged
Changes from 1 commit
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
141 changes: 141 additions & 0 deletions integration/cmd/environments/setup_local_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,141 @@
package environments_test

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not this file, but related: OWNERS routes this file to team:platform rather than team:ide.

/integration/team:platform (.github/OWNERS:65) and findOwners is last-match-wins (.github/scripts/owners.js:97-106), so this file — squarely localenv territory — won't route to the team that owns /libs/localenv/, /cmd/environments/, and /acceptance/localenv/ (added in #6187).

Worth adding, and it must go below line 65 to win:

/integration/cmd/environments/  team:ide

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done in 9d9c0ac — added /integration/cmd/environments/ team:ide below the /integration/ rule so it wins under last-match-wins. Verified with findOwners: this file now resolves to rugpanov rclarey anton-107 misha-db, while integration/cmd/jobs/ still resolves to team:platform.


import (
"encoding/json"
"os"
"path/filepath"
"runtime"
"testing"

"github.com/databricks/cli/integration/internal/acc"
"github.com/databricks/cli/internal/testcli"
"github.com/databricks/cli/libs/localenv"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)

// The serverless target needs no running compute: --serverless-version is used
// verbatim, so these tests resolve against the real Jobs/Clusters-free path and
// fetch from the public databricks/environments repo. v5 is a published LTS
// target (see the repo's python/serverless/ tree).
const testServerlessVersion = "5"

// TestSetupLocalServerlessProvision drives the full non-dry-run pipeline against
// the real published constraints: resolve -> fetch -> uv sync -> validate. It
// asserts a real .venv is created and the --output json contract carries the
// resolved versions. This is the one integration test that exercises a real uv
// provision end to end; the acceptance suite covers everything else via --dry-run.
func TestSetupLocalServerlessProvision(t *testing.T) {
ctx, _ := acc.WorkspaceTest(t)

// setup-local operates on the current working directory; run in a fresh
// greenfield project (no pre-existing pyproject.toml).
dir := t.TempDir()
t.Chdir(dir)

// Let uv bootstrap itself if the runner's PATH lacks it; CI installs uv, but
// this keeps the test robust on a developer machine that opted in.
t.Setenv(localenv.EnvAutoInstallUv, "1")

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This installs uv on a developer's machine as a side effect, and the comment has it backwards: the test sets the opt-in itself, so nobody opted in.

On a machine without uv, EnvAutoInstallUv=1 makes EnsureAvailable run curl -LsSf https://astral.sh/uv/install.sh | sh, mutating ~/.local/bin (libs/localenv/uv.go:363). The sibling test avoids exactly this — requireRealProvision gates on an explicit env var and skips when uv isn't discoverable (provision_integration_test.go:27-34).

Since CI already installs uv (.github/actions/setup-build-environment/action.yml:73), the robustness this buys applies only to the case where the side effect is unwanted. Suggest dropping the Setenv and skipping instead:

if _, err := exec.LookPath("uv"); err != nil {
    t.Skipf("uv not found on PATH (%v)", err)
}

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done in 9d9c0ac — replaced with an exec.LookPath("uv") skip. You're right that the comment had it backwards: the test set the opt-in itself, so the "opted in" claim was wrong, and on a machine without uv this ran the remote installer and mutated ~/.local/bin.


stdout, _ := testcli.RequireSuccessfulRun(t, ctx,
"environments", "setup-local",
"--serverless-version", testServerlessVersion,
"--output", "json",
)

var res localenv.Result
require.NoError(t, json.Unmarshal(stdout.Bytes(), &res))

assert.True(t, res.OK, "expected ok=true, got result: %s", stdout.String())
assert.False(t, res.DryRun)
assert.Equal(t, "environments setup-local", res.Command)
assert.Equal(t, "default", res.Mode)

require.NotNil(t, res.Compute)
assert.Equal(t, "serverless", res.Compute.Source)
assert.Equal(t, "serverless/serverless-v"+testServerlessVersion, res.Compute.EnvKey)

require.NotNil(t, res.Resolved)
assert.NotEmpty(t, res.Resolved.PythonVersion, "resolved python version should be reported")
// The default mode installs databricks-connect, so the resolved pin is present.
assert.NotEmpty(t, res.Resolved.DBConnectVersion)
// The artifact came from a successful fetch. The cache is shared (UserCacheDir),
// so a prior run may have seeded it; accept either source rather than assuming
// a cold cache and flaking on re-runs.
assert.Contains(t, []string{"network", "cache"}, res.Resolved.ArtifactSource)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This assertion is tautological and can never fail. ArtifactSource is set from artifactSource(c.FromCache) (libs/localenv/pipeline.go:191), whose entire range is those two values (pipeline.go:27-30), and it's assigned in exactly one place.

The reasoning in the comment is sound (avoid flaking on a warm shared cache) — the problem is the resulting assertion reads like coverage but checks nothing.

Either isolate the cache so "network" is deterministic, or drop the line. Note there's currently no cache-dir override knob — cmd/environments/sync.go:136 uses os.UserCacheDir() unconditionally — so isolating means adding one or leaning on HOME/XDG_CACHE_HOME, neither portable. Deleting is probably the honest option.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done in 9d9c0ac — deleted. artifactSource() only ever returns artifactCache or artifactNetwork (pipeline.go:535-540) from a single assignment site, so the membership assertion could never fail. Agreed that deleting beats adding a cache-dir override just to make it meaningful.


// Every phase must have reached ok, including the real provision and validate.
for _, ph := range res.Phases {
assert.Equalf(t, localenv.StatusOK, ph.Status, "phase %s not ok", ph.Phase)
}

// A real interpreter and lockfile were written into the project. testcli runs
// in-process, so runtime.GOOS is the runner's OS and picks the right layout.
assert.FileExists(t, venvPython(dir))
assert.FileExists(t, filepath.Join(dir, "pyproject.toml"))
assert.FileExists(t, filepath.Join(dir, "uv.lock"))
}

// TestSetupLocalDryRunWritesNothing verifies the --dry-run contract against the
// real repo: the plan resolves and fetches, reports ok, but writes no files.
func TestSetupLocalDryRunWritesNothing(t *testing.T) {
ctx, _ := acc.WorkspaceTest(t)

dir := t.TempDir()
t.Chdir(dir)

stdout, _ := testcli.RequireSuccessfulRun(t, ctx,
"environments", "setup-local",
"--serverless-version", testServerlessVersion,
"--dry-run",
"--output", "json",
)

var res localenv.Result
require.NoError(t, json.Unmarshal(stdout.Bytes(), &res))
assert.True(t, res.OK)
assert.True(t, res.DryRun)

// --dry-run must not touch disk.
entries, err := os.ReadDir(dir)
require.NoError(t, err)
assert.Empty(t, entries, "dry-run wrote files: %v", entries)
}

// TestSetupLocalUnpublishedVersion verifies the fetch-phase error contract: an
// unpublished serverless version resolves fine but has no artifact, so the run
// fails with E_ENV_UNSUPPORTED and a non-zero exit (surfaced as a run error).
func TestSetupLocalUnpublishedVersion(t *testing.T) {
ctx, _ := acc.WorkspaceTest(t)

dir := t.TempDir()
t.Chdir(dir)

// A version far above anything published; resolution succeeds, fetch 404s.
stdout, _, runErr := testcli.RequireErrorRun(t, ctx,
"environments", "setup-local",
"--serverless-version", "9999",
"--dry-run",
"--output", "json",
)
require.Error(t, runErr)

var res localenv.Result
require.NoError(t, json.Unmarshal(stdout.Bytes(), &res))
assert.False(t, res.OK)
require.NotNil(t, res.Error)
assert.Equal(t, localenv.ErrEnvUnsupported, res.Error.Code)
assert.Equal(t, localenv.PhaseFetch, res.Error.FailurePhase)

// Even a failed fetch must not have provisioned anything on a dry run.
assert.NoFileExists(t, filepath.Join(dir, ".venv", "bin", "python"))

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This hardcodes the Unix layout, defeating the venvPython helper defined 5 lines below (and used in test 1). On Windows this checks a path that could never exist, so the assertion is vacuous there.

Since this is a --dry-run — preflight's ensureWritable is skipped and cache writes are suppressed (pipeline.go:141-147) — the directory should be completely empty. The stronger and simpler assertion is the same os.ReadDir + assert.Empty that TestSetupLocalDryRunWritesNothing already uses.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done in 9d9c0ac — replaced with the os.ReadDir + assert.Empty check, matching TestSetupLocalDryRunWritesNothing. It was bypassing venvPython and vacuous on Windows; since --dry-run skips ensureWritable and suppresses cache writes, asserting the dir is empty is strictly stronger.

}

// venvPython returns the path to the created virtualenv's interpreter, accounting
// for the Windows (Scripts/python.exe) vs Unix (bin/python) layout.
func venvPython(dir string) string {
if runtime.GOOS == "windows" {
return filepath.Join(dir, ".venv", "Scripts", "python.exe")
}
return filepath.Join(dir, ".venv", "bin", "python")
}
Loading