From 7f6d2e8305a1387777496609cf2d6374ecba6940 Mon Sep 17 00:00:00 2001 From: Kris Date: Thu, 19 Nov 2015 10:33:27 -0800 Subject: [PATCH] Cleaning up client request unit tests 1. Closing test servers that we open. 2. Remove dependency on Config and just call NewRESTClient directly 3. Remove auth tests as they are redundant to the tests in transport_test.go --- pkg/client/unversioned/request_test.go | 102 +++++++++---------------- 1 file changed, 38 insertions(+), 64 deletions(-) diff --git a/pkg/client/unversioned/request_test.go b/pkg/client/unversioned/request_test.go index a8bafaf5868..f6a12f20e11 100644 --- a/pkg/client/unversioned/request_test.go +++ b/pkg/client/unversioned/request_test.go @@ -708,7 +708,7 @@ func TestDoRequestNewWay(t *testing.T) { } testServer := httptest.NewServer(&fakeHandler) defer testServer.Close() - c := NewOrDie(&Config{Host: testServer.URL, Version: testapi.Default.Version(), Username: "user", Password: "pass"}) + c := testRESTClient(t, testServer) obj, err := c.Verb("POST"). Prefix("foo", "bar"). Suffix("baz"). @@ -727,9 +727,6 @@ func TestDoRequestNewWay(t *testing.T) { requestURL := testapi.Default.ResourcePathWithPrefix("foo/bar", "", "", "baz") requestURL += "?timeout=1s" fakeHandler.ValidateRequest(t, requestURL, "POST", &reqBody) - if fakeHandler.RequestReceived.Header["Authorization"] == nil { - t.Errorf("Request is missing authorization header: %#v", *fakeHandler.RequestReceived) - } } func TestCheckRetryClosesBody(t *testing.T) { @@ -748,7 +745,7 @@ func TestCheckRetryClosesBody(t *testing.T) { })) defer testServer.Close() - c := NewOrDie(&Config{Host: testServer.URL, Version: testapi.Default.Version(), Username: "user", Password: "pass"}) + c := testRESTClient(t, testServer) _, err := c.Verb("POST"). Prefix("foo", "bar"). Suffix("baz"). @@ -780,7 +777,7 @@ func TestCheckRetryHandles429And5xx(t *testing.T) { })) defer testServer.Close() - c := NewOrDie(&Config{Host: testServer.URL, Version: testapi.Default.Version(), Username: "user", Password: "pass"}) + c := testRESTClient(t, testServer) _, err := c.Verb("POST"). Prefix("foo", "bar"). Suffix("baz"). @@ -796,7 +793,7 @@ func TestCheckRetryHandles429And5xx(t *testing.T) { } } -func BenchmarkCheckRetryClosesBody(t *testing.B) { +func BenchmarkCheckRetryClosesBody(b *testing.B) { count := 0 testServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, req *http.Request) { count++ @@ -809,16 +806,16 @@ func BenchmarkCheckRetryClosesBody(t *testing.B) { })) defer testServer.Close() - c := NewOrDie(&Config{Host: testServer.URL, Version: testapi.Default.Version(), Username: "user", Password: "pass"}) + c := testRESTClient(b, testServer) r := c.Verb("POST"). Prefix("foo", "bar"). Suffix("baz"). Timeout(time.Second). Body([]byte(strings.Repeat("abcd", 1000))) - for i := 0; i < t.N; i++ { + for i := 0; i < b.N; i++ { if _, err := r.DoRaw(); err != nil { - t.Fatalf("Unexpected error: %v %#v", err, err) + b.Fatalf("Unexpected error: %v %#v", err, err) } } } @@ -838,7 +835,8 @@ func TestDoRequestNewWayReader(t *testing.T) { T: t, } testServer := httptest.NewServer(&fakeHandler) - c := NewOrDie(&Config{Host: testServer.URL, Version: testapi.Default.Version(), Username: "user", Password: "pass"}) + defer testServer.Close() + c := testRESTClient(t, testServer) obj, err := c.Verb("POST"). Resource("bar"). Name("baz"). @@ -860,9 +858,6 @@ func TestDoRequestNewWayReader(t *testing.T) { requestURL := testapi.Default.ResourcePathWithPrefix("foo", "bar", "", "baz") requestURL += "?" + unversioned.LabelSelectorQueryParam(testapi.Default.Version()) + "=name%3Dfoo&timeout=1s" fakeHandler.ValidateRequest(t, requestURL, "POST", &tmpStr) - if fakeHandler.RequestReceived.Header["Authorization"] == nil { - t.Errorf("Request is missing authorization header: %#v", *fakeHandler.RequestReceived) - } } func TestDoRequestNewWayObj(t *testing.T) { @@ -880,7 +875,8 @@ func TestDoRequestNewWayObj(t *testing.T) { T: t, } testServer := httptest.NewServer(&fakeHandler) - c := NewOrDie(&Config{Host: testServer.URL, Version: testapi.Default.Version(), Username: "user", Password: "pass"}) + defer testServer.Close() + c := testRESTClient(t, testServer) obj, err := c.Verb("POST"). Suffix("baz"). Name("bar"). @@ -902,9 +898,6 @@ func TestDoRequestNewWayObj(t *testing.T) { requestURL := testapi.Default.ResourcePath("foo", "", "bar/baz") requestURL += "?" + unversioned.LabelSelectorQueryParam(testapi.Default.Version()) + "=name%3Dfoo&timeout=1s" fakeHandler.ValidateRequest(t, requestURL, "POST", &tmpStr) - if fakeHandler.RequestReceived.Header["Authorization"] == nil { - t.Errorf("Request is missing authorization header: %#v", *fakeHandler.RequestReceived) - } } func TestDoRequestNewWayFile(t *testing.T) { @@ -936,7 +929,8 @@ func TestDoRequestNewWayFile(t *testing.T) { T: t, } testServer := httptest.NewServer(&fakeHandler) - c := NewOrDie(&Config{Host: testServer.URL, Version: testapi.Default.Version(), Username: "user", Password: "pass"}) + defer testServer.Close() + c := testRESTClient(t, testServer) wasCreated := true obj, err := c.Verb("POST"). Prefix("foo/bar", "baz"). @@ -959,9 +953,6 @@ func TestDoRequestNewWayFile(t *testing.T) { requestURL := testapi.Default.ResourcePathWithPrefix("foo/bar/baz", "", "", "") requestURL += "?timeout=1s" fakeHandler.ValidateRequest(t, requestURL, "POST", &tmpStr) - if fakeHandler.RequestReceived.Header["Authorization"] == nil { - t.Errorf("Request is missing authorization header: %#v", *fakeHandler.RequestReceived) - } } func TestWasCreated(t *testing.T) { @@ -983,7 +974,8 @@ func TestWasCreated(t *testing.T) { T: t, } testServer := httptest.NewServer(&fakeHandler) - c := NewOrDie(&Config{Host: testServer.URL, Version: testapi.Default.Version(), Username: "user", Password: "pass"}) + defer testServer.Close() + c := testRESTClient(t, testServer) wasCreated := false obj, err := c.Verb("PUT"). Prefix("foo/bar", "baz"). @@ -1007,13 +999,10 @@ func TestWasCreated(t *testing.T) { requestURL := testapi.Default.ResourcePathWithPrefix("foo/bar/baz", "", "", "") requestURL += "?timeout=1s" fakeHandler.ValidateRequest(t, requestURL, "PUT", &tmpStr) - if fakeHandler.RequestReceived.Header["Authorization"] == nil { - t.Errorf("Request is missing authorization header: %#v", *fakeHandler.RequestReceived) - } } func TestVerbs(t *testing.T) { - c := NewOrDie(&Config{}) + c := testRESTClient(t, nil) if r := c.Post(); r.verb != "POST" { t.Errorf("Post verb is wrong") } @@ -1030,7 +1019,7 @@ func TestVerbs(t *testing.T) { func TestAbsPath(t *testing.T) { expectedPath := "/bar/foo" - c := NewOrDie(&Config{}) + c := testRESTClient(t, nil) r := c.Post().Prefix("/foo").AbsPath(expectedPath) if r.path != expectedPath { t.Errorf("unexpected path: %s, expected %s", r.path, expectedPath) @@ -1049,8 +1038,8 @@ func TestUintParam(t *testing.T) { } for _, item := range table { - c := NewOrDie(&Config{}) - r := c.Get().AbsPath("").UintParam(item.name, item.testVal) + u, _ := url.Parse("http://localhost") + r := NewRequest(nil, "GET", u, "test", nil).AbsPath("").UintParam(item.name, item.testVal) if e, a := item.expectStr, r.URL().String(); e != a { t.Errorf("expected %v, got %v", e, a) } @@ -1067,7 +1056,7 @@ func TestUnacceptableParamNames(t *testing.T) { } for _, item := range table { - c := NewOrDie(&Config{}) + c := testRESTClient(t, nil) r := c.Get().setParam(item.name, item.testVal) if e, a := item.expectSuccess, r.err == nil; e != a { t.Errorf("expected %v, got %v (%v)", e, a, r.err) @@ -1090,7 +1079,7 @@ func TestBody(t *testing.T) { } f.Close() - c := NewOrDie(&Config{}) + c := testRESTClient(t, nil) tests := []struct { input interface{} expected string @@ -1126,17 +1115,6 @@ func TestBody(t *testing.T) { } } -// checkAuth sets errors if the auth found in r doesn't match the expectation. -// TODO: Move to util, test in more places. -func checkAuth(t *testing.T, expectedUser, expectedPass string, r *http.Request) { - user, pass, found := r.BasicAuth() - if !found { - t.Errorf("no auth found") - } else if user != expectedUser || pass != expectedPass { - t.Fatalf("Wrong basic auth: wanted %s:%s, got %s:%s", expectedUser, expectedPass, user, pass) - } -} - func TestWatch(t *testing.T) { var table = []struct { t watch.EventType @@ -1148,7 +1126,6 @@ func TestWatch(t *testing.T) { } testServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - checkAuth(t, "user", "pass", r) flusher, ok := w.(http.Flusher) if !ok { panic("need flusher!") @@ -1166,17 +1143,9 @@ func TestWatch(t *testing.T) { flusher.Flush() } })) + defer testServer.Close() - s, err := New(&Config{ - Host: testServer.URL, - Version: testapi.Default.Version(), - Username: "user", - Password: "pass", - }) - if err != nil { - t.Fatalf("unexpected error: %v", err) - } - + s := testRESTClient(t, testServer) watching, err := s.Get().Prefix("path/to/watch/thing").Watch() if err != nil { t.Fatalf("Unexpected error") @@ -1205,7 +1174,6 @@ func TestStream(t *testing.T) { expectedBody := "expected body" testServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - checkAuth(t, "user", "pass", r) flusher, ok := w.(http.Flusher) if !ok { panic("need flusher!") @@ -1215,16 +1183,9 @@ func TestStream(t *testing.T) { w.Write([]byte(expectedBody)) flusher.Flush() })) + defer testServer.Close() - s, err := New(&Config{ - Host: testServer.URL, - Version: testapi.Default.Version(), - Username: "user", - Password: "pass", - }) - if err != nil { - t.Fatalf("unexpected error: %v", err) - } + s := testRESTClient(t, testServer) readCloser, err := s.Get().Prefix("path/to/stream/thing").Stream() if err != nil { t.Fatalf("unexpected error: %v", err) @@ -1238,3 +1199,16 @@ func TestStream(t *testing.T) { t.Errorf("Expected %s, got %s", expectedBody, resultBody) } } + +func testRESTClient(t testing.TB, srv *httptest.Server) *RESTClient { + baseURL, _ := url.Parse("http://localhost") + if srv != nil { + var err error + baseURL, err = url.Parse(srv.URL) + if err != nil { + t.Fatalf("failed to parse test URL: %v", err) + } + } + baseURL.Path = testapi.Default.ResourcePath("", "", "") + return NewRESTClient(baseURL, testapi.Default.Version(), testapi.Default.Codec(), 0, 0) +}