From f7149926c00e2ae8c23a2f41e68f05b9cb4e2d4c Mon Sep 17 00:00:00 2001 From: Clayton Coleman Date: Sun, 3 Aug 2014 19:59:47 -0400 Subject: [PATCH] Remove expectNoError from client, kubelet, and util --- pkg/client/podinfo_test.go | 41 +++++++++++++++-------- pkg/client/request_test.go | 14 ++++++-- pkg/kubelet/kubelet_test.go | 62 +++++++++++++++++++---------------- pkg/util/fake_handler_test.go | 60 +++++++++++++++++++++++---------- pkg/util/util_test.go | 5 ++- 5 files changed, 118 insertions(+), 64 deletions(-) diff --git a/pkg/client/podinfo_test.go b/pkg/client/podinfo_test.go index 620aced9cde..6d20cddcfd3 100644 --- a/pkg/client/podinfo_test.go +++ b/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), diff --git a/pkg/client/request_test.go b/pkg/client/request_test.go index c4c0f6779d3..613be01fb11 100644 --- a/pkg/client/request_test.go +++ b/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) diff --git a/pkg/kubelet/kubelet_test.go b/pkg/kubelet/kubelet_test.go index 1d8def44fd4..a6ea820f1ae 100644 --- a/pkg/kubelet/kubelet_test.go +++ b/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) diff --git a/pkg/util/fake_handler_test.go b/pkg/util/fake_handler_test.go index cfa7cd48ee0..2a45474bf81 100644 --- a/pkg/util/fake_handler_test.go +++ b/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 { diff --git a/pkg/util/util_test.go b/pkg/util/util_test.go index 44b264d36c4..8e4171b8edc 100644 --- a/pkg/util/util_test.go +++ b/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) }