From d99f3dcd56bbb9122b48fbac6e9e1c469bf573fa Mon Sep 17 00:00:00 2001 From: Gareth Smith Date: Fri, 24 Nov 2017 17:17:41 +0000 Subject: [PATCH] Extract TempDirManager We are now returning an error instead of using an Expectation inline. --- pkg/framework/test/etcd.go | 49 ++---- pkg/framework/test/temp_dir_manager.go | 49 ++++++ pkg/framework/test/temp_dir_manager_test.go | 111 ++++++++++++++ .../test/testfakes/fake_temp_dir_manager.go | 139 ------------------ 4 files changed, 173 insertions(+), 175 deletions(-) create mode 100644 pkg/framework/test/temp_dir_manager.go create mode 100644 pkg/framework/test/temp_dir_manager_test.go delete mode 100644 pkg/framework/test/testfakes/fake_temp_dir_manager.go diff --git a/pkg/framework/test/etcd.go b/pkg/framework/test/etcd.go index 0a693619a..64075021e 100644 --- a/pkg/framework/test/etcd.go +++ b/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 -} diff --git a/pkg/framework/test/temp_dir_manager.go b/pkg/framework/test/temp_dir_manager.go new file mode 100644 index 000000000..934d980ca --- /dev/null +++ b/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 +} diff --git a/pkg/framework/test/temp_dir_manager_test.go b/pkg/framework/test/temp_dir_manager_test.go new file mode 100644 index 000000000..a070b4b5b --- /dev/null +++ b/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")) + }) + }) +}) diff --git a/pkg/framework/test/testfakes/fake_temp_dir_manager.go b/pkg/framework/test/testfakes/fake_temp_dir_manager.go deleted file mode 100644 index 0f848099f..000000000 --- a/pkg/framework/test/testfakes/fake_temp_dir_manager.go +++ /dev/null @@ -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)