Merge pull request #752 from smarterclayton/remove_expect_no_error_util

Remove expectNoError from client, kubelet, and util

pkg/client/podinfo_test.go

@@ -30,19 +30,15 @@ import (
 	"github.com/fsouza/go-dockerclient"
 )
 
-// TODO: This doesn't reduce typing enough to make it worth the less readable errors. Remove.
-func expectNoError(t *testing.T, err error) {
-	if err != nil {
-		t.Errorf("Unexpected error: %#v", err)
-	}
-}
-
 func TestHTTPPodInfoGetter(t *testing.T) {
 	expectObj := api.PodInfo{
 		"myID": docker.Container{ID: "myID"},
 	}
 	body, err := json.Marshal(expectObj)
-	expectNoError(t, err)
+	if err != nil {
+		t.Errorf("unexpected error: %v", err)
+	}
+
 	fakeHandler := util.FakeHandler{
 		StatusCode:   200,
 		ResponseBody: string(body),
@@ -50,17 +46,25 @@ func TestHTTPPodInfoGetter(t *testing.T) {
 	testServer := httptest.NewServer(&fakeHandler)
 
 	hostURL, err := url.Parse(testServer.URL)
-	expectNoError(t, err)
+	if err != nil {
+		t.Errorf("unexpected error: %v", err)
+	}
+
 	parts := strings.Split(hostURL.Host, ":")
 
 	port, err := strconv.Atoi(parts[1])
-	expectNoError(t, err)
+	if err != nil {
+		t.Errorf("unexpected error: %v", err)
+	}
+
 	podInfoGetter := &HTTPPodInfoGetter{
 		Client: http.DefaultClient,
 		Port:   uint(port),
 	}
 	gotObj, err := podInfoGetter.GetPodInfo(parts[0], "foo")
-	expectNoError(t, err)
+	if err != nil {
+		t.Errorf("unexpected error: %v", err)
+	}
 
 	// reflect.DeepEqual(expectObj, gotObj) doesn't handle blank times well
 	if len(gotObj) != len(expectObj) || expectObj["myID"].ID != gotObj["myID"].ID {
@@ -73,7 +77,10 @@ func TestHTTPPodInfoGetterNotFound(t *testing.T) {
 		"myID": docker.Container{ID: "myID"},
 	}
 	_, err := json.Marshal(expectObj)
-	expectNoError(t, err)
+	if err != nil {
+		t.Errorf("unexpected error: %v", err)
+	}
+
 	fakeHandler := util.FakeHandler{
 		StatusCode:   404,
 		ResponseBody: "Pod not found",
@@ -81,11 +88,17 @@ func TestHTTPPodInfoGetterNotFound(t *testing.T) {
 	testServer := httptest.NewServer(&fakeHandler)
 
 	hostURL, err := url.Parse(testServer.URL)
-	expectNoError(t, err)
+	if err != nil {
+		t.Errorf("unexpected error: %v", err)
+	}
+
 	parts := strings.Split(hostURL.Host, ":")
 
 	port, err := strconv.Atoi(parts[1])
-	expectNoError(t, err)
+	if err != nil {
+		t.Errorf("unexpected error: %v", err)
+	}
+
 	podInfoGetter := &HTTPPodInfoGetter{
 		Client: http.DefaultClient,
 		Port:   uint(port),

pkg/client/request_test.go

@@ -153,11 +153,19 @@ func TestDoRequestNewWayObj(t *testing.T) {
 func TestDoRequestNewWayFile(t *testing.T) {
 	reqObj := &api.Pod{JSONBase: api.JSONBase{ID: "foo"}}
 	reqBodyExpected, err := api.Encode(reqObj)
-	expectNoError(t, err)
+	if err != nil {
+		t.Errorf("unexpected error: %v", err)
+	}
+
 	file, err := ioutil.TempFile("", "foo")
-	expectNoError(t, err)
+	if err != nil {
+		t.Errorf("unexpected error: %v", err)
+	}
+
 	_, err = file.Write(reqBodyExpected)
-	expectNoError(t, err)
+	if err != nil {
+		t.Errorf("unexpected error: %v", err)
+	}
 
 	expectedObj := &api.Service{Port: 12345}
 	expectedBody, _ := api.Encode(expectedObj)

pkg/kubelet/kubelet_test.go

@@ -32,25 +32,6 @@ import (
 	"github.com/stretchr/testify/mock"
 )
 
-// TODO: This doesn't reduce typing enough to make it worth the less readable errors. Remove.
-func expectNoError(t *testing.T, err error) {
-	if err != nil {
-		t.Errorf("Unexpected error: %#v", err)
-	}
-}
-
-func verifyNoError(t *testing.T, e error) {
-	if e != nil {
-		t.Errorf("Expected no error, found %#v", e)
-	}
-}
-
-func verifyError(t *testing.T, e error) {
-	if e == nil {
-		t.Errorf("Expected error, found nil")
-	}
-}
-
 func makeTestKubelet(t *testing.T) (*Kubelet, *tools.FakeEtcdClient, *FakeDockerClient) {
 	fakeEtcdClient := tools.MakeFakeEtcdClient(t)
 	fakeDocker := &FakeDockerClient{
@@ -166,7 +147,9 @@ func TestKillContainerWithError(t *testing.T) {
 	kubelet, _, _ := makeTestKubelet(t)
 	kubelet.dockerClient = fakeDocker
 	err := kubelet.killContainer(fakeDocker.containerList[0])
-	verifyError(t, err)
+	if err == nil {
+		t.Errorf("expected error, found nil")
+	}
 	verifyCalls(t, fakeDocker, []string{"stop"})
 }
 
@@ -187,7 +170,9 @@ func TestKillContainer(t *testing.T) {
 	}
 
 	err := kubelet.killContainer(fakeDocker.containerList[0])
-	verifyNoError(t, err)
+	if err != nil {
+		t.Errorf("unexpected error: %v", err)
+	}
 	verifyCalls(t, fakeDocker, []string{"stop"})
 }
 
@@ -246,7 +231,10 @@ func TestSyncPodsDoesNothing(t *testing.T) {
 			},
 		},
 	})
-	expectNoError(t, err)
+	if err != nil {
+		t.Errorf("unexpected error: %v", err)
+	}
+
 	verifyCalls(t, fakeDocker, []string{"list", "list"})
 }
 
@@ -269,7 +257,10 @@ func TestSyncPodsDeletes(t *testing.T) {
 		},
 	}
 	err := kubelet.SyncPods([]Pod{})
-	expectNoError(t, err)
+	if err != nil {
+		t.Errorf("unexpected error: %v", err)
+	}
+
 	verifyCalls(t, fakeDocker, []string{"list", "list", "stop", "stop"})
 
 	// A map iteration is used to delete containers, so must not depend on
@@ -319,7 +310,10 @@ func TestSyncPodDeletesDuplicate(t *testing.T) {
 			},
 		},
 	}, dockerContainers)
-	expectNoError(t, err)
+	if err != nil {
+		t.Errorf("unexpected error: %v", err)
+	}
+
 	verifyCalls(t, fakeDocker, []string{"list", "stop"})
 
 	// Expect one of the duplicates to be killed.
@@ -364,7 +358,10 @@ func TestSyncPodUnhealthy(t *testing.T) {
 			},
 		},
 	}, dockerContainers)
