From 58fc3c8c23dbd8de631b93cce355dfaa7e96eb1f Mon Sep 17 00:00:00 2001 From: Justin Santa Barbara Date: Mon, 8 Oct 2018 18:37:48 -0400 Subject: [PATCH] Introduce test harness into unit tests The harness lets us execute code after the test has completed. Use it to clean-up temp dirs in the flexvolume tests (initially). --- pkg/volume/flexvolume/BUILD | 1 + pkg/volume/flexvolume/attacher_test.go | 29 +++++++--- pkg/volume/flexvolume/common_test.go | 12 ++--- pkg/volume/flexvolume/detacher_test.go | 16 ++++-- pkg/volume/flexvolume/mounter_test.go | 8 ++- pkg/volume/flexvolume/plugin_test.go | 15 ++++-- pkg/volume/flexvolume/unmounter_test.go | 8 ++- test/utils/BUILD | 1 + test/utils/harness/BUILD | 23 ++++++++ test/utils/harness/harness.go | 71 +++++++++++++++++++++++++ 10 files changed, 156 insertions(+), 28 deletions(-) create mode 100644 test/utils/harness/BUILD create mode 100644 test/utils/harness/harness.go diff --git a/pkg/volume/flexvolume/BUILD b/pkg/volume/flexvolume/BUILD index fff15f102e..6b8b80a8d1 100644 --- a/pkg/volume/flexvolume/BUILD +++ b/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", diff --git a/pkg/volume/flexvolume/attacher_test.go b/pkg/volume/flexvolume/attacher_test.go index 8472886dad..f8e9f12cdb 100644 --- a/pkg/volume/flexvolume/attacher_test.go +++ b/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"), ) diff --git a/pkg/volume/flexvolume/common_test.go b/pkg/volume/flexvolume/common_test.go index fb99d1add4..618af300f1 100644 --- a/pkg/volume/flexvolume/common_test.go +++ b/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") diff --git a/pkg/volume/flexvolume/detacher_test.go b/pkg/volume/flexvolume/detacher_test.go index a1bc27b37a..6974feee2f 100644 --- a/pkg/volume/flexvolume/detacher_test.go +++ b/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"), diff --git a/pkg/volume/flexvolume/mounter_test.go b/pkg/volume/flexvolume/mounter_test.go index 70dac00728..bf10fd7481 100644 --- a/pkg/volume/flexvolume/mounter_test.go +++ b/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 diff --git a/pkg/volume/flexvolume/plugin_test.go b/pkg/volume/flexvolume/plugin_test.go index ae267fbd9f..f6afe704aa 100644 --- a/pkg/volume/flexvolume/plugin_test.go +++ b/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)), diff --git a/pkg/volume/flexvolume/unmounter_test.go b/pkg/volume/flexvolume/unmounter_test.go index fe7964a94a..0f0991944c 100644 --- a/pkg/volume/flexvolume/unmounter_test.go +++ b/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"), diff --git a/test/utils/BUILD b/test/utils/BUILD index 5a1023742f..f594b7ba14 100644 --- a/test/utils/BUILD +++ b/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", ], diff --git a/test/utils/harness/BUILD b/test/utils/harness/BUILD new file mode 100644 index 0000000000..510a812713 --- /dev/null +++ b/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"], +) diff --git a/test/utils/harness/harness.go b/test/utils/harness/harness.go new file mode 100644 index 0000000000..a3c827318f --- /dev/null +++ b/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 +}