Skip to content

Break import cycle between libs/env and internal/testutil #4708

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
pietern merged 1 commit into from
Mar 12, 2026
+40 −34
Merged

Break import cycle between libs/env and internal/testutil #4708

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
4 changes: 3 additions & 1 deletion internal/testutil/debug.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@ import (
"path"
"path/filepath"
"strings"

"github.com/databricks/cli/libs/env"
)

// Detects if test is run from "debug test" feature in VS Code.
Expand All @@ -19,7 +21,7 @@ func LoadDebugEnvIfRunFromIDE(t TestingT, key string) {
if !isInDebug() {
return
}
home, err := os.UserHomeDir() //nolint:forbidigo // import cycle: libs/env tests import internal/testutil
home, err := env.UserHomeDir(t.Context())
if err != nil {
t.Fatalf("cannot find user home: %s", err)
}
Expand Down
6 changes: 4 additions & 2 deletions internal/testutil/env.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,14 +4,16 @@ import (
"os"
"runtime"
"strings"

"github.com/databricks/cli/libs/env"
)

// CleanupEnvironment sets up a pristine environment containing only $PATH and $HOME.
// The original environment is restored upon test completion.
// Note: use of this function is incompatible with parallel execution.
func CleanupEnvironment(t TestingT) {
path := os.Getenv("PATH") //nolint:forbidigo // import cycle: libs/env tests import internal/testutil
pwd := os.Getenv("PWD") //nolint:forbidigo // import cycle: libs/env tests import internal/testutil
path := env.Get(t.Context(), "PATH")
pwd := env.Get(t.Context(), "PWD")

// Clear all environment variables.
NullEnvironment(t)
Expand Down
4 changes: 2 additions & 2 deletions internal/testutil/helpers.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,15 +1,15 @@
package testutil

import (
"os"
"strings"

"github.com/databricks/cli/libs/env"
"github.com/google/uuid"
)

// GetEnvOrSkipTest proceeds with test only with that env variable.
func GetEnvOrSkipTest(t TestingT, name string) string {
value := os.Getenv(name) //nolint:forbidigo // import cycle: libs/env tests import internal/testutil
value := env.Get(t.Context(), name)
if value == "" {
t.Skipf("Environment variable %s is missing", name)
}
Expand Down
51 changes: 26 additions & 25 deletions libs/env/context_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,9 +1,10 @@
package env
package env_test

import (
"testing"

"github.com/databricks/cli/internal/testutil"
"github.com/databricks/cli/libs/env"
"github.com/stretchr/testify/assert"
)

Expand All @@ -14,33 +15,33 @@ func TestContext(t *testing.T) {
ctx0 := t.Context()

// Get
assert.Equal(t, "bar", Get(ctx0, "FOO"))
assert.Equal(t, "", Get(ctx0, "dontexist"))
assert.Equal(t, "bar", env.Get(ctx0, "FOO"))
assert.Equal(t, "", env.Get(ctx0, "dontexist"))

// Lookup
v, ok := Lookup(ctx0, "FOO")
v, ok := env.Lookup(ctx0, "FOO")
assert.True(t, ok)
assert.Equal(t, "bar", v)
v, ok = Lookup(ctx0, "dontexist")
v, ok = env.Lookup(ctx0, "dontexist")
assert.False(t, ok)
assert.Equal(t, "", v)

// Set and get new context.
// Verify that the previous context remains unchanged.
ctx1 := Set(ctx0, "FOO", "baz")
assert.Equal(t, "baz", Get(ctx1, "FOO"))
assert.Equal(t, "bar", Get(ctx0, "FOO"))
ctx1 := env.Set(ctx0, "FOO", "baz")
assert.Equal(t, "baz", env.Get(ctx1, "FOO"))
assert.Equal(t, "bar", env.Get(ctx0, "FOO"))

// Set and get new context.
// Verify that the previous contexts remains unchanged.
ctx2 := Set(ctx1, "FOO", "qux")
assert.Equal(t, "qux", Get(ctx2, "FOO"))
assert.Equal(t, "baz", Get(ctx1, "FOO"))
assert.Equal(t, "bar", Get(ctx0, "FOO"))
ctx2 := env.Set(ctx1, "FOO", "qux")
assert.Equal(t, "qux", env.Get(ctx2, "FOO"))
assert.Equal(t, "baz", env.Get(ctx1, "FOO"))
assert.Equal(t, "bar", env.Get(ctx0, "FOO"))

ctx3 := Set(ctx2, "BAR", "x=y")
ctx3 := env.Set(ctx2, "BAR", "x=y")

all := All(ctx3)
all := env.All(ctx3)
assert.NotNil(t, all)
assert.Equal(t, "qux", all["FOO"])
assert.Equal(t, "x=y", all["BAR"])
Expand All @@ -49,8 +50,8 @@ func TestContext(t *testing.T) {

func TestHome(t *testing.T) {
ctx := t.Context()
ctx = WithUserHomeDir(ctx, "...")
home, err := UserHomeDir(ctx)
ctx = env.WithUserHomeDir(ctx, "...")
home, err := env.UserHomeDir(ctx)
assert.Equal(t, "...", home)
assert.NoError(t, err)
}
Expand All @@ -63,8 +64,8 @@ func TestGetBool(t *testing.T) {
trueValues := []string{"true", "TRUE", "True", "1", "t", "T", "yes", "YES", "Yes", "on", "ON", "On"}
for _, v := range trueValues {
t.Run("true_"+v, func(t *testing.T) {
ctx := Set(ctx, "TEST_BOOL", v)
val, ok := GetBool(ctx, "TEST_BOOL")
ctx := env.Set(ctx, "TEST_BOOL", v)
val, ok := env.GetBool(ctx, "TEST_BOOL")
assert.True(t, ok, "expected key to be set")
assert.True(t, val, "expected %q to be true", v)
})
Expand All @@ -74,8 +75,8 @@ func TestGetBool(t *testing.T) {
falseValues := []string{"false", "FALSE", "False", "0", "f", "F", "no", "NO", "No", "off", "OFF", "Off", ""}
for _, v := range falseValues {
t.Run("false_"+v, func(t *testing.T) {
ctx := Set(ctx, "TEST_BOOL", v)
val, ok := GetBool(ctx, "TEST_BOOL")
ctx := env.Set(ctx, "TEST_BOOL", v)
val, ok := env.GetBool(ctx, "TEST_BOOL")
assert.True(t, ok, "expected key to be set")
assert.False(t, val, "expected %q to be false", v)
})
Expand All @@ -85,26 +86,26 @@ func TestGetBool(t *testing.T) {
invalidValues := []string{"invalid", "random", "2", "maybe"}
for _, v := range invalidValues {
t.Run("invalid_"+v, func(t *testing.T) {
ctx := Set(ctx, "TEST_BOOL", v)
val, ok := GetBool(ctx, "TEST_BOOL")
ctx := env.Set(ctx, "TEST_BOOL", v)
val, ok := env.GetBool(ctx, "TEST_BOOL")
assert.True(t, ok, "expected key to be set")
assert.False(t, val, "expected %q to be false (invalid)", v)
})
}

// Test missing key returns ok=false
val, ok := GetBool(ctx, "NON_EXISTENT_KEY")
val, ok := env.GetBool(ctx, "NON_EXISTENT_KEY")
assert.False(t, ok, "expected key to not be set")
assert.False(t, val, "expected value to be false when not set")

// Test from actual environment variable
t.Setenv("TEST_ENV_BOOL", "true")
val, ok = GetBool(t.Context(), "TEST_ENV_BOOL")
val, ok = env.GetBool(t.Context(), "TEST_ENV_BOOL")
assert.True(t, ok)
assert.True(t, val)

t.Setenv("TEST_ENV_BOOL_FALSE", "0")
val, ok = GetBool(t.Context(), "TEST_ENV_BOOL_FALSE")
val, ok = env.GetBool(t.Context(), "TEST_ENV_BOOL_FALSE")
assert.True(t, ok)
assert.False(t, val)
}
9 changes: 5 additions & 4 deletions libs/env/loader_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,17 +1,18 @@
package env
package env_test

import (
"testing"

"github.com/databricks/cli/libs/env"
"github.com/databricks/databricks-sdk-go/config"
"github.com/stretchr/testify/assert"
)

func TestLoader(t *testing.T) {
ctx := t.Context()
ctx = Set(ctx, "DATABRICKS_WAREHOUSE_ID", "...")
ctx = Set(ctx, "DATABRICKS_CONFIG_PROFILE", "...")
loader := NewConfigLoader(ctx)
ctx = env.Set(ctx, "DATABRICKS_WAREHOUSE_ID", "...")
ctx = env.Set(ctx, "DATABRICKS_CONFIG_PROFILE", "...")
loader := env.NewConfigLoader(ctx)

cfg := &config.Config{
Profile: "abc",
Expand Down
Loading