Skip to content

Commit 79ca434

Browse files
kolyshkingopherbot
authored andcommitted
testing: add Chdir
Some tests need to use os.Chdir, but the use is complicated because - they must change back to the old working directory; - they must not use t.Parallel. Add Chdir that covers these cases, and sets PWD environment variable to the new directory for the duration of the test for Unix platforms. Unify the panic message when t.Parallel is used together with t.Setenv or t.Chdir. Add some tests. For #62516. Change-Id: Ib050d173b26eb28a27dba5a206b2d0d877d761c1 Reviewed-on: https://go-review.googlesource.com/c/go/+/529895 LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: Carlos Amedee <carlos@golang.org> Reviewed-by: Ian Lance Taylor <iant@google.com> Auto-Submit: Ian Lance Taylor <iant@google.com>
1 parent ff271cd commit 79ca434

File tree

5 files changed

+221
-53
lines changed

5 files changed

+221
-53
lines changed

api/next/62516.txt

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
pkg testing, method (*B) Chdir(string) #62516
2+
pkg testing, method (*F) Chdir(string) #62516
3+
pkg testing, method (*T) Chdir(string) #62516
4+
pkg testing, type TB interface, Chdir(string) #62516
Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
The new [T.Chdir] and [B.Chdir] methods can be used to change the working
2+
directory for the duration of a test or benchmark.

src/testing/export_test.go

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,3 +9,5 @@ var PrettyPrint = prettyPrint
99
type HighPrecisionTime = highPrecisionTime
1010

1111
var HighPrecisionTimeNow = highPrecisionTimeNow
12+
13+
const ParallelConflict = parallelConflict

src/testing/testing.go

