From 16217fe9b965804e86f6c274d3613d8ff17236f9 Mon Sep 17 00:00:00 2001 From: Daniel Nephin Date: Fri, 14 Aug 2020 20:06:01 -0400 Subject: [PATCH] testing: use t.Cleanup in testutil.TempFile So that it has the same behaviour as TempDir. Also remove the now unnecessary 'defer os.Remove' --- agent/util_test.go | 1 - command/config/write/config_write_test.go | 2 -- command/intention/create/create_test.go | 2 -- command/kv/put/kv_put_test.go | 2 -- .../services/deregister/deregister_test.go | 2 -- command/validate/validate_test.go | 2 -- sdk/testutil/io.go | 33 +++++++++++-------- 7 files changed, 20 insertions(+), 24 deletions(-) diff --git a/agent/util_test.go b/agent/util_test.go index 65df2cadc0..b091d853e7 100644 --- a/agent/util_test.go +++ b/agent/util_test.go @@ -31,7 +31,6 @@ func TestSetFilePermissions(t *testing.T) { } tempFile := testutil.TempFile(t, "consul") path := tempFile.Name() - defer os.Remove(path) // Bad UID fails if err := setFilePermissions(path, "%", "", ""); err == nil { diff --git a/command/config/write/config_write_test.go b/command/config/write/config_write_test.go index e111f323c6..a5b9120b29 100644 --- a/command/config/write/config_write_test.go +++ b/command/config/write/config_write_test.go @@ -2,7 +2,6 @@ package write import ( "io" - "os" "strings" "testing" "time" @@ -32,7 +31,6 @@ func TestConfigWrite(t *testing.T) { c := New(ui) f := testutil.TempFile(t, "config-write-svc-web.hcl") - defer os.Remove(f.Name()) _, err := f.WriteString(` Kind = "service-defaults" Name = "web" diff --git a/command/intention/create/create_test.go b/command/intention/create/create_test.go index b419f20310..1b9c690e24 100644 --- a/command/intention/create/create_test.go +++ b/command/intention/create/create_test.go @@ -1,7 +1,6 @@ package create import ( - "os" "strings" "testing" @@ -146,7 +145,6 @@ func TestCommand_File(t *testing.T) { contents := `{ "SourceName": "foo", "DestinationName": "bar", "Action": "allow" }` f := testutil.TempFile(t, "intention-create-command-file") - defer os.Remove(f.Name()) if _, err := f.WriteString(contents); err != nil { t.Fatalf("err: %#v", err) } diff --git a/command/kv/put/kv_put_test.go b/command/kv/put/kv_put_test.go index eec776b4e5..a5289681c2 100644 --- a/command/kv/put/kv_put_test.go +++ b/command/kv/put/kv_put_test.go @@ -4,7 +4,6 @@ import ( "bytes" "encoding/base64" "io" - "os" "strconv" "strings" "testing" @@ -176,7 +175,6 @@ func TestKVPutCommand_File(t *testing.T) { c := New(ui) f := testutil.TempFile(t, "kv-put-command-file") - defer os.Remove(f.Name()) if _, err := f.WriteString("bar"); err != nil { t.Fatalf("err: %#v", err) } diff --git a/command/services/deregister/deregister_test.go b/command/services/deregister/deregister_test.go index a460fe292a..b2a9c7a11a 100644 --- a/command/services/deregister/deregister_test.go +++ b/command/services/deregister/deregister_test.go @@ -172,13 +172,11 @@ func testFile(t *testing.T, suffix string) *os.File { newName := f.Name() + "." + suffix if err := os.Rename(f.Name(), newName); err != nil { - os.Remove(f.Name()) t.Fatalf("err: %s", err) } f, err := os.Create(newName) if err != nil { - os.Remove(newName) t.Fatalf("err: %s", err) } diff --git a/command/validate/validate_test.go b/command/validate/validate_test.go index a00a173589..c8cc3bf4d1 100644 --- a/command/validate/validate_test.go +++ b/command/validate/validate_test.go @@ -2,7 +2,6 @@ package validate import ( "io/ioutil" - "os" "path/filepath" "strings" "testing" @@ -22,7 +21,6 @@ func TestValidateCommand_noTabs(t *testing.T) { func TestValidateCommand_FailOnEmptyFile(t *testing.T) { t.Parallel() tmpFile := testutil.TempFile(t, "consul") - defer os.RemoveAll(tmpFile.Name()) cmd := New(cli.NewMockUi()) args := []string{tmpFile.Name()} diff --git a/sdk/testutil/io.go b/sdk/testutil/io.go index 28f215b677..b3396023c5 100644 --- a/sdk/testutil/io.go +++ b/sdk/testutil/io.go @@ -31,9 +31,10 @@ func init() { var noCleanup = strings.ToLower(os.Getenv("TEST_NOCLEANUP")) == "true" -// TempDir creates a temporary directory within tmpdir -// with the name 'testname-name'. If the directory cannot -// be created t.Fatal is called. +// TempDir creates a temporary directory within tmpdir with the name 'testname-name'. +// If the directory cannot be created t.Fatal is called. +// The directory will be removed when the test ends. Set TEST_NOCLEANUP env var +// to prevent the directory from being removed. func TempDir(t *testing.T, name string) string { if t == nil { panic("argument t must be non-nil") @@ -54,22 +55,28 @@ func TempDir(t *testing.T, name string) string { return d } -// TempFile creates a temporary file within tmpdir -// with the name 'testname-name'. If the file cannot -// be created t.Fatal is called. If a temporary directory -// has been created before consider storing the file -// inside this directory to avoid double cleanup. +// TempFile creates a temporary file within tmpdir with the name 'testname-name'. +// If the file cannot be created t.Fatal is called. If a temporary directory +// has been created before consider storing the file inside this directory to +// avoid double cleanup. +// The file will be removed when the test ends. Set TEST_NOCLEANUP env var +// to prevent the file from being removed. func TempFile(t *testing.T, name string) *os.File { - if t != nil && t.Name() != "" { - name = t.Name() + "-" + name + if t == nil { + panic("argument t must be non-nil") } + name = t.Name() + "-" + name name = strings.Replace(name, "/", "_", -1) f, err := ioutil.TempFile(tmpdir, name) if err != nil { - if t == nil { - panic(err) - } t.Fatalf("err: %s", err) } + t.Cleanup(func() { + if noCleanup { + t.Logf("skipping cleanup because TEST_NOCLEANUP was enabled") + return + } + os.Remove(f.Name()) + }) return f }