Skip to content

Commit

Permalink
unix: modernize test helpers
Browse files Browse the repository at this point in the history
Test helper functions mktmpfifo and chtmpdir were written
before t.Cleanup was available, which necessitated returning
of a cleanup function and the use of defer.

Let's use t.Cleanup (available since Go 1.14) and simplify the callers.

While at it,
 - use t.Helper (added in Go 1.9);
 - simplify some error messages (errors that come from os package
   usually doesn't need to be wrapped).

Change-Id: Id981ae1c85fa2642a76358a31fc18a9af2f78711
Reviewed-on: https://go-review.googlesource.com/c/sys/+/526695
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
Reviewed-by: Ian Lance Taylor <iant@google.com>
Auto-Submit: Ian Lance Taylor <iant@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
  • Loading branch information
kolyshkin authored and gopherbot committed Sep 14, 2023
1 parent a26c6de commit 8d9dcc4
Show file tree
Hide file tree
Showing 6 changed files with 33 additions and 32 deletions.
2 changes: 1 addition & 1 deletion unix/syscall_aix_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ func TestTime(t *testing.T) {
}

func TestUtime(t *testing.T) {
defer chtmpdir(t)()
chtmpdir(t)

touch(t, "file1")

Expand Down
11 changes: 5 additions & 6 deletions unix/syscall_linux_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -277,9 +277,8 @@ func TestPpoll(t *testing.T) {
t.Skip("mkfifo syscall is not available on android, skipping test")
}

defer chtmpdir(t)()
f, cleanup := mktmpfifo(t)
defer cleanup()
chtmpdir(t)
f := mktmpfifo(t)

const timeout = 100 * time.Millisecond

Expand Down Expand Up @@ -335,7 +334,7 @@ func TestTime(t *testing.T) {
}

func TestUtime(t *testing.T) {
defer chtmpdir(t)()
chtmpdir(t)

touch(t, "file1")

Expand Down Expand Up @@ -548,7 +547,7 @@ func TestStatx(t *testing.T) {
t.Fatalf("Statx: %v", err)
}

defer chtmpdir(t)()
chtmpdir(t)
touch(t, "file1")

var st unix.Stat_t
Expand Down Expand Up @@ -636,7 +635,7 @@ func stringsFromByteSlice(buf []byte) []string {
}

func TestFaccessat(t *testing.T) {
defer chtmpdir(t)()
chtmpdir(t)
touch(t, "file1")

err := unix.Faccessat(unix.AT_FDCWD, "file1", unix.R_OK, 0)
Expand Down
2 changes: 1 addition & 1 deletion unix/syscall_netbsd_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ func TestIoctlPtmget(t *testing.T) {
}

func TestStatvfs(t *testing.T) {
defer chtmpdir(t)()
chtmpdir(t)
touch(t, "file1")

var statvfs1, statvfs2 unix.Statvfs_t
Expand Down
5 changes: 2 additions & 3 deletions unix/syscall_openbsd_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,8 @@ import (
)

func TestPpoll(t *testing.T) {
defer chtmpdir(t)()
f, cleanup := mktmpfifo(t)
defer cleanup()
chtmpdir(t)
f := mktmpfifo(t)

const timeout = 100 * time.Millisecond

Expand Down
43 changes: 23 additions & 20 deletions unix/syscall_unix_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -502,9 +502,8 @@ func TestPoll(t *testing.T) {
t.Skip("mkfifo syscall is not available on android and iOS, skipping test")
}

defer chtmpdir(t)()
f, cleanup := mktmpfifo(t)
defer cleanup()
chtmpdir(t)
f := mktmpfifo(t)

const timeout = 100

Expand Down Expand Up @@ -710,7 +709,7 @@ func compareStat_t(t *testing.T, otherStat string, st1, st2 *unix.Stat_t) {
}

func TestFstatat(t *testing.T) {
defer chtmpdir(t)()
chtmpdir(t)

touch(t, "file1")

Expand Down Expand Up @@ -747,7 +746,7 @@ func TestFstatat(t *testing.T) {
}

func TestFchmodat(t *testing.T) {
defer chtmpdir(t)()
chtmpdir(t)

touch(t, "file1")
err := os.Symlink("file1", "symlink1")
Expand Down Expand Up @@ -857,7 +856,7 @@ func TestPipe(t *testing.T) {
}

func TestRenameat(t *testing.T) {
defer chtmpdir(t)()
chtmpdir(t)

from, to := "renamefrom", "renameto"

Expand All @@ -880,7 +879,7 @@ func TestRenameat(t *testing.T) {
}

func TestUtimesNanoAt(t *testing.T) {
defer chtmpdir(t)()
chtmpdir(t)

symlink := "symlink1"
os.Remove(symlink)
Expand Down Expand Up @@ -1111,8 +1110,9 @@ func TestRecvmsgControl(t *testing.T) {
defer unix.Close(cfds[0])
}

// mktmpfifo creates a temporary FIFO and provides a cleanup function.
func mktmpfifo(t *testing.T) (*os.File, func()) {
// mktmpfifo creates a temporary FIFO and sets up a cleanup function.
func mktmpfifo(t *testing.T) *os.File {
t.Helper()
err := unix.Mkfifo("fifo", 0666)
if err != nil {
t.Fatalf("mktmpfifo: failed to create FIFO: %v", err)
Expand All @@ -1121,18 +1121,20 @@ func mktmpfifo(t *testing.T) (*os.File, func()) {
f, err := os.OpenFile("fifo", os.O_RDWR, 0666)
if err != nil {
os.Remove("fifo")
t.Fatalf("mktmpfifo: failed to open FIFO: %v", err)
t.Fatal(err)
}

return f, func() {
t.Cleanup(func() {
f.Close()
os.Remove("fifo")
}
})

return f
}

// utilities taken from os/os_test.go

func touch(t *testing.T, name string) {
t.Helper()
f, err := os.Create(name)
if err != nil {
t.Fatal(err)
Expand All @@ -1143,18 +1145,19 @@ func touch(t *testing.T, name string) {
}

// chtmpdir changes the working directory to a new temporary directory and
// provides a cleanup function. Used when PWD is read-only.
func chtmpdir(t *testing.T) func() {
// sets up a cleanup function. Used when PWD is read-only.
func chtmpdir(t *testing.T) {
t.Helper()
oldwd, err := os.Getwd()
if err != nil {
t.Fatalf("chtmpdir: %v", err)
t.Fatal(err)
}
if err := os.Chdir(t.TempDir()); err != nil {
t.Fatalf("chtmpdir: %v", err)
t.Fatal(err)
}
return func() {
t.Cleanup(func() {
if err := os.Chdir(oldwd); err != nil {
t.Fatalf("chtmpdir: %v", err)
t.Fatal(err)
}
}
})
}
2 changes: 1 addition & 1 deletion unix/xattr_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ import (
)

func TestXattr(t *testing.T) {
defer chtmpdir(t)()
chtmpdir(t)

f := "xattr1"
touch(t, f)
Expand Down

0 comments on commit 8d9dcc4

Please sign in to comment.