From 8d9dcc467bd1c72f22d7c89d16612e2259b8aa87 Mon Sep 17 00:00:00 2001 From: Kir Kolyshkin Date: Thu, 7 Sep 2023 11:04:37 -0700 Subject: [PATCH] unix: modernize test helpers 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 Reviewed-by: Ian Lance Taylor Auto-Submit: Ian Lance Taylor LUCI-TryBot-Result: Go LUCI --- unix/syscall_aix_test.go | 2 +- unix/syscall_linux_test.go | 11 +++++---- unix/syscall_netbsd_test.go | 2 +- unix/syscall_openbsd_test.go | 5 ++--- unix/syscall_unix_test.go | 43 +++++++++++++++++++----------------- unix/xattr_test.go | 2 +- 6 files changed, 33 insertions(+), 32 deletions(-) diff --git a/unix/syscall_aix_test.go b/unix/syscall_aix_test.go index 4b885e8c8..ffff394c1 100644 --- a/unix/syscall_aix_test.go +++ b/unix/syscall_aix_test.go @@ -60,7 +60,7 @@ func TestTime(t *testing.T) { } func TestUtime(t *testing.T) { - defer chtmpdir(t)() + chtmpdir(t) touch(t, "file1") diff --git a/unix/syscall_linux_test.go b/unix/syscall_linux_test.go index ae2d6f43a..200ccd860 100644 --- a/unix/syscall_linux_test.go +++ b/unix/syscall_linux_test.go @@ -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 @@ -335,7 +334,7 @@ func TestTime(t *testing.T) { } func TestUtime(t *testing.T) { - defer chtmpdir(t)() + chtmpdir(t) touch(t, "file1") @@ -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 @@ -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) diff --git a/unix/syscall_netbsd_test.go b/unix/syscall_netbsd_test.go index aac432081..d26fe314c 100644 --- a/unix/syscall_netbsd_test.go +++ b/unix/syscall_netbsd_test.go @@ -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 diff --git a/unix/syscall_openbsd_test.go b/unix/syscall_openbsd_test.go index dabe4d598..add529d7c 100644 --- a/unix/syscall_openbsd_test.go +++ b/unix/syscall_openbsd_test.go @@ -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 diff --git a/unix/syscall_unix_test.go b/unix/syscall_unix_test.go index 10cddb436..d147923cd 100644 --- a/unix/syscall_unix_test.go +++ b/unix/syscall_unix_test.go @@ -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 @@ -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") @@ -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") @@ -857,7 +856,7 @@ func TestPipe(t *testing.T) { } func TestRenameat(t *testing.T) { - defer chtmpdir(t)() + chtmpdir(t) from, to := "renamefrom", "renameto" @@ -880,7 +879,7 @@ func TestRenameat(t *testing.T) { } func TestUtimesNanoAt(t *testing.T) { - defer chtmpdir(t)() + chtmpdir(t) symlink := "symlink1" os.Remove(symlink) @@ -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) @@ -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) @@ -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) } - } + }) } diff --git a/unix/xattr_test.go b/unix/xattr_test.go index 55dd630c8..94411bd04 100644 --- a/unix/xattr_test.go +++ b/unix/xattr_test.go @@ -18,7 +18,7 @@ import ( ) func TestXattr(t *testing.T) { - defer chtmpdir(t)() + chtmpdir(t) f := "xattr1" touch(t, f)