Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Introduce test harness into unit tests #69544

Merged
merged 1 commit into from
Oct 30, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions pkg/volume/flexvolume/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
29 changes: 21 additions & 8 deletions pkg/volume/flexvolume/attacher_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"),
Expand All @@ -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)),
Expand All @@ -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)),
Expand All @@ -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"),
)
Expand Down
12 changes: 4 additions & 8 deletions pkg/volume/flexvolume/common_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand All @@ -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")
Expand Down
16 changes: 12 additions & 4 deletions pkg/volume/flexvolume/detacher_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"),
Expand All @@ -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"),
Expand Down
8 changes: 6 additions & 2 deletions pkg/volume/flexvolume/mounter_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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{
Expand All @@ -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
Expand Down
15 changes: 11 additions & 4 deletions pkg/volume/flexvolume/plugin_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"),
)
Expand All @@ -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)),
Expand Down
8 changes: 6 additions & 2 deletions pkg/volume/flexvolume/unmounter_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"),
Expand Down
1 change: 1 addition & 0 deletions test/utils/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -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",
],
Expand Down
23 changes: 23 additions & 0 deletions test/utils/harness/BUILD
Original file line number Diff line number Diff line change
@@ -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"],
)
71 changes: 71 additions & 0 deletions test/utils/harness/harness.go
Original file line number Diff line number Diff line change
@@ -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 {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: Maybe make Harness an interface and TempDirCleanupHarness a concrete type of Harness? That way folks can implement different types of Harnesses?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So the reason not to is because Harness embeds testing.T, so we can use it more interchangeably with existing code that wants a testing.T. I do agree that once we've got it so that everyone is using the harness, we should move to an interface!

*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 {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@justinsb nice pattern for cleaning tempdirs. I think we could use this harness pattern to cleanup any other resources we allocate . Would be interesting to see where else can we leverage this

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
}