From e759059a092e67f069900997b1572fa9b4b298ed Mon Sep 17 00:00:00 2001 From: Victor Marmol Date: Mon, 22 Sep 2014 10:20:54 -0700 Subject: [PATCH] Refactor and dependency inject containerData deps --- container/mock.go | 6 ++ manager/container.go | 10 +-- manager/container_test.go | 139 ++++++++++++-------------------------- manager/manager.go | 6 +- manager/manager_test.go | 40 +++++------ 5 files changed, 79 insertions(+), 122 deletions(-) diff --git a/container/mock.go b/container/mock.go index 28b7fcea..208d0d24 100644 --- a/container/mock.go +++ b/container/mock.go @@ -26,6 +26,12 @@ type MockContainerHandler struct { Aliases []string } +func NewMockContainerHandler(containerName string) *MockContainerHandler { + return &MockContainerHandler{ + Name: containerName, + } +} + // If self.Name is not empty, then ContainerReference() will return self.Name and self.Aliases. // Otherwise, it will use the value provided by .On().Return(). func (self *MockContainerHandler) ContainerReference() (info.ContainerReference, error) { diff --git a/manager/container.go b/manager/container.go index d6e7e97d..81b3552b 100644 --- a/manager/container.go +++ b/manager/container.go @@ -84,15 +84,15 @@ func (c *containerData) GetInfo() (*containerInfo, error) { return &ret, nil } -func NewContainerData(containerName string, driver storage.StorageDriver) (*containerData, error) { +func newContainerData(containerName string, driver storage.StorageDriver, handler container.ContainerHandler) (*containerData, error) { if driver == nil { return nil, fmt.Errorf("nil storage driver") } - cont := &containerData{} - handler, err := container.NewContainerHandler(containerName) - if err != nil { - return nil, err + if handler == nil { + return nil, fmt.Errorf("nil container handler") } + + cont := &containerData{} cont.handler = handler ref, err := handler.ContainerReference() if err != nil { diff --git a/manager/container_test.go b/manager/container_test.go index 5910ed3b..cf0e654c 100644 --- a/manager/container_test.go +++ b/manager/container_test.go @@ -25,53 +25,32 @@ import ( "github.com/google/cadvisor/container" "github.com/google/cadvisor/info" itest "github.com/google/cadvisor/info/test" - "github.com/google/cadvisor/storage" stest "github.com/google/cadvisor/storage/test" ) -func createContainerDataAndSetHandler( - driver storage.StorageDriver, - f func(*container.MockContainerHandler), - t *testing.T, -) *containerData { - factory := &container.FactoryForMockContainerHandler{ - Name: "factoryForMockContainer", - PrepareContainerHandlerFunc: func(name string, handler *container.MockContainerHandler) { - handler.Name = name - f(handler) - }, - } - container.ClearContainerHandlerFactories() - container.RegisterContainerHandlerFactory(factory) +const containerName = "/container" - if driver == nil { - driver = &stest.MockStorageDriver{} - } - - ret, err := NewContainerData("/container", driver) +// Create a containerData instance for a test. Optionsl storage driver may be specified (one is made otherwise). +func newTestContainerData(t *testing.T) (*containerData, *container.MockContainerHandler, *stest.MockStorageDriver) { + mockHandler := container.NewMockContainerHandler(containerName) + mockDriver := &stest.MockStorageDriver{} + ret, err := newContainerData(containerName, mockDriver, mockHandler) if err != nil { t.Fatal(err) } - return ret + return ret, mockHandler, mockDriver } -func TestContainerUpdateSubcontainers(t *testing.T) { - var handler *container.MockContainerHandler +func TestUpdateSubcontainers(t *testing.T) { subcontainers := []info.ContainerReference{ {Name: "/container/ee0103"}, {Name: "/container/abcd"}, {Name: "/container/something"}, } - cd := createContainerDataAndSetHandler( + cd, mockHandler, _ := newTestContainerData(t) + mockHandler.On("ListContainers", container.LIST_SELF).Return( + subcontainers, nil, - func(h *container.MockContainerHandler) { - h.On("ListContainers", container.LIST_SELF).Return( - subcontainers, - nil, - ) - handler = h - }, - t, ) err := cd.updateSubcontainers() @@ -95,21 +74,14 @@ func TestContainerUpdateSubcontainers(t *testing.T) { } } - handler.AssertExpectations(t) + mockHandler.AssertExpectations(t) } -func TestContainerUpdateSubcontainersWithError(t *testing.T) { - var handler *container.MockContainerHandler - cd := createContainerDataAndSetHandler( - nil, - func(h *container.MockContainerHandler) { - h.On("ListContainers", container.LIST_SELF).Return( - []info.ContainerReference{}, - fmt.Errorf("some error"), - ) - handler = h - }, - t, +func TestUpdateSubcontainersWithError(t *testing.T) { + cd, mockHandler, _ := newTestContainerData(t) + mockHandler.On("ListContainers", container.LIST_SELF).Return( + []info.ContainerReference{}, + fmt.Errorf("some error"), ) err := cd.updateSubcontainers() @@ -120,54 +92,35 @@ func TestContainerUpdateSubcontainersWithError(t *testing.T) { t.Errorf("Received %v subcontainers, should be 0", len(cd.info.Subcontainers)) } - handler.AssertExpectations(t) + mockHandler.AssertExpectations(t) } -func TestContainerUpdateStats(t *testing.T) { - var handler *container.MockContainerHandler - var ref info.ContainerReference - - driver := &stest.MockStorageDriver{} - +func TestUpdateStats(t *testing.T) { statsList := itest.GenerateRandomStats(1, 4, 1*time.Second) stats := statsList[0] - cd := createContainerDataAndSetHandler( - driver, - func(h *container.MockContainerHandler) { - h.On("GetStats").Return( - stats, - nil, - ) - handler = h - ref.Name = h.Name - }, - t, + cd, mockHandler, mockDriver := newTestContainerData(t) + mockHandler.On("GetStats").Return( + stats, + nil, ) - driver.On("AddStats", ref, stats).Return(nil) + mockDriver.On("AddStats", info.ContainerReference{Name: mockHandler.Name}, stats).Return(nil) err := cd.updateStats() if err != nil { t.Fatal(err) } - handler.AssertExpectations(t) + mockHandler.AssertExpectations(t) } -func TestContainerUpdateSpec(t *testing.T) { - var handler *container.MockContainerHandler +func TestUpdateSpec(t *testing.T) { spec := itest.GenerateRandomContainerSpec(4) - cd := createContainerDataAndSetHandler( + cd, mockHandler, _ := newTestContainerData(t) + mockHandler.On("GetSpec").Return( + spec, nil, - func(h *container.MockContainerHandler) { - h.On("GetSpec").Return( - spec, - nil, - ) - handler = h - }, - t, ) err := cd.updateSpec() @@ -175,41 +128,33 @@ func TestContainerUpdateSpec(t *testing.T) { t.Fatal(err) } - handler.AssertExpectations(t) + mockHandler.AssertExpectations(t) } -func TestContainerGetInfo(t *testing.T) { - var handler *container.MockContainerHandler +func TestGetInfo(t *testing.T) { spec := itest.GenerateRandomContainerSpec(4) subcontainers := []info.ContainerReference{ {Name: "/container/ee0103"}, {Name: "/container/abcd"}, {Name: "/container/something"}, } - aliases := []string{"a1", "a2"} - cd := createContainerDataAndSetHandler( + cd, mockHandler, _ := newTestContainerData(t) + mockHandler.On("GetSpec").Return( + spec, nil, - func(h *container.MockContainerHandler) { - h.On("GetSpec").Return( - spec, - nil, - ) - h.On("ListContainers", container.LIST_SELF).Return( - subcontainers, - nil, - ) - h.Aliases = aliases - handler = h - }, - t, ) + mockHandler.On("ListContainers", container.LIST_SELF).Return( + subcontainers, + nil, + ) + mockHandler.Aliases = []string{"a1", "a2"} info, err := cd.GetInfo() if err != nil { t.Fatal(err) } - handler.AssertExpectations(t) + mockHandler.AssertExpectations(t) if len(info.Subcontainers) != len(subcontainers) { t.Errorf("Received %v subcontainers, should be %v", len(info.Subcontainers), len(subcontainers)) @@ -231,7 +176,7 @@ func TestContainerGetInfo(t *testing.T) { t.Errorf("received wrong container spec") } - if info.Name != handler.Name { - t.Errorf("received wrong container name: received %v; should be %v", info.Name, handler.Name) + if info.Name != mockHandler.Name { + t.Errorf("received wrong container name: received %v; should be %v", info.Name, mockHandler.Name) } } diff --git a/manager/manager.go b/manager/manager.go index 3817a770..1b84f0dd 100644 --- a/manager/manager.go +++ b/manager/manager.go @@ -222,7 +222,11 @@ func (m *manager) GetVersionInfo() (*info.VersionInfo, error) { // Create a container. func (m *manager) createContainer(containerName string) error { - cont, err := NewContainerData(containerName, m.storageDriver) + handler, err := container.NewContainerHandler(containerName) + if err != nil { + return err + } + cont, err := newContainerData(containerName, m.storageDriver, handler) if err != nil { return err } diff --git a/manager/manager_test.go b/manager/manager_test.go index 033f4045..37b35a84 100644 --- a/manager/manager_test.go +++ b/manager/manager_test.go @@ -36,34 +36,19 @@ func createManagerAndAddContainers( if driver == nil { driver = &stest.MockStorageDriver{} } - factory := &container.FactoryForMockContainerHandler{ - Name: "factoryForManager", - PrepareContainerHandlerFunc: func(name string, handler *container.MockContainerHandler) { - handler.Name = name - found := false - for _, c := range containers { - if c == name { - found = true - } - } - if !found { - t.Errorf("Asked to create a container with name %v, which is unknown.", name) - } - f(handler) - }, - } container.ClearContainerHandlerFactories() - container.RegisterContainerHandlerFactory(factory) mif, err := New(driver) if err != nil { t.Fatal(err) } if ret, ok := mif.(*manager); ok { - for _, container := range containers { - ret.containers[container], err = NewContainerData(container, driver) + for _, name := range containers { + mockHandler := container.NewMockContainerHandler(name) + ret.containers[name], err = newContainerData(name, driver, mockHandler) if err != nil { t.Fatal(err) } + f(mockHandler) } return ret } @@ -179,3 +164,20 @@ func TestSubcontainersInfo(t *testing.T) { } } } + +func TestNew(t *testing.T) { + manager, err := New(&stest.MockStorageDriver{}) + if err != nil { + t.Fatalf("Expected manager.New to succeed: %s", err) + } + if manager == nil { + t.Fatalf("Expected returned manager to not be nil") + } +} + +func TestNewNilManager(t *testing.T) { + _, err := New(nil) + if err == nil { + t.Fatalf("Expected nil manager to return error") + } +}