Extract TempDirManager

We are now returning an error instead of using an Expectation inline.

pkg/framework/test/etcd.go

@@ -1,8 +1,6 @@
 package test
 
 import (
-	"io/ioutil"
-	"os"
 	"os/exec"
 
 	"github.com/onsi/gomega"
@@ -18,14 +16,23 @@ type Etcd struct {
 	session        *gexec.Session
 	stdOut         *gbytes.Buffer
 	stdErr         *gbytes.Buffer
-	tempDirManager TempDirManager
+	dataDirManager DataDirManager
+}
+
+// DataDirManager knows how to create and destroy Etcd's data directory.
+type DataDirManager interface {
+	Create() (string, error)
+	Destroy() error
 }
 
 // Start starts the etcd, and returns a gexec.Session. To stop it again, call Terminate and Wait on that session.
 func (e *Etcd) Start() error {
-	e.tempDirManager = &tempDirManager{}
+	e.dataDirManager = NewTempDirManager()
 
-	dataDir := e.tempDirManager.Create()
+	dataDir, err := e.dataDirManager.Create()
+	if err != nil {
+		return err
+	}
 
 	args := []string{
 		"--debug",
@@ -38,7 +45,6 @@ func (e *Etcd) Start() error {
 	}
 
 	command := exec.Command(e.Path, args...)
-	var err error
 	e.session, err = gexec.Start(command, e.stdOut, e.stdErr)
 	return err
 }
@@ -47,7 +53,7 @@ func (e *Etcd) Start() error {
 func (e *Etcd) Stop() {
 	if e.session != nil {
 		e.session.Terminate().Wait()
-		err := e.tempDirManager.Destroy()
+		err := e.dataDirManager.Destroy()
 		gomega.Expect(err).NotTo(gomega.HaveOccurred())
 	}
 }
@@ -61,32 +67,3 @@ func (e *Etcd) ExitCode() int {
 func (e *Etcd) Buffer() *gbytes.Buffer {
 	return e.session.Buffer()
 }
-
-//------
-
-// TempDirManager knows how to create and destroy temporary directories.
-type TempDirManager interface {
-	Create() string
-	Destroy() error
-}
-
-//go:generate counterfeiter . TempDirManager
-
-type tempDirManager struct {
-	dir string
-}
-
-func (t *tempDirManager) Create() string {
-	var err error
-	t.dir, err = ioutil.TempDir("", "kube-test-framework")
-	gomega.ExpectWithOffset(2, err).NotTo(gomega.HaveOccurred(),
-		"expected to be able to create a temporary directory in the kube test framework")
-	return t.dir
-}
-
-func (t *tempDirManager) Destroy() error {
-	if t.dir != "" {
-		return os.RemoveAll(t.dir)
-	}
-	return nil
-}

pkg/framework/test/temp_dir_manager.go

@@ -0,0 +1,49 @@
+package test
+
+import (
+	"io/ioutil"
+	"os"
+)
+
+// TempDirMaker can create directories.
+type TempDirMaker func(dir, prefix string) (name string, err error)
+
+// TempDirRemover can delete directories
+type TempDirRemover func(dir string) error
+
+// NewTempDirManager returns a new manager for creation and deleteion of temporary directories.
+func NewTempDirManager() *TempDirManager {
+	return &TempDirManager{
+		Maker:   ioutil.TempDir,
+		Remover: os.RemoveAll,
+	}
+}
+
+// TempDirManager knows when to call the directory maker and remover and keeps track of created directories.
+type TempDirManager struct {
+	Maker   TempDirMaker
+	Remover TempDirRemover
+	dir     string
+}
+
+// Create knows how to create a temporary directory and how to keep track of it.
+func (t *TempDirManager) Create() (string, error) {
+	if t.dir == "" {
+		dir, err := t.Maker("", "kube-test-framework")
+		if err != nil {
+			return "", err
+		}
+		t.dir = dir
+	}
+	return t.dir, nil
+}
+
+// Destroy knows how to destroy a previously created directory.
+func (t *TempDirManager) Destroy() error {
+	if t.dir != "" {
+		err := t.Remover(t.dir)
+		t.dir = ""
+		return err
+	}
+	return nil
+}

pkg/framework/test/temp_dir_manager_test.go

@@ -0,0 +1,111 @@
+package test_test
+
+import (
+	"fmt"
+
+	. "k8s.io/kubectl/pkg/framework/test"
+
+	. "github.com/onsi/ginkgo"
+	. "github.com/onsi/gomega"
+)
+
+var _ = Describe("TempDirManager", func() {
+	var (
+		manager            *TempDirManager
+		removerError       error
+		createError        error
+		managedDirCount    int
+		separateDirCounter int
+	)
+	BeforeEach(func() {
+		managedDirCount = 0
+		separateDirCounter = 0
+		createError = nil
+		removerError = nil
+		manager = NewTempDirManager()
+		manager.Maker = func(dir, prefix string) (string, error) {
+			managedDirCount += 1
+			separateDirCounter += 1
+			return fmt.Sprintf("%d-%s-%s", separateDirCounter, dir, prefix), createError
+		}
+		manager.Remover = func(dir string) error {
+			managedDirCount -= 1
+			return removerError
+		}
+	})
+
+	It("can creates and remove directories", func() {
+		Expect(managedDirCount).To(Equal(0))
+		manager.Create()
+		Expect(managedDirCount).To(Equal(1))
+		manager.Destroy()
+		Expect(managedDirCount).To(Equal(0))
+	})
+
+	Context("when I call Create() multiple times on the same manager", func() {
+		It("returns the same directory every time", func() {
+			var dir1, dir2 string
+			var err error
+
+			Expect(managedDirCount).To(Equal(0))
+
+			dir1, err = manager.Create()
+			Expect(err).NotTo(HaveOccurred())
+			Expect(managedDirCount).To(Equal(1))
+
+			dir2, err = manager.Create()
+			Expect(err).NotTo(HaveOccurred())
+			Expect(managedDirCount).To(Equal(1))
+			Expect(dir1).To(Equal(dir2))
+		})
+
+		It("deletes the managed directory as soon as Destroy() is called even once", func() {
+			var err error
+
+			Expect(managedDirCount).To(Equal(0))
+
+			_, err = manager.Create()
+			Expect(err).NotTo(HaveOccurred())
+			_, err = manager.Create()
+			Expect(err).NotTo(HaveOccurred())
+			Expect(managedDirCount).To(Equal(1))
+
+			manager.Destroy()
+			Expect(managedDirCount).To(Equal(0))
+		})
+	})
+
+	Context("when I call Destroy() without calling create first", func() {
+		It("does nothing", func() {
+			Expect(managedDirCount).To(Equal(0))
+			manager.Destroy()
+			Expect(managedDirCount).To(Equal(0))
+		})
+	})
+
+	Context("when the remover returns an error", func() {
+		JustBeforeEach(func() {
+			removerError = fmt.Errorf("Error on removing dir")
+		})
+		It("handles that error depending on whether Create() has been called", func() {
+			By("avoiding the error if Create() has not been called")
+			err := manager.Destroy()
+			Expect(err).NotTo(HaveOccurred())
+
+			By("propagating the error if Create() has been called")
+			manager.Create()
+			err = manager.Destroy()
+			Expect(err).To(MatchError("Error on removing dir"))
+		})
+	})
+
+	Context("when the creater returns an error", func() {
+		JustBeforeEach(func() {
+			createError = fmt.Errorf("Error on creating dir")
+		})
+		It("bubbles up the error", func() {
+			_, err := manager.Create()
+			Expect(err).To(MatchError("Error on creating dir"))
+		})
+	})
+})

pkg/framework/test/testfakes/fake_temp_dir_manager.go

@@ -1,139 +0,0 @@
-// Code generated by counterfeiter. DO NOT EDIT.
-package testfakes
-
-import (
-	"sync"
-
-	"k8s.io/kubectl/pkg/framework/test"
-)
-
-type FakeTempDirManager struct {
-	CreateStub        func() string
-	createMutex       sync.RWMutex
-	createArgsForCall []struct{}
-	createReturns     struct {
-		result1 string
-	}
-	createReturnsOnCall map[int]struct {
-		result1 string
-	}
-	DestroyStub        func() error
-	destroyMutex       sync.RWMutex
-	destroyArgsForCall []struct{}
-	destroyReturns     struct {
-		result1 error
-	}
-	destroyReturnsOnCall map[int]struct {
-		result1 error
-	}
-	invocations      map[string][][]interface{}
-	invocationsMutex sync.RWMutex
-}
-
-func (fake *FakeTempDirManager) Create() string {
-	fake.createMutex.Lock()
-	ret, specificReturn := fake.createReturnsOnCall[len(fake.createArgsForCall)]
-	fake.createArgsForCall = append(fake.createArgsForCall, struct{}{})
-	fake.recordInvocation("Create", []interface{}{})
-	fake.createMutex.Unlock()
-	if fake.CreateStub != nil {
-		return fake.CreateStub()
-	}
-	if specificReturn {
-		return ret.result1
-	}
-	return fake.createReturns.result1
-}
-
-func (fake *FakeTempDirManager) CreateCallCount() int {
-	fake.createMutex.RLock()
-	defer fake.createMutex.RUnlock()
-	return len(fake.createArgsForCall)
-}
-
-func (fake *FakeTempDirManager) CreateReturns(result1 string) {
-	fake.CreateStub = nil
-	fake.createReturns = struct {
-		result1 string
-	}{result1}
-}
-
-func (fake *FakeTempDirManager) CreateReturnsOnCall(i int, result1 string) {
-	fake.CreateStub = nil
-	if fake.createReturnsOnCall == nil {
-		fake.createReturnsOnCall = make(map[int]struct {
-			result1 string
-		})
-	}
-	fake.createReturnsOnCall[i] = struct {
-		result1 string
-	}{result1}
-}
-
-func (fake *FakeTempDirManager) Destroy() error {
-	fake.destroyMutex.Lock()
-	ret, specificReturn := fake.destroyReturnsOnCall[len(fake.destroyArgsForCall)]
-	fake.destroyArgsForCall = append(fake.destroyArgsForCall, struct{}{})
-	fake.recordInvocation("Destroy", []interface{}{})
-	fake.destroyMutex.Unlock()
-	if fake.DestroyStub != nil {
-		return fake.DestroyStub()
-	}
-	if specificReturn {
-		return ret.result1
-	}
-	return fake.destroyReturns.result1
-}
-
-func (fake *FakeTempDirManager) DestroyCallCount() int {
-	fake.destroyMutex.RLock()
-	defer fake.destroyMutex.RUnlock()
-	return len(fake.destroyArgsForCall)
-}
-
-func (fake *FakeTempDirManager) DestroyReturns(result1 error) {
-	fake.DestroyStub = nil
-	fake.destroyReturns = struct {
-		result1 error
-	}{result1}
-}
-
-func (fake *FakeTempDirManager) DestroyReturnsOnCall(i int, result1 error) {
-	fake.DestroyStub = nil
-	if fake.destroyReturnsOnCall == nil {
-		fake.destroyReturnsOnCall = make(map[int]struct {
-			result1 error
-		})
-	}
-	fake.destroyReturnsOnCall[i] = struct {
-		result1 error
-	}{result1}
-}
-
-func (fake *FakeTempDirManager) Invocations() map[string][][]interface{} {
-	fake.invocationsMutex.RLock()
-	defer fake.invocationsMutex.RUnlock()
-	fake.createMutex.RLock()
-	defer fake.createMutex.RUnlock()
-	fake.destroyMutex.RLock()
-	defer fake.destroyMutex.RUnlock()
-	copiedInvocations := map[string][][]interface{}{}
-	for key, value := range fake.invocations {
-		copiedInvocations[key] = value
-	}
-	return copiedInvocations
-}
-
-func (fake *FakeTempDirManager) recordInvocation(key string, args []interface{}) {
-	fake.invocationsMutex.Lock()
-	defer fake.invocationsMutex.Unlock()
-	if fake.invocations == nil {
-		fake.invocations = map[string][][]interface{}{}
-	}
-	if fake.invocations[key] == nil {
-		fake.invocations[key] = [][]interface{}{}
-	}
-	fake.invocations[key] = append(fake.invocations[key], args)
-}
-
-var _ test.TempDirManager = new(FakeTempDirManager)