Lines changed: 75 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -379,6 +379,7 @@ import (
379379
"io"
380380
"math/rand"
381381
"os"
382+
"path/filepath"
382383
"reflect"
383384
"runtime"
384385
"runtime/debug"
@@ -891,6 +892,7 @@ type TB interface {
891892
Logf(format string, args ...any)
892893
Name() string
893894
Setenv(key, value string)
895+
Chdir(dir string)
894896
Skip(args ...any)
895897
SkipNow()
896898
Skipf(format string, args ...any)
@@ -917,8 +919,8 @@ var _ TB = (*B)(nil)
917919
// may be called simultaneously from multiple goroutines.
918920
type T struct {
919921
common
920-
isEnvSet bool
921-
context *testContext // For running tests and subtests.
922+
denyParallel bool
923+
context *testContext // For running tests and subtests.
922924
}
923925

924926
func (c *common) private() {}
@@ -1307,6 +1309,48 @@ func (c *common) Setenv(key, value string) {
13071309
}
13081310
}
13091311

1312+
// Chdir calls os.Chdir(dir) and uses Cleanup to restore the current
1313+
// working directory to its original value after the test. On Unix, it
1314+
// also sets PWD environment variable for the duration of the test.
1315+
//
1316+
// Because Chdir affects the whole process, it cannot be used
1317+
// in parallel tests or tests with parallel ancestors.
1318+
func (c *common) Chdir(dir string) {
1319+
c.checkFuzzFn("Chdir")
1320+
oldwd, err := os.Open(".")
1321+
if err != nil {
1322+
c.Fatal(err)
1323+
}
1324+
if err := os.Chdir(dir); err != nil {
1325+
c.Fatal(err)
1326+
}
1327+
// On POSIX platforms, PWD represents “an absolute pathname of the
1328+
// current working directory.” Since we are changing the working
1329+
// directory, we should also set or update PWD to reflect that.
1330+
switch runtime.GOOS {
1331+
case "windows", "plan9":
1332+
// Windows and Plan 9 do not use the PWD variable.
1333+
default:
1334+
if !filepath.IsAbs(dir) {
1335+
dir, err = os.Getwd()
1336+
if err != nil {
1337+
c.Fatal(err)
1338+
}
1339+
}
1340+
c.Setenv("PWD", dir)
1341+
}
1342+
c.Cleanup(func() {
1343+
err := oldwd.Chdir()
1344+
oldwd.Close()
1345+
if err != nil {
1346+
// It's not safe to continue with tests if we can't
1347+
// get back to the original working directory. Since
1348+
// we are holding a dirfd, this is highly unlikely.
1349+
panic("testing.Chdir: " + err.Error())
1350+
}
1351+
})
1352+
}
1353+
13101354
// panicHandling controls the panic handling used by runCleanup.
13111355
type panicHandling int
13121356

@@ -1436,6 +1480,8 @@ func pcToName(pc uintptr) string {
14361480
return frame.Function
14371481
}
14381482

1483+
const parallelConflict = `testing: test using t.Setenv or t.Chdir can not use t.Parallel`
1484+
14391485
// Parallel signals that this test is to be run in parallel with (and only with)
14401486
// other parallel tests. When a test is run multiple times due to use of
14411487
// -test.count or -test.cpu, multiple instances of a single test never run in
@@ -1444,8 +1490,8 @@ func (t *T) Parallel() {
14441490
if t.isParallel {
14451491
panic("testing: t.Parallel called multiple times")
14461492
}
1447-
if t.isEnvSet {
1448-
panic("testing: t.Parallel called after t.Setenv; cannot set environment variables in parallel tests")
1493+
if t.denyParallel {
1494+
panic(parallelConflict)
14491495
}
14501496
t.isParallel = true
14511497
if t.parent.barrier == nil {
@@ -1500,34 +1546,43 @@ func (t *T) Parallel() {
15001546
t.lastRaceErrors.Store(int64(race.Errors()))
15011547
}
15021548

1503-
// Setenv calls os.Setenv(key, value) and uses Cleanup to
1504-
// restore the environment variable to its original value
1505-
// after the test.
1506-
//
1507-
// Because Setenv affects the whole process, it cannot be used
1508-
// in parallel tests or tests with parallel ancestors.
1509-
func (t *T) Setenv(key, value string) {
1549+
func (t *T) checkParallel() {
15101550
// Non-parallel subtests that have parallel ancestors may still
15111551
// run in parallel with other tests: they are only non-parallel
15121552
// with respect to the other subtests of the same parent.
1513-
// Since SetEnv affects the whole process, we need to disallow it
1514-
// if the current test or any parent is parallel.
1515-
isParallel := false
1553+
// Since calls like SetEnv or Chdir affects the whole process, we need
1554+
// to deny those if the current test or any parent is parallel.
15161555
for c := &t.common; c != nil; c = c.parent {
15171556
if c.isParallel {
1518-
isParallel = true
1519-
break
1557+
panic(parallelConflict)
15201558
}
15211559
}
1522-
if isParallel {
1523-
panic("testing: t.Setenv called after t.Parallel; cannot set environment variables in parallel tests")
1524-
}
15251560

1526-
t.isEnvSet = true
1561+
t.denyParallel = true
1562+
}
15271563

1564+
// Setenv calls os.Setenv(key, value) and uses Cleanup to
1565+
// restore the environment variable to its original value
1566+
// after the test.
1567+
//
1568+
// Because Setenv affects the whole process, it cannot be used
1569+
// in parallel tests or tests with parallel ancestors.
1570+
func (t *T) Setenv(key, value string) {
1571+
t.checkParallel()
15281572
t.common.Setenv(key, value)
15291573
}
15301574

1575+
// Chdir calls os.Chdir(dir) and uses Cleanup to restore the current
1576+
// working directory to its original value after the test. On Unix, it
1577+
// also sets PWD environment variable for the duration of the test.
1578+
//
1579+
// Because Chdir affects the whole process, it cannot be used
1580+
// in parallel tests or tests with parallel ancestors.
1581+
func (t *T) Chdir(dir string) {
1582+
t.checkParallel()
1583+
t.common.Chdir(dir)
1584+
}
1585+
15311586
// InternalTest is an internal type but exported because it is cross-package;
15321587
// it is part of the implementation of the "go test" command.
15331588
type InternalTest struct {

src/testing/testing_test.go

Lines changed: 138 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ import (
1313
"os/exec"
1414
"path/filepath"
1515
"regexp"
16+
"runtime"
1617
"slices"
1718
"strings"
1819
"sync"
@@ -200,64 +201,168 @@ func TestSetenv(t *testing.T) {
200201
}
201202
}
202203

203-
func TestSetenvWithParallelAfterSetenv(t *testing.T) {
204-
defer func() {
205-
want := "testing: t.Parallel called after t.Setenv; cannot set environment variables in parallel tests"
206-
if got := recover(); got != want {
207-
t.Fatalf("expected panic; got %#v want %q", got, want)
208-
}
209-
}()
204+
func expectParallelConflict(t *testing.T) {
205+
want := testing.ParallelConflict
206+
if got := recover(); got != want {
207+
t.Fatalf("expected panic; got %#v want %q", got, want)
208+
}
209+
}
210210

211-
t.Setenv("GO_TEST_KEY_1", "value")
211+
func testWithParallelAfter(t *testing.T, fn func(*testing.T)) {
212+
defer expectParallelConflict(t)
212213

214+
fn(t)
213215
t.Parallel()
214216
}
215217

216-
func TestSetenvWithParallelBeforeSetenv(t *testing.T) {
217-
defer func() {
218-
want := "testing: t.Setenv called after t.Parallel; cannot set environment variables in parallel tests"
219-
if got := recover(); got != want {
220-
t.Fatalf("expected panic; got %#v want %q", got, want)
221-
}
222-
}()
218+
func testWithParallelBefore(t *testing.T, fn func(*testing.T)) {
219+
defer expectParallelConflict(t)
223220

224221
t.Parallel()
225-
226-
t.Setenv("GO_TEST_KEY_1", "value")
222+
fn(t)
227223
}
228224

229-
func TestSetenvWithParallelParentBeforeSetenv(t *testing.T) {
225+
func testWithParallelParentBefore(t *testing.T, fn func(*testing.T)) {
230226
t.Parallel()
231227

232228
t.Run("child", func(t *testing.T) {
233-
defer func() {
234-
want := "testing: t.Setenv called after t.Parallel; cannot set environment variables in parallel tests"
235-
if got := recover(); got != want {
236-
t.Fatalf("expected panic; got %#v want %q", got, want)
237-
}
238-
}()
229+
defer expectParallelConflict(t)
239230

240-
t.Setenv("GO_TEST_KEY_1", "value")
231+
fn(t)
241232
})
242233
}
243234

244-
func TestSetenvWithParallelGrandParentBeforeSetenv(t *testing.T) {
235+
func testWithParallelGrandParentBefore(t *testing.T, fn func(*testing.T)) {
245236
t.Parallel()
246237

247238
t.Run("child", func(t *testing.T) {
248239
t.Run("grand-child", func(t *testing.T) {
249-
defer func() {
250-
want := "testing: t.Setenv called after t.Parallel; cannot set environment variables in parallel tests"
251-
if got := recover(); got != want {
252-
t.Fatalf("expected panic; got %#v want %q", got, want)
253-
}
254-
}()
240+
defer expectParallelConflict(t)
255241

256-
t.Setenv("GO_TEST_KEY_1", "value")
242+
fn(t)
257243
})
258244
})
259245
}
260246

247+
func tSetenv(t *testing.T) {
248+
t.Setenv("GO_TEST_KEY_1", "value")
249+
}
250+
251+
func TestSetenvWithParallelAfter(t *testing.T) {
252+
testWithParallelAfter(t, tSetenv)
253+
}
254+
255+
func TestSetenvWithParallelBefore(t *testing.T) {
256+
testWithParallelBefore(t, tSetenv)
257+
}
258+
259+
func TestSetenvWithParallelParentBefore(t *testing.T) {
260+
testWithParallelParentBefore(t, tSetenv)
261+
}
262+
263+
func TestSetenvWithParallelGrandParentBefore(t *testing.T) {
264+
testWithParallelGrandParentBefore(t, tSetenv)
265+
}
266+
267+
func tChdir(t *testing.T) {
268+
t.Chdir(t.TempDir())
269+
}
270+
271+
func TestChdirWithParallelAfter(t *testing.T) {
272+
testWithParallelAfter(t, tChdir)
273+
}
274+
275+
func TestChdirWithParallelBefore(t *testing.T) {
276+
testWithParallelBefore(t, tChdir)
277+
}
278+
279+
func TestChdirWithParallelParentBefore(t *testing.T) {
280+
testWithParallelParentBefore(t, tChdir)
281+
}
282+
283+
func TestChdirWithParallelGrandParentBefore(t *testing.T) {
284+
testWithParallelGrandParentBefore(t, tChdir)
285+
}
286+
287+
func TestChdir(t *testing.T) {
288+
oldDir, err := os.Getwd()
289+
if err != nil {
290+
t.Fatal(err)
291+
}
292+
defer os.Chdir(oldDir)
293+
294+
tmp := t.TempDir()
295+
rel, err := filepath.Rel(oldDir, tmp)
296+
if err != nil {
297+
t.Fatal(err)
298+
}
299+
300+
for _, tc := range []struct {
301+
name, dir, pwd string
302+
extraChdir bool
303+
}{
304+
{
305+
name: "absolute",
306+
dir: tmp,
307+
pwd: tmp,
308+
},
309+
{
310+
name: "relative",
311+
dir: rel,
312+
pwd: tmp,
313+
},
314+
{
315+
name: "current (absolute)",
316+
dir: oldDir,
317+
pwd: oldDir,
318+
},
319+
{
320+
name: "current (relative) with extra os.Chdir",
321+
dir: ".",
322+
pwd: oldDir,
323+
324+
extraChdir: true,
325+
},
326+
} {
327+
t.Run(tc.name, func(t *testing.T) {
328+
if !filepath.IsAbs(tc.pwd) {
329+
t.Fatalf("Bad tc.pwd: %q (must be absolute)", tc.pwd)
330+
}
331+
332+
t.Chdir(tc.dir)
333+
334+
newDir, err := os.Getwd()
335+
if err != nil {
336+
t.Fatal(err)
337+
}
338+
if newDir != tc.pwd {
339+
t.Fatalf("failed to chdir to %q: getwd: got %q, want %q", tc.dir, newDir, tc.pwd)
340+
}
341+
342+
switch runtime.GOOS {
343+
case "windows", "plan9":
344+
// Windows and Plan 9 do not use the PWD variable.
345+
default:
346+
if pwd := os.Getenv("PWD"); pwd != tc.pwd {
347+
t.Fatalf("PWD: got %q, want %q", pwd, tc.pwd)
348+
}
349+
}
350+
351+
if tc.extraChdir {
352+
os.Chdir("..")
353+
}
354+
})
355+
356+
newDir, err := os.Getwd()
357+
if err != nil {
358+
t.Fatal(err)
359+
}
360+
if newDir != oldDir {
361+
t.Fatalf("failed to restore wd to %s: getwd: %s", oldDir, newDir)
362+
}
363+
}
364+
}
365+
261366
// testingTrueInInit is part of TestTesting.
262367
var testingTrueInInit = false
263368

0 commit comments

Comments
 (0)