Skip to content

Commit 344db5f

Browse files
pieternclaude
andauthored
Use t.Chdir() instead of manual os.Chdir patterns in tests (#4660)
## Summary - Replace manual working directory manipulation with `t.Chdir()`, added in Go 1.24 - Remove `testutil.Chdir` and `testutil.TestData` which are now unused - Add a gorule (`libs/gorules/rule_os_chdir.go`) that rejects `os.Chdir` in `_test.go` files going forward ## Test plan - [x] All affected package tests pass - [x] `make lintfull` passes with 0 issues - [x] Gorule correctly flags `os.Chdir` in test files 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
1 parent cb290d4 commit 344db5f

File tree

12 files changed

+54
-154
lines changed

12 files changed

+54
-154
lines changed

acceptance/internal/cmd_server.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,7 @@ func StartCmdServer(t *testing.T) *testserver.Server {
4444
}
4545

4646
// chdir variant that is intended to be used with defer so that it can switch back before function ends.
47-
// This is unlike testutil.Chdir which switches back only when tests end.
47+
// This is unlike t.Chdir which switches back only when tests end.
4848
func chdir(t *testing.T, cwd string) func() {
4949
require.NotEmpty(t, cwd)
5050
prevDir, err := os.Getwd()

bundle/bundle_test.go

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,6 @@ import (
99
"github.com/databricks/cli/bundle/config"
1010
"github.com/databricks/cli/bundle/config/resources"
1111
"github.com/databricks/cli/bundle/env"
12-
"github.com/databricks/cli/internal/testutil"
1312
"github.com/databricks/cli/libs/diag"
1413
"github.com/databricks/cli/libs/dyn"
1514
"github.com/databricks/cli/libs/logdiag"
@@ -116,7 +115,7 @@ func TestBundleMustLoadFailureWithEnv(t *testing.T) {
116115
}
117116

118117
func TestBundleMustLoadFailureIfNotFound(t *testing.T) {
119-
testutil.Chdir(t, t.TempDir())
118+
t.Chdir(t.TempDir())
120119
b, diags := mustLoad(t)
121120
require.Nil(t, b)
122121
require.Len(t, diags, 1, "expected diagnostics")
@@ -142,7 +141,7 @@ func TestBundleTryLoadFailureWithEnv(t *testing.T) {
142141
}
143142

144143
func TestBundleTryLoadOkIfNotFound(t *testing.T) {
145-
testutil.Chdir(t, t.TempDir())
144+
t.Chdir(t.TempDir())
146145
b, diags := tryLoad(t)
147146
assert.Nil(t, b)
148147
assert.Empty(t, diags, "expected no diagnostics")

bundle/config/mutator/python/python_mutator_test.go

Lines changed: 2 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -563,18 +563,11 @@ func loadYaml(name, content string) *bundle.Bundle {
563563
}
564564

565565
func withFakeVEnv(t *testing.T, venvPath string) {
566-
cwd, err := os.Getwd()
567-
if err != nil {
568-
panic(err)
569-
}
570-
571-
if err := os.Chdir(t.TempDir()); err != nil {
572-
panic(err)
573-
}
566+
t.Chdir(t.TempDir())
574567

575568
interpreterPath := interpreterPath(venvPath)
576569

577-
err = os.MkdirAll(filepath.Dir(interpreterPath), 0o755)
570+
err := os.MkdirAll(filepath.Dir(interpreterPath), 0o755)
578571
if err != nil {
579572
panic(err)
580573
}
@@ -588,12 +581,6 @@ func withFakeVEnv(t *testing.T, venvPath string) {
588581
if err != nil {
589582
panic(err)
590583
}
591-
592-
t.Cleanup(func() {
593-
if err := os.Chdir(cwd); err != nil {
594-
panic(err)
595-
}
596-
})
597584
}
598585

599586
func interpreterPath(venvPath string) string {

bundle/root_test.go

Lines changed: 15 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,6 @@ import (
77

88
"github.com/databricks/cli/bundle/config"
99
"github.com/databricks/cli/bundle/env"
10-
"github.com/databricks/cli/internal/testutil"
1110
"github.com/stretchr/testify/require"
1211
)
1312

@@ -62,7 +61,15 @@ func TestRootLookup(t *testing.T) {
6261
t.Setenv(env.RootVariable, "")
6362
os.Unsetenv(env.RootVariable)
6463

65-
testutil.Chdir(t, t.TempDir())
64+
t.Chdir(t.TempDir())
65+
66+
// Resolve to canonical path for comparison below. This is needed because
67+
// os.Getwd may return a path with symlinks (macOS) or 8.3 short names
68+
// (Windows) after a relative chdir.
69+
root, err := os.Getwd()
70+
require.NoError(t, err)
71+
root, err = filepath.EvalSymlinks(root)
72+
require.NoError(t, err)
6673

6774
// Create databricks.yml file.
6875
f, err := os.Create(config.FileNames[0])
@@ -74,10 +81,12 @@ func TestRootLookup(t *testing.T) {
7481
require.NoError(t, err)
7582

7683
// It should find the project root from $PWD.
77-
wd := testutil.Chdir(t, "./a/b/c")
78-
root, err := mustGetRoot(ctx)
84+
t.Chdir("./a/b/c")
85+
foundRoot, err := mustGetRoot(ctx)
86+
require.NoError(t, err)
87+
foundRoot, err = filepath.EvalSymlinks(foundRoot)
7988
require.NoError(t, err)
80-
require.Equal(t, wd, root)
89+
require.Equal(t, root, foundRoot)
8190
}
8291

8392
func TestRootLookupError(t *testing.T) {
@@ -88,7 +97,7 @@ func TestRootLookupError(t *testing.T) {
8897
os.Unsetenv(env.RootVariable)
8998

9099
// It can't find a project root from a temporary directory.
91-
_ = testutil.Chdir(t, t.TempDir())
100+
t.Chdir(t.TempDir())
92101
_, err := mustGetRoot(ctx)
93102
require.ErrorContains(t, err, "unable to locate bundle root")
94103
}

cmd/apps/dev_test.go

Lines changed: 2 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -63,15 +63,10 @@ func TestStartViteDevServerNoNode(t *testing.T) {
6363

6464
// Create a temporary directory to act as project root
6565
tmpDir := t.TempDir()
66-
oldWd, err := os.Getwd()
67-
require.NoError(t, err)
68-
defer func() { _ = os.Chdir(oldWd) }()
69-
70-
err = os.Chdir(tmpDir)
71-
require.NoError(t, err)
66+
t.Chdir(tmpDir)
7267

7368
// Create a client directory
74-
err = os.Mkdir("client", 0o755)
69+
err := os.Mkdir("client", 0o755)
7570
require.NoError(t, err)
7671

7772
// Try to start Vite server with invalid app URL (will fail fast)

cmd/apps/import_test.go

Lines changed: 9 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -103,14 +103,7 @@ env: []`,
103103
for _, tt := range tests {
104104
t.Run(tt.name, func(t *testing.T) {
105105
// Create temp directory and change to it
106-
tmpDir := t.TempDir()
107-
originalDir, err := os.Getwd()
108-
require.NoError(t, err)
109-
defer func() {
110-
_ = os.Chdir(originalDir)
111-
}()
112-
err = os.Chdir(tmpDir)
113-
require.NoError(t, err)
106+
t.Chdir(t.TempDir())
114107

115108
// Setup files
116109
for filename, content := range tt.setupFiles {
@@ -159,17 +152,10 @@ env: []`,
159152

160153
func TestInlineAppConfigFileErrors(t *testing.T) {
161154
t.Run("invalid yaml", func(t *testing.T) {
162-
tmpDir := t.TempDir()
163-
originalDir, err := os.Getwd()
164-
require.NoError(t, err)
165-
defer func() {
166-
_ = os.Chdir(originalDir)
167-
}()
168-
err = os.Chdir(tmpDir)
169-
require.NoError(t, err)
155+
t.Chdir(t.TempDir())
170156

171157
// Create invalid YAML
172-
err = os.WriteFile("app.yml", []byte("invalid: yaml: content:\n - broken"), 0o644)
158+
err := os.WriteFile("app.yml", []byte("invalid: yaml: content:\n - broken"), 0o644)
173159
require.NoError(t, err)
174160

175161
appValue := dyn.V(map[string]dyn.Value{"name": dyn.V("test")})
@@ -179,16 +165,9 @@ func TestInlineAppConfigFileErrors(t *testing.T) {
179165
})
180166

181167
t.Run("app value not a map", func(t *testing.T) {
182-
tmpDir := t.TempDir()
183-
originalDir, err := os.Getwd()
184-
require.NoError(t, err)
185-
defer func() {
186-
_ = os.Chdir(originalDir)
187-
}()
188-
err = os.Chdir(tmpDir)
189-
require.NoError(t, err)
168+
t.Chdir(t.TempDir())
190169

191-
err = os.WriteFile("app.yml", []byte("command: [\"test\"]"), 0o644)
170+
err := os.WriteFile("app.yml", []byte("command: [\"test\"]"), 0o644)
192171
require.NoError(t, err)
193172

194173
appValue := dyn.V("not a map")
@@ -198,18 +177,11 @@ func TestInlineAppConfigFileErrors(t *testing.T) {
198177
})
199178

200179
t.Run("unreadable app.yml", func(t *testing.T) {
201-
tmpDir := t.TempDir()
202-
originalDir, err := os.Getwd()
203-
require.NoError(t, err)
204-
defer func() {
205-
_ = os.Chdir(originalDir)
206-
}()
207-
err = os.Chdir(tmpDir)
208-
require.NoError(t, err)
180+
t.Chdir(t.TempDir())
209181

210182
// Create file with no read permissions
211183
filename := "app.yml"
212-
err = os.WriteFile(filename, []byte("command: [\"test\"]"), 0o644)
184+
err := os.WriteFile(filename, []byte("command: [\"test\"]"), 0o644)
213185
require.NoError(t, err)
214186

215187
if runtime.GOOS == "windows" {
@@ -239,16 +211,9 @@ func TestInlineAppConfigFileErrors(t *testing.T) {
239211

240212
func TestInlineAppConfigFileFieldPriority(t *testing.T) {
241213
t.Run("all fields present", func(t *testing.T) {
242-
tmpDir := t.TempDir()
243-
originalDir, err := os.Getwd()
244-
require.NoError(t, err)
245-
defer func() {
246-
_ = os.Chdir(originalDir)
247-
}()
248-
err = os.Chdir(tmpDir)
249-
require.NoError(t, err)
214+
t.Chdir(t.TempDir())
250215

251-
err = os.WriteFile("app.yml", []byte(`command: ["python", "app.py"]
216+
err := os.WriteFile("app.yml", []byte(`command: ["python", "app.py"]
252217
env:
253218
- name: FOO
254219
value: bar

cmd/labs/project/installer_test.go

Lines changed: 2 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -355,15 +355,8 @@ func TestInstallerWorksForDevelopment(t *testing.T) {
355355
}))
356356
defer server.Close()
357357

358-
wd, _ := os.Getwd()
359-
defer func() {
360-
err := os.Chdir(wd)
361-
require.NoError(t, err)
362-
}()
363-
364358
devDir := copyTestdata(t, "testdata/installed-in-home/.databricks/labs/blueprint/lib")
365-
err := os.Chdir(devDir)
366-
require.NoError(t, err)
359+
t.Chdir(devDir)
367360

368361
ctx := installerContext(t, server)
369362
py, _ := python.DetectExecutable(ctx)
@@ -379,7 +372,7 @@ func TestInstallerWorksForDevelopment(t *testing.T) {
379372
ctx = env.Set(ctx, "DATABRICKS_WAREHOUSE_ID", "efg-id")
380373

381374
home, _ := env.UserHomeDir(ctx)
382-
err = os.WriteFile(filepath.Join(home, ".databrickscfg"), []byte(fmt.Sprintf(`
375+
err := os.WriteFile(filepath.Join(home, ".databrickscfg"), []byte(fmt.Sprintf(`
383376
[profile-one]
384377
host = %s
385378
token = ...

cmd/root/bundle_test.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,7 @@ func setupWithHost(t *testing.T, cmd *cobra.Command, host string) []diag.Diagnos
4747
setupDatabricksCfg(t)
4848

4949
rootPath := t.TempDir()
50-
testutil.Chdir(t, rootPath)
50+
t.Chdir(rootPath)
5151

5252
contents := fmt.Sprintf(`
5353
workspace:
@@ -67,7 +67,7 @@ func setupWithProfile(t *testing.T, cmd *cobra.Command, profile string) []diag.D
6767
setupDatabricksCfg(t)
6868

6969
rootPath := t.TempDir()
70-
testutil.Chdir(t, rootPath)
70+
t.Chdir(rootPath)
7171

7272
contents := fmt.Sprintf(`
7373
workspace:
@@ -228,7 +228,7 @@ func TestBundleConfigureMultiMatchInteractivePromptFires(t *testing.T) {
228228
setupDatabricksCfg(t)
229229

230230
rootPath := t.TempDir()
231-
testutil.Chdir(t, rootPath)
231+
t.Chdir(rootPath)
232232

233233
contents := `
234234
workspace:

internal/testutil/env.go

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

33
import (
44
"os"
5-
"path/filepath"
65
"runtime"
76
"strings"
8-
9-
"github.com/stretchr/testify/require"
107
)
118

129
// CleanupEnvironment sets up a pristine environment containing only $PATH and $HOME.
@@ -46,37 +43,3 @@ func NullEnvironment(t TestingT) {
4643

4744
os.Clearenv()
4845
}
49-
50-
// Changes into specified directory for the duration of the test.
51-
// Returns the current working directory.
52-
func Chdir(t TestingT, dir string) string {
53-
// Prevent parallel execution when changing the working directory.
54-
// t.Setenv automatically fails if t.Parallel is set.
55-
t.Setenv("DO_NOT_RUN_IN_PARALLEL", "true")
56-
57-
wd, err := os.Getwd()
58-
require.NoError(t, err)
59-
if os.Getenv("TESTS_ORIG_WD") == "" {
60-
t.Setenv("TESTS_ORIG_WD", wd)
61-
}
62-
63-
abs, err := filepath.Abs(dir)
64-
require.NoError(t, err)
65-
66-
err = os.Chdir(abs)
67-
require.NoError(t, err)
68-
69-
t.Cleanup(func() {
70-
err := os.Chdir(wd)
71-
require.NoError(t, err)
72-
})
73-
74-
return wd
75-
}
76-
77-
// Return filename ff testutil.Chdir was not called.
78-
// Return absolute path to filename testutil.Chdir() was called.
79-
func TestData(filename string) string {
80-
// Note, if TESTS_ORIG_WD is not set, Getenv return "" and Join returns filename
81-
return filepath.Join(os.Getenv("TESTS_ORIG_WD"), filename)
82-
}

libs/apps/vite/bridge_test.go

Lines changed: 4 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -19,17 +19,11 @@ import (
1919
func TestValidateFilePath(t *testing.T) {
2020
// Create a temporary directory structure for testing
2121
tmpDir := t.TempDir()
22-
oldWd, err := os.Getwd()
23-
require.NoError(t, err)
24-
defer func() { _ = os.Chdir(oldWd) }()
25-
26-
// Change to temp directory
27-
err = os.Chdir(tmpDir)
28-
require.NoError(t, err)
22+
t.Chdir(tmpDir)
2923

3024
// Create the allowed directory
3125
queriesDir := filepath.Join(tmpDir, "config", "queries")
32-
err = os.MkdirAll(queriesDir, 0o755)
26+
err := os.MkdirAll(queriesDir, 0o755)
3327
require.NoError(t, err)
3428

3529
// Create a valid test file
@@ -201,15 +195,10 @@ func TestBridgeHandleMessage(t *testing.T) {
201195
func TestBridgeHandleFileReadRequest(t *testing.T) {
202196
// Create a temporary directory structure
203197
tmpDir := t.TempDir()
204-
oldWd, err := os.Getwd()
205-
require.NoError(t, err)
206-
defer func() { _ = os.Chdir(oldWd) }()
207-
208-
err = os.Chdir(tmpDir)
209-
require.NoError(t, err)
198+
t.Chdir(tmpDir)
210199

211200
queriesDir := filepath.Join(tmpDir, "config", "queries")
212-
err = os.MkdirAll(queriesDir, 0o755)
201+
err := os.MkdirAll(queriesDir, 0o755)
213202
require.NoError(t, err)
214203

215204
testContent := "SELECT * FROM users WHERE id = 1"

0 commit comments

Comments
 (0)