From 729720f549ba7e03a39144cda01fcfed9265d954 Mon Sep 17 00:00:00 2001 From: Radek Simko Date: Mon, 5 Oct 2020 10:49:18 +0100 Subject: [PATCH] Cleanup tests (#92) --- tfexec/apply_test.go | 2 - tfexec/destroy_test.go | 2 - tfexec/fmt_test.go | 2 - tfexec/import_test.go | 2 - tfexec/init_test.go | 2 - tfexec/internal/testutil/temp_dir.go | 24 +++++++ tfexec/internal/testutil/tfcache.go | 44 ------------- tfexec/internal/testutil/tfcache_find.go | 56 ++++++++++++++++ tfexec/internal/testutil/tfcache_find_noop.go | 14 ++++ tfexec/output_test.go | 2 - tfexec/plan_test.go | 2 - tfexec/providers_schema_test.go | 2 - tfexec/refresh_test.go | 2 - tfexec/show_test.go | 5 -- tfexec/terraform_go113_test.go | 27 -------- tfexec/terraform_test.go | 64 +++++++------------ tfexec/workspace_new_test.go | 1 - 17 files changed, 116 insertions(+), 137 deletions(-) create mode 100644 tfexec/internal/testutil/temp_dir.go create mode 100644 tfexec/internal/testutil/tfcache_find.go create mode 100644 tfexec/internal/testutil/tfcache_find_noop.go delete mode 100644 tfexec/terraform_go113_test.go diff --git a/tfexec/apply_test.go b/tfexec/apply_test.go index 39bde06f..f1c6a1f2 100644 --- a/tfexec/apply_test.go +++ b/tfexec/apply_test.go @@ -2,7 +2,6 @@ package tfexec import ( "context" - "os" "testing" "github.com/hashicorp/terraform-exec/tfexec/internal/testutil" @@ -10,7 +9,6 @@ import ( func TestApplyCmd(t *testing.T) { td := testTempDir(t) - defer os.RemoveAll(td) tf, err := NewTerraform(td, tfVersion(t, testutil.Latest012)) if err != nil { diff --git a/tfexec/destroy_test.go b/tfexec/destroy_test.go index ef6f7439..7ee20793 100644 --- a/tfexec/destroy_test.go +++ b/tfexec/destroy_test.go @@ -2,7 +2,6 @@ package tfexec import ( "context" - "os" "testing" "github.com/hashicorp/terraform-exec/tfexec/internal/testutil" @@ -10,7 +9,6 @@ import ( func TestDestroyCmd(t *testing.T) { td := testTempDir(t) - defer os.RemoveAll(td) tf, err := NewTerraform(td, tfVersion(t, testutil.Latest012)) if err != nil { diff --git a/tfexec/fmt_test.go b/tfexec/fmt_test.go index 517c2d04..4abe1128 100644 --- a/tfexec/fmt_test.go +++ b/tfexec/fmt_test.go @@ -3,13 +3,11 @@ package tfexec import ( "context" "errors" - "os" "testing" ) func TestFormat(t *testing.T) { td := testTempDir(t) - defer os.RemoveAll(td) tf, err := NewTerraform(td, tfVersion(t, "0.7.6")) if err != nil { diff --git a/tfexec/import_test.go b/tfexec/import_test.go index db4d5add..2f81e768 100644 --- a/tfexec/import_test.go +++ b/tfexec/import_test.go @@ -2,7 +2,6 @@ package tfexec import ( "context" - "os" "testing" "github.com/hashicorp/terraform-exec/tfexec/internal/testutil" @@ -10,7 +9,6 @@ import ( func TestImportCmd(t *testing.T) { td := testTempDir(t) - defer os.RemoveAll(td) tf, err := NewTerraform(td, tfVersion(t, testutil.Latest012)) if err != nil { diff --git a/tfexec/init_test.go b/tfexec/init_test.go index 1306f1cc..0434c398 100644 --- a/tfexec/init_test.go +++ b/tfexec/init_test.go @@ -2,7 +2,6 @@ package tfexec import ( "context" - "os" "testing" "github.com/hashicorp/terraform-exec/tfexec/internal/testutil" @@ -10,7 +9,6 @@ import ( func TestInitCmd(t *testing.T) { td := testTempDir(t) - defer os.RemoveAll(td) tf, err := NewTerraform(td, tfVersion(t, testutil.Latest012)) if err != nil { diff --git a/tfexec/internal/testutil/temp_dir.go b/tfexec/internal/testutil/temp_dir.go new file mode 100644 index 00000000..618a4fd6 --- /dev/null +++ b/tfexec/internal/testutil/temp_dir.go @@ -0,0 +1,24 @@ +// +build go1.14 + +package testutil + +import ( + "io/ioutil" + "os" + "testing" +) + +// TODO: Remove once we drop support for Go <1.15 +// in favour of native t.TempDir() +func TempDir(t *testing.T) string { + dir, err := ioutil.TempDir("", "tf") + if err != nil { + t.Fatalf("error creating temporary test directory: %s", err) + } + + t.Cleanup(func() { + os.RemoveAll(dir) + }) + + return dir +} diff --git a/tfexec/internal/testutil/tfcache.go b/tfexec/internal/testutil/tfcache.go index a7fb5de2..3826a4ea 100644 --- a/tfexec/internal/testutil/tfcache.go +++ b/tfexec/internal/testutil/tfcache.go @@ -1,11 +1,6 @@ package testutil import ( - "context" - "fmt" - "os" - "path/filepath" - "strings" "sync" "testing" @@ -49,42 +44,3 @@ func (tf *TFCache) Version(t *testing.T, v string) string { return f }) } - -func (tf *TFCache) find(t *testing.T, key string, finder func(dir string) tfinstall.ExecPathFinder) string { - t.Helper() - - if tf.dir == "" { - // panic instead of t.fatal as this is going to affect all downstream tests reusing the cache entry - panic("installDir not yet configured") - } - - tf.Lock() - defer tf.Unlock() - - if path, ok := tf.execs[key]; ok { - return path - } - - keyDir := key - keyDir = strings.ReplaceAll(keyDir, ":", "-") - keyDir = strings.ReplaceAll(keyDir, "/", "-") - - dir := filepath.Join(tf.dir, keyDir) - - t.Logf("caching exec %q in dir %q", key, dir) - - err := os.MkdirAll(dir, 0777) - if err != nil { - // panic instead of t.fatal as this is going to affect all downstream tests reusing the cache entry - panic(fmt.Sprintf("unable to mkdir %q: %s", dir, err)) - } - - path, err := tfinstall.Find(context.Background(), finder(dir)) - if err != nil { - // panic instead of t.fatal as this is going to affect all downstream tests reusing the cache entry - panic(fmt.Sprintf("error installing terraform %q: %s", key, err)) - } - tf.execs[key] = path - - return path -} diff --git a/tfexec/internal/testutil/tfcache_find.go b/tfexec/internal/testutil/tfcache_find.go new file mode 100644 index 00000000..28a249a4 --- /dev/null +++ b/tfexec/internal/testutil/tfcache_find.go @@ -0,0 +1,56 @@ +// +build go1.14 + +package testutil + +import ( + "context" + "fmt" + "os" + "path/filepath" + "strings" + "testing" + + "github.com/hashicorp/terraform-exec/tfinstall" +) + +func (tf *TFCache) find(t *testing.T, key string, finder func(dir string) tfinstall.ExecPathFinder) string { + t.Helper() + + if tf.dir == "" { + // panic instead of t.fatal as this is going to affect all downstream tests reusing the cache entry + panic("installDir not yet configured") + } + + tf.Lock() + defer tf.Unlock() + + if path, ok := tf.execs[key]; ok { + return path + } + + keyDir := key + keyDir = strings.ReplaceAll(keyDir, ":", "-") + keyDir = strings.ReplaceAll(keyDir, "/", "-") + + dir := filepath.Join(tf.dir, keyDir) + + t.Logf("caching exec %q in dir %q", key, dir) + + err := os.MkdirAll(dir, 0777) + if err != nil { + // panic instead of t.fatal as this is going to affect all downstream tests reusing the cache entry + panic(fmt.Sprintf("unable to mkdir %q: %s", dir, err)) + } + + ctx, cancelFunc := context.WithCancel(context.Background()) + t.Cleanup(cancelFunc) + + path, err := tfinstall.Find(ctx, finder(dir)) + if err != nil { + // panic instead of t.fatal as this is going to affect all downstream tests reusing the cache entry + panic(fmt.Sprintf("error installing terraform %q: %s", key, err)) + } + tf.execs[key] = path + + return path +} diff --git a/tfexec/internal/testutil/tfcache_find_noop.go b/tfexec/internal/testutil/tfcache_find_noop.go new file mode 100644 index 00000000..5b8909d7 --- /dev/null +++ b/tfexec/internal/testutil/tfcache_find_noop.go @@ -0,0 +1,14 @@ +// +build !go1.14 + +package testutil + +import ( + "runtime" + "testing" + + "github.com/hashicorp/terraform-exec/tfinstall" +) + +func (tf *TFCache) find(t *testing.T, key string, finder func(dir string) tfinstall.ExecPathFinder) string { + panic("not implemented for " + runtime.Version()) +} diff --git a/tfexec/output_test.go b/tfexec/output_test.go index 5adf0141..01249b53 100644 --- a/tfexec/output_test.go +++ b/tfexec/output_test.go @@ -2,7 +2,6 @@ package tfexec import ( "context" - "os" "testing" "github.com/hashicorp/terraform-exec/tfexec/internal/testutil" @@ -10,7 +9,6 @@ import ( func TestOutputCmd(t *testing.T) { td := testTempDir(t) - defer os.RemoveAll(td) tf, err := NewTerraform(td, tfVersion(t, testutil.Latest012)) if err != nil { diff --git a/tfexec/plan_test.go b/tfexec/plan_test.go index a5ad9cbc..b30c74f8 100644 --- a/tfexec/plan_test.go +++ b/tfexec/plan_test.go @@ -2,7 +2,6 @@ package tfexec import ( "context" - "os" "testing" "github.com/hashicorp/terraform-exec/tfexec/internal/testutil" @@ -10,7 +9,6 @@ import ( func TestPlanCmd(t *testing.T) { td := testTempDir(t) - defer os.RemoveAll(td) tf, err := NewTerraform(td, tfVersion(t, testutil.Latest012)) if err != nil { diff --git a/tfexec/providers_schema_test.go b/tfexec/providers_schema_test.go index 8a127adc..640e2661 100644 --- a/tfexec/providers_schema_test.go +++ b/tfexec/providers_schema_test.go @@ -2,7 +2,6 @@ package tfexec import ( "context" - "os" "testing" "github.com/hashicorp/terraform-exec/tfexec/internal/testutil" @@ -10,7 +9,6 @@ import ( func TestProvidersSchemaCmd(t *testing.T) { td := testTempDir(t) - defer os.RemoveAll(td) tf, err := NewTerraform(td, tfVersion(t, testutil.Latest012)) if err != nil { diff --git a/tfexec/refresh_test.go b/tfexec/refresh_test.go index 67f6d5ee..e17d9938 100644 --- a/tfexec/refresh_test.go +++ b/tfexec/refresh_test.go @@ -2,7 +2,6 @@ package tfexec import ( "context" - "os" "testing" "github.com/hashicorp/terraform-exec/tfexec/internal/testutil" @@ -10,7 +9,6 @@ import ( func TestRefreshCmd(t *testing.T) { td := testTempDir(t) - defer os.RemoveAll(td) tf, err := NewTerraform(td, tfVersion(t, testutil.Latest013)) if err != nil { diff --git a/tfexec/show_test.go b/tfexec/show_test.go index 59fb1e0b..b33ab6c2 100644 --- a/tfexec/show_test.go +++ b/tfexec/show_test.go @@ -2,7 +2,6 @@ package tfexec import ( "context" - "os" "testing" "github.com/hashicorp/terraform-exec/tfexec/internal/testutil" @@ -10,7 +9,6 @@ import ( func TestShowCmd(t *testing.T) { td := testTempDir(t) - defer os.RemoveAll(td) tf, err := NewTerraform(td, tfVersion(t, testutil.Latest012)) if err != nil { @@ -32,7 +30,6 @@ func TestShowCmd(t *testing.T) { func TestShowStateFileCmd(t *testing.T) { td := testTempDir(t) - defer os.RemoveAll(td) tf, err := NewTerraform(td, tfVersion(t, testutil.Latest012)) if err != nil { @@ -54,7 +51,6 @@ func TestShowStateFileCmd(t *testing.T) { func TestShowPlanFileCmd(t *testing.T) { td := testTempDir(t) - defer os.RemoveAll(td) tf, err := NewTerraform(td, tfVersion(t, testutil.Latest012)) if err != nil { @@ -76,7 +72,6 @@ func TestShowPlanFileCmd(t *testing.T) { func TestShowPlanFileRawCmd(t *testing.T) { td := testTempDir(t) - defer os.RemoveAll(td) tf, err := NewTerraform(td, tfVersion(t, testutil.Latest012)) if err != nil { diff --git a/tfexec/terraform_go113_test.go b/tfexec/terraform_go113_test.go deleted file mode 100644 index 1f0b5494..00000000 --- a/tfexec/terraform_go113_test.go +++ /dev/null @@ -1,27 +0,0 @@ -// This file contains tests that only compile/work in Go 1.13 and forward -// +build go1.13 - -package tfexec - -import ( - "errors" - "os" - "testing" -) - -// test that a suitable error is returned if NewTerraform is called without a valid -// executable path -func TestNoTerraformBinary(t *testing.T) { - td := testTempDir(t) - defer os.RemoveAll(td) - - _, err := NewTerraform(td, "") - if err == nil { - t.Fatal("expected NewTerraform to error, but it did not") - } - - var e *ErrNoSuitableBinary - if !errors.As(err, &e) { - t.Fatal("expected error to be ErrNoSuitableBinary") - } -} diff --git a/tfexec/terraform_test.go b/tfexec/terraform_test.go index 93b8924f..cb1cd614 100644 --- a/tfexec/terraform_test.go +++ b/tfexec/terraform_test.go @@ -5,30 +5,30 @@ import ( "errors" "io/ioutil" "os" - "path/filepath" - "sync" "testing" "github.com/hashicorp/terraform-exec/tfexec/internal/testutil" - "github.com/hashicorp/terraform-exec/tfinstall" ) +var tfCache *testutil.TFCache + func TestMain(m *testing.M) { os.Exit(func() int { var err error - installDir, err = ioutil.TempDir("", "tfinstall") + installDir, err := ioutil.TempDir("", "tfinstall") if err != nil { panic(err) } defer os.RemoveAll(installDir) + tfCache = testutil.NewTFCache(installDir) + return m.Run() }()) } func TestSetEnv(t *testing.T) { td := testTempDir(t) - defer os.RemoveAll(td) tf, err := NewTerraform(td, tfVersion(t, testutil.Latest012)) if err != nil { @@ -63,7 +63,6 @@ func TestSetEnv(t *testing.T) { func TestCheckpointDisablePropagation(t *testing.T) { td := testTempDir(t) - defer os.RemoveAll(td) tf, err := NewTerraform(td, tfVersion(t, testutil.Latest012)) if err != nil { @@ -141,49 +140,30 @@ func TestCheckpointDisablePropagation(t *testing.T) { }) } -func testTempDir(t *testing.T) string { - d, err := ioutil.TempDir("", "tf") - if err != nil { - t.Fatalf("error creating temporary test directory: %s", err) +// test that a suitable error is returned if NewTerraform is called without a valid +// executable path +func TestNoTerraformBinary(t *testing.T) { + td := testTempDir(t) + + _, err := NewTerraform(td, "") + if err == nil { + t.Fatal("expected NewTerraform to error, but it did not") } - // TODO: add t.Cleanup so we can remove the defers - return d + var e *ErrNoSuitableBinary + if !errors.As(err, &e) { + t.Fatal("expected error to be ErrNoSuitableBinary") + } } -type installedVersion struct { - path string - err error +func testTempDir(t *testing.T) string { + return testutil.TempDir(t) } -var ( - installDir string - installedVersionLock sync.Mutex - installedVersions = map[string]installedVersion{} -) - func tfVersion(t *testing.T, v string) string { - if installDir == "" { - t.Fatalf("installDir not yet configured, TestMain must run first") - } - - installedVersionLock.Lock() - defer installedVersionLock.Unlock() - - iv, ok := installedVersions[v] - if !ok { - dir := filepath.Join(installDir, v) - err := os.MkdirAll(dir, 0777) - if err != nil { - t.Fatal(err) - } - iv.path, iv.err = tfinstall.Find(context.Background(), tfinstall.ExactVersion(v, dir)) - installedVersions[v] = iv - } - - if iv.err != nil { - t.Fatalf("error installing terraform version %q: %s", v, iv.err) + if tfCache == nil { + t.Fatalf("tfCache not yet configured, TestMain must run first") } - return iv.path + return tfCache.Version(t, v) } diff --git a/tfexec/workspace_new_test.go b/tfexec/workspace_new_test.go index 0dda3339..eaf7c9db 100644 --- a/tfexec/workspace_new_test.go +++ b/tfexec/workspace_new_test.go @@ -10,7 +10,6 @@ import ( func TestWorkspaceNewCmd(t *testing.T) { td := testTempDir(t) - defer os.RemoveAll(td) tf, err := NewTerraform(td, tfVersion(t, testutil.Latest013)) if err != nil {