Skip to content

Commit a52a3f7

Browse files
pieternclaude
pietern
and
claude
authored
Break import cycle between libs/env and internal/testutil (#4708)
## Summary - Move `libs/env` tests from `package env` to `package env_test` to break the import cycle - This allows `internal/testutil` to import `libs/env` directly - Replace `os.Getenv`/`os.UserHomeDir` calls in `internal/testutil` with `env.Get(ctx)`/`env.UserHomeDir(ctx)`, removing all `nolint:forbidigo` exceptions Follow-up to #4700 and #4654 which added the forbidigo linter rules but had to add `nolint` exceptions in `internal/testutil` due to the import cycle. ## Test plan - [x] `go test ./libs/env/...` passes - [x] `go test ./internal/testutil/...` passes - [x] `make lintfull` passes (no more nolint exceptions for the cycle) 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
1 parent 3d4eede commit a52a3f7

File tree

5 files changed

+40
-34
lines changed

5 files changed

+40
-34
lines changed

‎internal/testutil/debug.go‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,8 @@ import (
66
"path"
77
"path/filepath"
88
"strings"
9+
10+
"github.com/databricks/cli/libs/env"
911
)
1012

1113
// Detects if test is run from "debug test" feature in VS Code.
@@ -19,7 +21,7 @@ func LoadDebugEnvIfRunFromIDE(t TestingT, key string) {
1921
if !isInDebug() {
2022
return
2123
}
22-
home, err := os.UserHomeDir() //nolint:forbidigo // import cycle: libs/env tests import internal/testutil
24+
home, err := env.UserHomeDir(t.Context())
2325
if err != nil {
2426
t.Fatalf("cannot find user home: %s", err)
2527
}

‎internal/testutil/env.go‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,14 +4,16 @@ import (
44
"os"
55
"runtime"
66
"strings"
7+
8+
"github.com/databricks/cli/libs/env"
79
)
810

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

1618
// Clear all environment variables.
1719
NullEnvironment(t)

‎internal/testutil/helpers.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,15 @@
11
package testutil
22

33
import (
4-
"os"
54
"strings"
65

6+
"github.com/databricks/cli/libs/env"
77
"github.com/google/uuid"
88
)
99

1010
// GetEnvOrSkipTest proceeds with test only with that env variable.
1111
func GetEnvOrSkipTest(t TestingT, name string) string {
12-
value := os.Getenv(name) //nolint:forbidigo // import cycle: libs/env tests import internal/testutil
12+
value := env.Get(t.Context(), name)
1313
if value == "" {
1414
t.Skipf("Environment variable %s is missing", name)
1515
}

‎libs/env/context_test.go‎

Lines changed: 26 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,10 @@
1-
package env
1+
package env_test
22

33
import (
44
"testing"
55

66
"github.com/databricks/cli/internal/testutil"
7+
"github.com/databricks/cli/libs/env"
78
"github.com/stretchr/testify/assert"
89
)
910

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

1617
// Get
17-
assert.Equal(t, "bar", Get(ctx0, "FOO"))
18-
assert.Equal(t, "", Get(ctx0, "dontexist"))
18+
assert.Equal(t, "bar", env.Get(ctx0, "FOO"))
19+
assert.Equal(t, "", env.Get(ctx0, "dontexist"))
1920

2021
// Lookup
21-
v, ok := Lookup(ctx0, "FOO")
22+
v, ok := env.Lookup(ctx0, "FOO")
2223
assert.True(t, ok)
2324
assert.Equal(t, "bar", v)
24-
v, ok = Lookup(ctx0, "dontexist")
25+
v, ok = env.Lookup(ctx0, "dontexist")
2526
assert.False(t, ok)
2627
assert.Equal(t, "", v)
2728

2829
// Set and get new context.
2930
// Verify that the previous context remains unchanged.
30-
ctx1 := Set(ctx0, "FOO", "baz")
31-
assert.Equal(t, "baz", Get(ctx1, "FOO"))
32-
assert.Equal(t, "bar", Get(ctx0, "FOO"))
31+
ctx1 := env.Set(ctx0, "FOO", "baz")
32+
assert.Equal(t, "baz", env.Get(ctx1, "FOO"))
33+
assert.Equal(t, "bar", env.Get(ctx0, "FOO"))
3334

3435
// Set and get new context.
3536
// Verify that the previous contexts remains unchanged.
36-
ctx2 := Set(ctx1, "FOO", "qux")
37-
assert.Equal(t, "qux", Get(ctx2, "FOO"))
38-
assert.Equal(t, "baz", Get(ctx1, "FOO"))
39-
assert.Equal(t, "bar", Get(ctx0, "FOO"))
37+
ctx2 := env.Set(ctx1, "FOO", "qux")
38+
assert.Equal(t, "qux", env.Get(ctx2, "FOO"))
39+
assert.Equal(t, "baz", env.Get(ctx1, "FOO"))
40+
assert.Equal(t, "bar", env.Get(ctx0, "FOO"))
4041

41-
ctx3 := Set(ctx2, "BAR", "x=y")
42+
ctx3 := env.Set(ctx2, "BAR", "x=y")
4243

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

5051
func TestHome(t *testing.T) {
5152
ctx := t.Context()
52-
ctx = WithUserHomeDir(ctx, "...")
53-
home, err := UserHomeDir(ctx)
53+
ctx = env.WithUserHomeDir(ctx, "...")
54+
home, err := env.UserHomeDir(ctx)
5455
assert.Equal(t, "...", home)
5556
assert.NoError(t, err)
5657
}
@@ -63,8 +64,8 @@ func TestGetBool(t *testing.T) {
6364
trueValues := []string{"true", "TRUE", "True", "1", "t", "T", "yes", "YES", "Yes", "on", "ON", "On"}
6465
for _, v := range trueValues {
6566
t.Run("true_"+v, func(t *testing.T) {
66-
ctx := Set(ctx, "TEST_BOOL", v)
67-
val, ok := GetBool(ctx, "TEST_BOOL")
67+
ctx := env.Set(ctx, "TEST_BOOL", v)
68+
val, ok := env.GetBool(ctx, "TEST_BOOL")
6869
assert.True(t, ok, "expected key to be set")
6970
assert.True(t, val, "expected %q to be true", v)
7071
})
@@ -74,8 +75,8 @@ func TestGetBool(t *testing.T) {
7475
falseValues := []string{"false", "FALSE", "False", "0", "f", "F", "no", "NO", "No", "off", "OFF", "Off", ""}
7576
for _, v := range falseValues {
7677
t.Run("false_"+v, func(t *testing.T) {
77-
ctx := Set(ctx, "TEST_BOOL", v)
78-
val, ok := GetBool(ctx, "TEST_BOOL")
78+
ctx := env.Set(ctx, "TEST_BOOL", v)
79+
val, ok := env.GetBool(ctx, "TEST_BOOL")
7980
assert.True(t, ok, "expected key to be set")
8081
assert.False(t, val, "expected %q to be false", v)
8182
})
@@ -85,26 +86,26 @@ func TestGetBool(t *testing.T) {
8586
invalidValues := []string{"invalid", "random", "2", "maybe"}
8687
for _, v := range invalidValues {
8788
t.Run("invalid_"+v, func(t *testing.T) {
88-
ctx := Set(ctx, "TEST_BOOL", v)
89-
val, ok := GetBool(ctx, "TEST_BOOL")
89+
ctx := env.Set(ctx, "TEST_BOOL", v)
90+
val, ok := env.GetBool(ctx, "TEST_BOOL")
9091
assert.True(t, ok, "expected key to be set")
9192
assert.False(t, val, "expected %q to be false (invalid)", v)
9293
})
9394
}
9495

9596
// Test missing key returns ok=false
96-
val, ok := GetBool(ctx, "NON_EXISTENT_KEY")
97+
val, ok := env.GetBool(ctx, "NON_EXISTENT_KEY")
9798
assert.False(t, ok, "expected key to not be set")
9899
assert.False(t, val, "expected value to be false when not set")
99100

100101
// Test from actual environment variable
101102
t.Setenv("TEST_ENV_BOOL", "true")
102-
val, ok = GetBool(t.Context(), "TEST_ENV_BOOL")
103+
val, ok = env.GetBool(t.Context(), "TEST_ENV_BOOL")
103104
assert.True(t, ok)
104105
assert.True(t, val)
105106

106107
t.Setenv("TEST_ENV_BOOL_FALSE", "0")
107-
val, ok = GetBool(t.Context(), "TEST_ENV_BOOL_FALSE")
108+
val, ok = env.GetBool(t.Context(), "TEST_ENV_BOOL_FALSE")
108109
assert.True(t, ok)
109110
assert.False(t, val)
110111
}

‎libs/env/loader_test.go‎

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,17 +1,18 @@
1-
package env
1+
package env_test
22

33
import (
44
"testing"
55

6+
"github.com/databricks/cli/libs/env"
67
"github.com/databricks/databricks-sdk-go/config"
78
"github.com/stretchr/testify/assert"
89
)
910

1011
func TestLoader(t *testing.T) {
1112
ctx := t.Context()
12-
ctx = Set(ctx, "DATABRICKS_WAREHOUSE_ID", "...")
13-
ctx = Set(ctx, "DATABRICKS_CONFIG_PROFILE", "...")
14-
loader := NewConfigLoader(ctx)
13+
ctx = env.Set(ctx, "DATABRICKS_WAREHOUSE_ID", "...")
14+
ctx = env.Set(ctx, "DATABRICKS_CONFIG_PROFILE", "...")
15+
loader := env.NewConfigLoader(ctx)
1516

1617
cfg := &config.Config{
1718
Profile: "abc",

0 commit comments

Comments
 (0)