Merge pull request #69544 from justinsb/tempdir_cleanup

Introduce test harness into unit tests

pkg/volume/flexvolume/BUILD

@@ -64,6 +64,7 @@ go_test(
         "//staging/src/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library",
         "//staging/src/k8s.io/apimachinery/pkg/types:go_default_library",
         "//staging/src/k8s.io/client-go/util/testing:go_default_library",
+        "//test/utils/harness:go_default_library",
         "//vendor/github.com/fsnotify/fsnotify:go_default_library",
         "//vendor/github.com/stretchr/testify/assert:go_default_library",
         "//vendor/k8s.io/utils/exec:go_default_library",

pkg/volume/flexvolume/attacher_test.go

@@ -22,12 +22,16 @@ import (
 
 	"k8s.io/api/core/v1"
 	"k8s.io/kubernetes/pkg/volume"
+	"k8s.io/kubernetes/test/utils/harness"
 )
 
-func TestAttach(t *testing.T) {
+func TestAttach(tt *testing.T) {
+	t := harness.For(tt)
+	defer t.Close()
+
 	spec := fakeVolumeSpec()
 
-	plugin, _ := testPlugin()
+	plugin, _ := testPlugin(t)
 	plugin.runner = fakeRunner(
 		assertDriverCall(t, notSupportedOutput(), attachCmd,
 			specJSON(plugin, spec, nil), "localhost"),
@@ -37,10 +41,13 @@ func TestAttach(t *testing.T) {
 	a.Attach(spec, "localhost")
 }
 
-func TestWaitForAttach(t *testing.T) {
+func TestWaitForAttach(tt *testing.T) {
+	t := harness.For(tt)
+	defer t.Close()
+
 	spec := fakeVolumeSpec()
 	var pod *v1.Pod
-	plugin, _ := testPlugin()
+	plugin, _ := testPlugin(t)
 	plugin.runner = fakeRunner(
 		assertDriverCall(t, notSupportedOutput(), waitForAttachCmd, "/dev/sdx",
 			specJSON(plugin, spec, nil)),
@@ -50,10 +57,13 @@ func TestWaitForAttach(t *testing.T) {
 	a.WaitForAttach(spec, "/dev/sdx", pod, 1*time.Second)
 }
 
-func TestMountDevice(t *testing.T) {
+func TestMountDevice(tt *testing.T) {
+	t := harness.For(tt)
+	defer t.Close()
+
 	spec := fakeVolumeSpec()
 
-	plugin, rootDir := testPlugin()
+	plugin, rootDir := testPlugin(t)
 	plugin.runner = fakeRunner(
 		assertDriverCall(t, notSupportedOutput(), mountDeviceCmd, rootDir+"/mount-dir", "/dev/sdx",
 			specJSON(plugin, spec, nil)),
@@ -63,10 +73,13 @@ func TestMountDevice(t *testing.T) {
 	a.MountDevice(spec, "/dev/sdx", rootDir+"/mount-dir")
 }
 
-func TestIsVolumeAttached(t *testing.T) {
+func TestIsVolumeAttached(tt *testing.T) {
+	t := harness.For(tt)
+	defer t.Close()
+
 	spec := fakeVolumeSpec()
 
-	plugin, _ := testPlugin()
+	plugin, _ := testPlugin(t)
 	plugin.runner = fakeRunner(
 		assertDriverCall(t, notSupportedOutput(), isAttached, specJSON(plugin, spec, nil), "localhost"),
 	)

pkg/volume/flexvolume/common_test.go

@@ -18,22 +18,18 @@ package flexvolume
 
 import (
 	"encoding/json"
-	"testing"
 
 	"k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
-	utiltesting "k8s.io/client-go/util/testing"
 	"k8s.io/kubernetes/pkg/volume"
 	volumetesting "k8s.io/kubernetes/pkg/volume/testing"
+	"k8s.io/kubernetes/test/utils/harness"
 	"k8s.io/utils/exec"
 	fakeexec "k8s.io/utils/exec/testing"
 )
 
-func testPlugin() (*flexVolumeAttachablePlugin, string) {
-	rootDir, err := utiltesting.MkTmpdir("flexvolume_test")
-	if err != nil {
-		panic("error creating temp dir: " + err.Error())
-	}
+func testPlugin(h *harness.Harness) (*flexVolumeAttachablePlugin, string) {
+	rootDir := h.TempDir("", "flexvolume_test")
 	return &flexVolumeAttachablePlugin{
 		flexVolumePlugin: &flexVolumePlugin{
 			driverName:          "test",
@@ -44,7 +40,7 @@ func testPlugin() (*flexVolumeAttachablePlugin, string) {
 	}, rootDir
 }
 
-func assertDriverCall(t *testing.T, output fakeexec.FakeCombinedOutputAction, expectedCommand string, expectedArgs ...string) fakeexec.FakeCommandAction {
+func assertDriverCall(t *harness.Harness, output fakeexec.FakeCombinedOutputAction, expectedCommand string, expectedArgs ...string) fakeexec.FakeCommandAction {
 	return func(cmd string, args ...string) exec.Cmd {
 		if cmd != "/plugin/test" {
 			t.Errorf("Wrong executable called: got %v, expected %v", cmd, "/plugin/test")

pkg/volume/flexvolume/detacher_test.go

@@ -18,10 +18,15 @@ package flexvolume
 
 import (
 	"testing"
+
+	"k8s.io/kubernetes/test/utils/harness"
 )
 
-func TestDetach(t *testing.T) {
-	plugin, _ := testPlugin()
+func TestDetach(tt *testing.T) {
+	t := harness.For(tt)
+	defer t.Close()
+
+	plugin, _ := testPlugin(t)
 	plugin.runner = fakeRunner(
 		assertDriverCall(t, notSupportedOutput(), detachCmd,
 			"sdx", "localhost"),
@@ -31,8 +36,11 @@ func TestDetach(t *testing.T) {
 	d.Detach("sdx", "localhost")
 }
 
-func TestUnmountDevice(t *testing.T) {
-	plugin, rootDir := testPlugin()
+func TestUnmountDevice(tt *testing.T) {
+	t := harness.For(tt)
+	defer t.Close()
+
+	plugin, rootDir := testPlugin(t)
 	plugin.runner = fakeRunner(
 		assertDriverCall(t, notSupportedOutput(), unmountDeviceCmd,
 			rootDir+"/mount-dir"),

pkg/volume/flexvolume/mounter_test.go

@@ -23,9 +23,13 @@ import (
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/types"
 	"k8s.io/kubernetes/pkg/util/mount"
+	"k8s.io/kubernetes/test/utils/harness"
 )
 
-func TestSetUpAt(t *testing.T) {
+func TestSetUpAt(tt *testing.T) {
+	t := harness.For(tt)
+	defer t.Close()
+
 	spec := fakeVolumeSpec()
 	pod := &v1.Pod{
 		ObjectMeta: metav1.ObjectMeta{
@@ -39,7 +43,7 @@ func TestSetUpAt(t *testing.T) {
 	}
 	mounter := &mount.FakeMounter{}
 
-	plugin, rootDir := testPlugin()
+	plugin, rootDir := testPlugin(t)
 	plugin.unsupportedCommands = []string{"unsupportedCmd"}
 	plugin.runner = fakeRunner(
 		// first call without fsGroup

pkg/volume/flexvolume/plugin_test.go

@@ -19,11 +19,15 @@ package flexvolume
 import (
 	"testing"
 
+	"k8s.io/kubernetes/test/utils/harness"
 	exec "k8s.io/utils/exec/testing"
 )
 
-func TestInit(t *testing.T) {
-	plugin, _ := testPlugin()
+func TestInit(tt *testing.T) {
+	t := harness.For(tt)
+	defer t.Close()
+
+	plugin, _ := testPlugin(t)
 	plugin.runner = fakeRunner(
 		assertDriverCall(t, successOutput(), "init"),
 	)
@@ -37,9 +41,12 @@ func fakeVolumeNameOutput(name string) exec.FakeCombinedOutputAction {
 	})
 }
 
-func TestGetVolumeName(t *testing.T) {
+func TestGetVolumeName(tt *testing.T) {
+	t := harness.For(tt)
+	defer t.Close()
+
 	spec := fakeVolumeSpec()
-	plugin, _ := testPlugin()
+	plugin, _ := testPlugin(t)
 	plugin.runner = fakeRunner(
 		assertDriverCall(t, fakeVolumeNameOutput(spec.Name()), getVolumeNameCmd,
 			specJSON(plugin, spec, nil)),

pkg/volume/flexvolume/unmounter_test.go

@@ -21,12 +21,16 @@ import (
 
 	"k8s.io/apimachinery/pkg/types"
 	"k8s.io/kubernetes/pkg/util/mount"
+	"k8s.io/kubernetes/test/utils/harness"
 )
 
-func TestTearDownAt(t *testing.T) {
+func TestTearDownAt(tt *testing.T) {
+	t := harness.For(tt)
+	defer t.Close()
+
 	mounter := &mount.FakeMounter{}
 
-	plugin, rootDir := testPlugin()
+	plugin, rootDir := testPlugin(t)
 	plugin.runner = fakeRunner(
 		assertDriverCall(t, notSupportedOutput(), unmountCmd,
 			rootDir+"/mount-dir"),

test/utils/BUILD

@@ -72,6 +72,7 @@ filegroup(
     name = "all-srcs",
     srcs = [
         ":package-srcs",
+        "//test/utils/harness:all-srcs",
         "//test/utils/image:all-srcs",
         "//test/utils/junit:all-srcs",
     ],

test/utils/harness/BUILD

@@ -0,0 +1,23 @@
+load("@io_bazel_rules_go//go:def.bzl", "go_library")
+
+go_library(
+    name = "go_default_library",
+    srcs = ["harness.go"],
+    importpath = "k8s.io/kubernetes/test/utils/harness",
+    visibility = ["//visibility:public"],
+    deps = ["//vendor/github.com/golang/glog:go_default_library"],
+)
+
+filegroup(
+    name = "package-srcs",
+    srcs = glob(["**"]),
+    tags = ["automanaged"],
+    visibility = ["//visibility:private"],
+)
+
+filegroup(
+    name = "all-srcs",
+    srcs = [":package-srcs"],
+    tags = ["automanaged"],
+    visibility = ["//visibility:public"],
+)

test/utils/harness/harness.go

@@ -0,0 +1,71 @@
+/*
+Copyright 2018 The Kubernetes Authors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package harness
+
+import (
+	"io/ioutil"
+	"os"
+	"testing"
+
+	"github.com/golang/glog"
+)
+
+// Harness adds some functionality to testing.T, in particular resource cleanup.
+// It embeds testing.T, so should have the same signature.
+//
+// Example usage:
+// ```
+// func MyTest(tt *testing.T) {
+//   t := harness.For(tt)
+//   defer t.Close()
+//   ...
+// }
+// ```
+type Harness struct {
+	*testing.T
+	defers []func() error
+}
+
+// For creates a Harness from a testing.T
+// Callers must call Close on the Harness so that resources can be cleaned up
+func For(t *testing.T) *Harness {
+	h := &Harness{T: t}
+	return h
+}
+
+// Close cleans up any owned resources, and should be called in a defer block after For
+func (h *Harness) Close() {
+	for _, d := range h.defers {
+		if err := d(); err != nil {
+			glog.Warningf("error closing harness: %v", err)
+		}
+	}
+}
+
+// TempDir is a wrapper around ioutil.TempDir for tests.
+// It automatically fails the test if we can't create a temp file,
+// and deletes the temp directory when Close is called on the Harness
+func (h *Harness) TempDir(baseDir string, prefix string) string {
+	tempDir, err := ioutil.TempDir(baseDir, prefix)
+	if err != nil {
+		h.Fatalf("unable to create tempdir: %v", err)
+	}
+	h.defers = append(h.defers, func() error {
+		return os.RemoveAll(tempDir)
+	})
+	return tempDir
+}