Add test coverage and fix a few minor problems

cmd/integration/integration.go

@@ -118,7 +118,7 @@ func startComponents(manifestURL string) (apiServerURL string) {
 	}
 
 	cl := client.NewOrDie(&client.Config{Host: apiServer.URL, Version: testapi.Version()})
-	cl.PollPeriod = time.Second * 1
+	cl.PollPeriod = time.Millisecond * 100
 	cl.Sync = true
 
 	helper, err := master.NewEtcdHelper(etcdClient, "")
@@ -292,7 +292,6 @@ func runAtomicPutTest(c *client.Client) {
 				err := c.Get().
 					Path("services").
 					Path(svc.Name).
-					PollPeriod(100 * time.Millisecond).
 					Do().
 					Into(&tmpSvc)
 				if err != nil {

pkg/api/types.go

@@ -603,7 +603,7 @@ type Status struct {
 type StatusDetails struct {
 	// The ID attribute of the resource associated with the status StatusReason
 	// (when there is a single ID which can be described).
-	// TODO: replace with Name
+	// TODO: replace with Name with v1beta3
 	ID string `json:"id,omitempty" yaml:"id,omitempty"`
 	// The kind attribute of the resource associated with the status StatusReason.
 	// On some operations may differ from the requested resource Kind.

pkg/client/flags_test.go

+/*
+Copyright 2014 Google Inc. All rights reserved.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package client
+
+import (
+	"testing"
+
+	"github.com/GoogleCloudPlatform/kubernetes/pkg/util"
+)
+
+type fakeFlagSet struct {
+	t   *testing.T
+	set util.StringSet
+}
+
+func (f *fakeFlagSet) StringVar(p *string, name, value, usage string) {
+	if p == nil {
+		f.t.Errorf("unexpected nil pointer")
+	}
+	if usage == "" {
+		f.t.Errorf("unexpected empty usage")
+	}
+	f.set.Insert(name)
+}
+
+func (f *fakeFlagSet) BoolVar(p *bool, name string, value bool, usage string) {
+	if p == nil {
+		f.t.Errorf("unexpected nil pointer")
+	}
+	if usage == "" {
+		f.t.Errorf("unexpected empty usage")
+	}
+	f.set.Insert(name)
+}
+
+func (f *fakeFlagSet) UintVar(p *uint, name string, value uint, usage string) {
+	if p == nil {
+		f.t.Errorf("unexpected nil pointer")
+	}
+	if usage == "" {
+		f.t.Errorf("unexpected empty usage")
+	}
+	f.set.Insert(name)
+}
+
+func TestBindClientConfigFlags(t *testing.T) {
+	flags := &fakeFlagSet{t, util.StringSet{}}
+	config := &Config{}
+	BindClientConfigFlags(flags, config)
+	if len(flags.set) != 6 {
+		t.Errorf("unexpected flag set: %#v", flags)
+	}
+}
+
+func TestBindKubeletClientConfigFlags(t *testing.T) {
+	flags := &fakeFlagSet{t, util.StringSet{}}
+	config := &KubeletConfig{}
+	BindKubeletClientConfigFlags(flags, config)
+	if len(flags.set) != 5 {
+		t.Errorf("unexpected flag set: %#v", flags)
+	}
+}

pkg/client/request.go

@@ -109,6 +109,9 @@ func (r *Request) Sync(sync bool) *Request {
 
 // Namespace applies the namespace scope to a request
 func (r *Request) Namespace(namespace string) *Request {
+	if r.err != nil {
+		return r
+	}
 	if len(namespace) > 0 {
 		return r.setParam("namespace", namespace)
 	}
@@ -225,8 +228,9 @@ func (r *Request) NoPoll() *Request {
 	return r.Poller(nil)
 }
 
-// Poller indicates this request should use the specify poll function to determine whether
-// a server "working" response should be retried.
+// Poller indicates this request should use the specified poll function to determine whether
+// a server "working" response should be retried. The poller is responsible for waiting or
+// outputting messages to the client.
 func (r *Request) Poller(poller PollFunc) *Request {
 	if r.err != nil {
 		return r

pkg/client/request_test.go

@@ -19,10 +19,12 @@ package client
 import (
 	"bytes"
 	"encoding/base64"
+	"errors"
 	"io/ioutil"
 	"net/http"
 	"net/http/httptest"
 	"net/url"
+	"os"
 	"reflect"
 	"strings"
 	"testing"
@@ -40,6 +42,83 @@ import (
 	watchjson "github.com/GoogleCloudPlatform/kubernetes/pkg/watch/json"
 )
 
+func skipPolling(name string) (*Request, bool) {
+	return nil, false
+}
+
+func TestRequestWithErrorWontChange(t *testing.T) {
+	original := Request{err: errors.New("test")}
+	r := original
+	changed := r.Param("foo", "bar").
+		SelectorParam("labels", labels.Set{"a": "b"}.AsSelector()).
+		UintParam("uint", 1).
+		AbsPath("/abs").
+		Path("test").
+		ParseSelectorParam("foo", "a=b").
+		Namespace("new").
+		NoPoll().
+		Body("foo").
+		Poller(skipPolling).
+		Timeout(time.Millisecond).
+		Sync(true)
+	if changed != &r {
+		t.Errorf("returned request should point to the same object")
+	}
+	if !reflect.DeepEqual(&original, changed) {
+		t.Errorf("expected %#v, got %#v", &original, changed)
+	}
+}
+
+func TestRequestParseSelectorParam(t *testing.T) {
+	r := (&Request{}).ParseSelectorParam("foo", "a")
+	if r.err == nil || r.params != nil {
+		t.Errorf("should have set err and left params nil: %#v", r)
+	}
+}
+
+func TestRequestParam(t *testing.T) {
+	r := (&Request{}).Param("foo", "a")
+	if !reflect.DeepEqual(map[string]string{"foo": "a"}, r.params) {
+		t.Errorf("should have set a param: %#v", r)
+	}
+}
+
+type NotAnAPIObject struct{}
+
+func (NotAnAPIObject) IsAnAPIObject() {}
+
+func TestRequestBody(t *testing.T) {
+	// test unknown type
+	r := (&Request{}).Body([]string{"test"})
+	if r.err == nil || r.body != nil {
+		t.Errorf("should have set err and left body nil: %#v", r)
+	}
+
+	// test error set when failing to read file
+	f, err := ioutil.TempFile("", "test")
+	if err != nil {
+		t.Fatalf("unable to create temp file")
+	}
+	os.Remove(f.Name())
+	r = (&Request{}).Body(f.Name())
+	if r.err == nil || r.body != nil {
+		t.Errorf("should have set err and left body nil: %#v", r)
+	}
+
+	// test unencodable api object
+	r = (&Request{codec: latest.Codec}).Body(&NotAnAPIObject{})
+	if r.err == nil || r.body != nil {
+		t.Errorf("should have set err and left body nil: %#v", r)
+	}
+}
+
+func TestResultIntoWithErrReturnsErr(t *testing.T) {
+	res := Result{err: errors.New("test")}
+	if err := res.Into(&api.Pod{}); err != res.err {
+		t.Errorf("should have returned exact error from result")
+	}
+}
+
 func TestTransformResponse(t *testing.T) {
 	invalid := []byte("aaaaa")
 	uri, _ := url.Parse("http://localhost")
@@ -75,6 +154,126 @@ func TestTransformResponse(t *testing.T) {
 	}
 }
 
+type clientFunc func(req *http.Request) (*http.Response, error)
+
+func (f clientFunc) Do(req *http.Request) (*http.Response, error) {
+	return f(req)
+}
+
+func TestRequestWatch(t *testing.T) {
+	testCases := []struct {
+		Request *Request
+		Err     bool
+	}{
+		{
+			Request: &Request{err: errors.New("bail")},
+			Err:     true,
+		},
+		{
+			Request: &Request{baseURL: &url.URL{}, path: "%"},
+			Err:     true,
+		},
+		{
+			Request: &Request{
+				client: clientFunc(func(req *http.Request) (*http.Response, error) {
+					return nil, errors.New("err")
+				}),
+				baseURL: &url.URL{},
+			},
+			Err: true,
+		},
+		{
+			Request: &Request{
+				client: clientFunc(func(req *http.Request) (*http.Response, error) {
+					return &http.Response{StatusCode: http.StatusForbidden}, nil
+				}),
+				baseURL: &url.URL{},
+			},
+			Err: true,
+		},
+	}
+	for i, testCase := range testCases {
+		watch, err := testCase.Request.Watch()
+		hasErr := err != nil
+		if hasErr != testCase.Err {
+			t.Errorf("%d: expected %f, got %f: %v", i, testCase.Err, hasErr, err)
+		}
+		if hasErr && watch != nil {
+			t.Errorf("%d: watch should be nil when error is returned", i)
+		}
+	}
+}
+
+func TestRequestStream(t *testing.T) {
+	testCases := []struct {
+		Request *Request
+		Err     bool
+	}{
+		{
+			Request: &Request{err: errors.New("bail")},
+			Err:     true,
+		},
+		{
+			Request: &Request{baseURL: &url.URL{}, path: "%"},
+			Err:     true,
+		},
+		{
+			Request: &Request{
+				client: clientFunc(func(req *http.Request) (*http.Response, error) {
+					return nil, errors.New("err")
+				}),
+				baseURL: &url.URL{},
+			},
+			Err: true,
+		},
+	}
+	for i, testCase := range testCases {
+		body, err := testCase.Request.Stream()
+		hasErr := err != nil
+		if hasErr != testCase.Err {
+			t.Errorf("%d: expected %f, got %f: %v", i, testCase.Err, hasErr, err)
+		}
+		if hasErr && body != nil {
+			t.Errorf("%d: body should be nil when error is returned", i)
+		}
+	}
+}
+
+func TestRequestDo(t *testing.T) {
+	testCases := []struct {
+		Request *Request
+		Err     bool
+	}{
+		{
+			Request: &Request{err: errors.New("bail")},
+			Err:     true,
+		},
+		{
+			Request: &Request{baseURL: &url.URL{}, path: "%"},
+			Err:     true,
+		},
+		{
+			Request: &Request{
+				client: clientFunc(func(req *http.Request) (*http.Response, error) {
+					return nil, errors.New("err")
+				}),
+				baseURL: &url.URL{},
+			},
+			Err: true,
+		},
+	}
+	for i, testCase := range testCases {
+		body, err := testCase.Request.Do().Raw()
+		hasErr := err != nil
+		if hasErr != testCase.Err {
+			t.Errorf("%d: expected %f, got %f: %v", i, testCase.Err, hasErr, err)
+		}
+		if hasErr && body != nil {
+			t.Errorf("%d: body should be nil when error is returned", i)
+		}
+	}
+}
+
 func TestDoRequestNewWay(t *testing.T) {
 	reqBody := "request body"
 	expectedObj := &api.Service{Port: 12345}

pkg/client/restclient_test.go

@@ -242,3 +242,10 @@ func TestDoRequestCreated(t *testing.T) {
 	}
 	fakeHandler.ValidateRequest(t, "/"+testapi.Version()+"/test", "GET", nil)
 }
+
+func TestDefaultPoll(t *testing.T) {
+	c := &RESTClient{PollPeriod: 0}
+	if req, ok := c.DefaultPoll("test"); req != nil || ok {
+		t.Errorf("expected nil request and not poll")
+	}
+}