-	expectNoError(t, err)
+	if err != nil {
+		t.Errorf("unexpected error: %v", err)
+	}
+
 	verifyCalls(t, fakeDocker, []string{"list", "stop", "create", "start"})
 
 	// A map interation is used to delete containers, so must not depend on
@@ -387,15 +384,24 @@ func TestEventWriting(t *testing.T) {
 		},
 	}
 	err := kubelet.LogEvent(&expectedEvent)
-	expectNoError(t, err)
+	if err != nil {
+		t.Errorf("unexpected error: %v", err)
+	}
+
 	if fakeEtcd.Ix != 1 {
 		t.Errorf("Unexpected number of children added: %d, expected 1", fakeEtcd.Ix)
 	}
 	response, err := fakeEtcd.Get("/events/foo/1", false, false)
-	expectNoError(t, err)
+	if err != nil {
+		t.Errorf("unexpected error: %v", err)
+	}
+
 	var event api.Event
 	err = json.Unmarshal([]byte(response.Node.Value), &event)
-	expectNoError(t, err)
+	if err != nil {
+		t.Errorf("unexpected error: %v", err)
+	}
+
 	if event.Event != expectedEvent.Event ||
 		event.Container.Name != expectedEvent.Container.Name {
 		t.Errorf("Event's don't match.  Expected: %#v Saw: %#v", expectedEvent, event)

pkg/util/fake_handler_test.go

@@ -23,12 +23,6 @@ import (
 	"testing"
 )
 
-func expectNoError(t *testing.T, err error) {
-	if err != nil {
-		t.Errorf("Unexpected error: %#v", err)
-	}
-}
-
 func TestFakeHandlerPath(t *testing.T) {
 	handler := FakeHandler{}
 	server := httptest.NewServer(&handler)
@@ -37,10 +31,15 @@ func TestFakeHandlerPath(t *testing.T) {
 	body := "somebody"
 
 	req, err := http.NewRequest(method, server.URL+path, bytes.NewBufferString(body))
-	expectNoError(t, err)
+	if err != nil {
+		t.Errorf("unexpected error: %v", err)
+	}
+
 	client := http.Client{}
 	_, err = client.Do(req)
-	expectNoError(t, err)
+	if err != nil {
+		t.Errorf("unexpected error: %v", err)
+	}
 
 	handler.ValidateRequest(t, path, method, &body)
 }
@@ -52,10 +51,15 @@ func TestFakeHandlerPathNoBody(t *testing.T) {
 	path := "/foo/bar"
 
 	req, err := http.NewRequest(method, server.URL+path, nil)
-	expectNoError(t, err)
+	if err != nil {
+		t.Errorf("unexpected error: %v", err)
+	}
+
 	client := http.Client{}
 	_, err = client.Do(req)
-	expectNoError(t, err)
+	if err != nil {
+		t.Errorf("unexpected error: %v", err)
+	}
 
 	handler.ValidateRequest(t, path, method, nil)
 }
@@ -76,10 +80,15 @@ func TestFakeHandlerWrongPath(t *testing.T) {
 	fakeT := fakeError{}
 
 	req, err := http.NewRequest(method, server.URL+"/foo/baz", nil)
-	expectNoError(t, err)
+	if err != nil {
+		t.Errorf("unexpected error: %v", err)
+	}
+
 	client := http.Client{}
 	_, err = client.Do(req)
-	expectNoError(t, err)
+	if err != nil {
+		t.Errorf("unexpected error: %v", err)
+	}
 
 	handler.ValidateRequest(&fakeT, path, method, nil)
 	if len(fakeT.errors) != 1 {
@@ -95,10 +104,15 @@ func TestFakeHandlerWrongMethod(t *testing.T) {
 	fakeT := fakeError{}
 
 	req, err := http.NewRequest("PUT", server.URL+path, nil)
-	expectNoError(t, err)
+	if err != nil {
+		t.Errorf("unexpected error: %v", err)
+	}
+
 	client := http.Client{}
 	_, err = client.Do(req)
-	expectNoError(t, err)
+	if err != nil {
+		t.Errorf("unexpected error: %v", err)
+	}
 
 	handler.ValidateRequest(&fakeT, path, method, nil)
 	if len(fakeT.errors) != 1 {
@@ -115,10 +129,15 @@ func TestFakeHandlerWrongBody(t *testing.T) {
 	fakeT := fakeError{}
 
 	req, err := http.NewRequest(method, server.URL+path, bytes.NewBufferString(body))
-	expectNoError(t, err)
+	if err != nil {
+		t.Errorf("unexpected error: %v", err)
+	}
+
 	client := http.Client{}
 	_, err = client.Do(req)
-	expectNoError(t, err)
+	if err != nil {
+		t.Errorf("unexpected error: %v", err)
+	}
 
 	otherbody := "otherbody"
 	handler.ValidateRequest(&fakeT, path, method, &otherbody)
@@ -136,10 +155,15 @@ func TestFakeHandlerNilBody(t *testing.T) {
 	fakeT := fakeError{}
 
 	req, err := http.NewRequest(method, server.URL+path, nil)
-	expectNoError(t, err)
+	if err != nil {
+		t.Errorf("unexpected error: %v", err)
+	}
+
 	client := http.Client{}
 	_, err = client.Do(req)
-	expectNoError(t, err)
+	if err != nil {
+		t.Errorf("unexpected error: %v", err)
+	}
 
 	handler.ValidateRequest(&fakeT, path, method, &body)
 	if len(fakeT.errors) != 1 {

pkg/util/util_test.go

@@ -47,7 +47,10 @@ func TestMakeJSONString(t *testing.T) {
 	body := MakeJSONString(pod)
 
 	expectedBody, err := json.Marshal(pod)
-	expectNoError(t, err)
+	if err != nil {
+		t.Errorf("unexpected error: %v", err)
+	}
+
 	if string(expectedBody) != body {
 		t.Errorf("JSON doesn't match.  Expected %s, saw %s", expectedBody, body)
 	}