Refactor and dependency inject containerData deps

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

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{}
+	if handler == nil {
-	handler, err := container.NewContainerHandler(containerName)
+		return nil, fmt.Errorf("nil container handler")
-	if err != nil {
-		return nil, err
 	}
+
+	cont := &containerData{}
 	cont.handler = handler
 	ref, err := handler.ContainerReference()
 	if err != nil {

manager/container_test.go

@@ -25,54 +25,33 @@ 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(
+const containerName = "/container"
-	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)
 
-	if driver == nil {
+// Create a containerData instance for a test. Optionsl storage driver may be specified (one is made otherwise).
-		driver = &stest.MockStorageDriver{}
+func newTestContainerData(t *testing.T) (*containerData, *container.MockContainerHandler, *stest.MockStorageDriver) {
-	}
+	mockHandler := container.NewMockContainerHandler(containerName)
-
+	mockDriver := &stest.MockStorageDriver{}
-	ret, err := NewContainerData("/container", driver)
+	ret, err := newContainerData(containerName, mockDriver, mockHandler)
 	if err != nil {
 		t.Fatal(err)
 	}
-	return ret
+	return ret, mockHandler, mockDriver
 }
 
-func TestContainerUpdateSubcontainers(t *testing.T) {
+func TestUpdateSubcontainers(t *testing.T) {
-	var handler *container.MockContainerHandler
 	subcontainers := []info.ContainerReference{
 		{Name: "/container/ee0103"},
 		{Name: "/container/abcd"},
 		{Name: "/container/something"},
 	}
-	cd := createContainerDataAndSetHandler(
+	cd, mockHandler, _ := newTestContainerData(t)
-		nil,
+	mockHandler.On("ListContainers", container.LIST_SELF).Return(
-		func(h *container.MockContainerHandler) {
-			h.On("ListContainers", container.LIST_SELF).Return(
 		subcontainers,
 		nil,
 	)
-			handler = h
-		},
-		t,
-	)
 
 	err := cd.updateSubcontainers()
 	if err != nil {
@@ -95,22 +74,15 @@ func TestContainerUpdateSubcontainers(t *testing.T) {
 		}
 	}
 
-	handler.AssertExpectations(t)
+	mockHandler.AssertExpectations(t)
 }
 
-func TestContainerUpdateSubcontainersWithError(t *testing.T) {
+func TestUpdateSubcontainersWithError(t *testing.T) {
-	var handler *container.MockContainerHandler
+	cd, mockHandler, _ := newTestContainerData(t)
-	cd := createContainerDataAndSetHandler(
+	mockHandler.On("ListContainers", container.LIST_SELF).Return(
-		nil,
-		func(h *container.MockContainerHandler) {
-			h.On("ListContainers", container.LIST_SELF).Return(
 		[]info.ContainerReference{},
 		fmt.Errorf("some error"),
 	)
-			handler = h
-		},
-		t,
-	)
 
 	err := cd.updateSubcontainers()
 	if err == nil {
@@ -120,96 +92,69 @@ 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) {
+func TestUpdateStats(t *testing.T) {
-	var handler *container.MockContainerHandler
-	var ref info.ContainerReference
-
-	driver := &stest.MockStorageDriver{}
-
 	statsList := itest.GenerateRandomStats(1, 4, 1*time.Second)
 	stats := statsList[0]
 
-	cd := createContainerDataAndSetHandler(
+	cd, mockHandler, mockDriver := newTestContainerData(t)
-		driver,
+	mockHandler.On("GetStats").Return(
-		func(h *container.MockContainerHandler) {
-			h.On("GetStats").Return(
 		stats,
 		nil,
 	)
-			handler = h
-			ref.Name = h.Name
-		},
-		t,
-	)
 
-	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) {
+func TestUpdateSpec(t *testing.T) {
-	var handler *container.MockContainerHandler
 	spec := itest.GenerateRandomContainerSpec(4)
-	cd := createContainerDataAndSetHandler(
+	cd, mockHandler, _ := newTestContainerData(t)
-		nil,
+	mockHandler.On("GetSpec").Return(
-		func(h *container.MockContainerHandler) {
-			h.On("GetSpec").Return(
 		spec,
 		nil,
 	)
-			handler = h
-		},
-		t,
-	)
 
 	err := cd.updateSpec()
 	if err != nil {
 		t.Fatal(err)
 	}
 
-	handler.AssertExpectations(t)
+	mockHandler.AssertExpectations(t)
 }
 
-func TestContainerGetInfo(t *testing.T) {
+func TestGetInfo(t *testing.T) {
-	var handler *container.MockContainerHandler
 	spec := itest.GenerateRandomContainerSpec(4)
 	subcontainers := []info.ContainerReference{
 		{Name: "/container/ee0103"},
 		{Name: "/container/abcd"},
 		{Name: "/container/something"},
 	}
-	aliases := []string{"a1", "a2"}
+	cd, mockHandler, _ := newTestContainerData(t)
-	cd := createContainerDataAndSetHandler(
+	mockHandler.On("GetSpec").Return(
-		nil,
-		func(h *container.MockContainerHandler) {
-			h.On("GetSpec").Return(
 		spec,
 		nil,
 	)
-			h.On("ListContainers", container.LIST_SELF).Return(
+	mockHandler.On("ListContainers", container.LIST_SELF).Return(
 		subcontainers,
 		nil,
 	)
-			h.Aliases = aliases
+	mockHandler.Aliases = []string{"a1", "a2"}
-			handler = h
-		},
-		t,
-	)
 
 	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 {
+	if info.Name != mockHandler.Name {
-		t.Errorf("received wrong container name: received %v; should be %v", info.Name, handler.Name)
+		t.Errorf("received wrong container name: received %v; should be %v", info.Name, mockHandler.Name)
 	}
 }

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
 	}

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 {
+		for _, name := range containers {
-			ret.containers[container], err = NewContainerData(container, driver)
+			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")
+	}
+}