From 87fa19cdfecac467ac784617a63141d330cfec09 Mon Sep 17 00:00:00 2001 From: Kelsey Hightower Date: Sun, 27 Jul 2014 06:30:32 -0700 Subject: [PATCH] Remove extra test flags from all commands Currently all commands are being build with extra flags. The extra flags appear because of a direct import of the testing package from the fake_etcd_client.go source file. Remove the direct import of the testing package. Add a tools.T interface to support existing behavior. Also clean up two TODO items by remove using of the expectError and expectNoError functions. Fixes #579 --- pkg/kubelet/config/config_test.go | 14 ---------- pkg/kubelet/config/etcd_test.go | 20 ++++++++++---- pkg/kubelet/config/file_test.go | 45 ++++++++++++++++++++++--------- pkg/kubelet/config/http_test.go | 16 ++++++++--- pkg/tools/fake_etcd_client.go | 11 +++++--- 5 files changed, 68 insertions(+), 38 deletions(-) diff --git a/pkg/kubelet/config/config_test.go b/pkg/kubelet/config/config_test.go index 338b77bcfc2..52c1faf0eb7 100644 --- a/pkg/kubelet/config/config_test.go +++ b/pkg/kubelet/config/config_test.go @@ -24,20 +24,6 @@ import ( "github.com/GoogleCloudPlatform/kubernetes/pkg/kubelet" ) -// TODO: remove this -func expectError(t *testing.T, err error) { - if err == nil { - t.Errorf("Expected error, Got %v", err) - } -} - -// TODO: remove this -func expectNoError(t *testing.T, err error) { - if err != nil { - t.Errorf("Expected no error, Got %v", err) - } -} - func expectEmptyChannel(t *testing.T, ch <-chan interface{}) { select { case update := <-ch: diff --git a/pkg/kubelet/config/etcd_test.go b/pkg/kubelet/config/etcd_test.go index e71ff32f0a8..dbf7039a2b6 100644 --- a/pkg/kubelet/config/etcd_test.go +++ b/pkg/kubelet/config/etcd_test.go @@ -64,7 +64,9 @@ func TestGetEtcdNoData(t *testing.T) { } c := SourceEtcd{"/registry/hosts/machine/kubelet", fakeClient, ch, time.Millisecond} _, err := c.fetchNextState(0) - expectError(t, err) + if err == nil { + t.Errorf("Expected error") + } expectEmptyChannel(t, ch) } @@ -82,7 +84,9 @@ func TestGetEtcd(t *testing.T) { } c := SourceEtcd{"/registry/hosts/machine/kubelet", fakeClient, ch, time.Millisecond} lastIndex, err := c.fetchNextState(0) - expectNoError(t, err) + if err != nil { + t.Errorf("Unexpected error: %v", err) + } if lastIndex != 2 { t.Errorf("Expected %#v, Got %#v", 2, lastIndex) } @@ -107,7 +111,9 @@ func TestWatchEtcd(t *testing.T) { } c := SourceEtcd{"/registry/hosts/machine/kubelet", fakeClient, ch, time.Millisecond} lastIndex, err := c.fetchNextState(1) - expectNoError(t, err) + if err != nil { + t.Errorf("Unexpected error: %v", err) + } if lastIndex != 3 { t.Errorf("Expected %d, Got %d", 3, lastIndex) } @@ -127,7 +133,9 @@ func TestGetEtcdNotFound(t *testing.T) { } c := SourceEtcd{"/registry/hosts/machine/kubelet", fakeClient, ch, time.Millisecond} _, err := c.fetchNextState(0) - expectError(t, err) + if err == nil { + t.Errorf("Expected error") + } expectEmptyChannel(t, ch) } @@ -142,6 +150,8 @@ func TestGetEtcdError(t *testing.T) { } c := SourceEtcd{"/registry/hosts/machine/kubelet", fakeClient, ch, time.Millisecond} _, err := c.fetchNextState(0) - expectError(t, err) + if err == nil { + t.Errorf("Expected error") + } expectEmptyChannel(t, ch) } diff --git a/pkg/kubelet/config/file_test.go b/pkg/kubelet/config/file_test.go index 6909eca5efb..f505c3b0c84 100644 --- a/pkg/kubelet/config/file_test.go +++ b/pkg/kubelet/config/file_test.go @@ -34,7 +34,9 @@ func TestExtractFromNonExistentFile(t *testing.T) { ch := make(chan interface{}, 1) c := SourceFile{"/some/fake/file", ch} err := c.extractFromPath() - expectError(t, err) + if err == nil { + t.Errorf("Expected error") + } } func TestUpdateOnNonExistentFile(t *testing.T) { @@ -94,7 +96,9 @@ func TestExtractFromBadDataFile(t *testing.T) { ch := make(chan interface{}, 1) c := SourceFile{file.Name(), ch} err := c.extractFromPath() - expectError(t, err) + if err == nil { + t.Errorf("Expected error") + } expectEmptyChannel(t, ch) } @@ -102,15 +106,18 @@ func TestExtractFromValidDataFile(t *testing.T) { manifest := api.ContainerManifest{ID: ""} text, err := json.Marshal(manifest) - expectNoError(t, err) + if err != nil { + t.Errorf("Unexpected error: %v", err) + } file := writeTestFile(t, os.TempDir(), "test_pod_config", string(text)) defer os.Remove(file.Name()) ch := make(chan interface{}, 1) c := SourceFile{file.Name(), ch} err = c.extractFromPath() - expectNoError(t, err) - + if err != nil { + t.Errorf("Unexpected error: %v", err) + } update := (<-ch).(kubelet.PodUpdate) expected := CreatePodUpdate(kubelet.SET, kubelet.Pod{Name: simpleSubdomainSafeHash(file.Name()), Manifest: manifest}) if !reflect.DeepEqual(expected, update) { @@ -120,13 +127,17 @@ func TestExtractFromValidDataFile(t *testing.T) { func TestExtractFromEmptyDir(t *testing.T) { dirName, err := ioutil.TempDir("", "foo") - expectNoError(t, err) + if err != nil { + t.Errorf("Unexpected error: %v", err) + } defer os.RemoveAll(dirName) ch := make(chan interface{}, 1) c := SourceFile{dirName, ch} err = c.extractFromPath() - expectNoError(t, err) + if err != nil { + t.Errorf("Unexpected error: %v", err) + } update := (<-ch).(kubelet.PodUpdate) expected := CreatePodUpdate(kubelet.SET) @@ -143,15 +154,23 @@ func TestExtractFromDir(t *testing.T) { files := make([]*os.File, len(manifests)) dirName, err := ioutil.TempDir("", "foo") - expectNoError(t, err) + if err != nil { + t.Errorf("Unexpected error: %v", err) + } for i, manifest := range manifests { data, err := json.Marshal(manifest) - expectNoError(t, err) + if err != nil { + t.Errorf("Unexpected error: %v", err) + } file, err := ioutil.TempFile(dirName, manifest.ID) - expectNoError(t, err) + if err != nil { + t.Errorf("Unexpected error: %v", err) + } name := file.Name() - expectNoError(t, file.Close()) + if err := file.Close(); err != nil { + t.Errorf("Unexpected error: %v", err) + } ioutil.WriteFile(name, data, 0755) files[i] = file } @@ -159,7 +178,9 @@ func TestExtractFromDir(t *testing.T) { ch := make(chan interface{}, 1) c := SourceFile{dirName, ch} err = c.extractFromPath() - expectNoError(t, err) + if err != nil { + t.Errorf("Unexpected error: %v", err) + } update := (<-ch).(kubelet.PodUpdate) expected := CreatePodUpdate( diff --git a/pkg/kubelet/config/http_test.go b/pkg/kubelet/config/http_test.go index b1306f06116..bf5efb5add4 100644 --- a/pkg/kubelet/config/http_test.go +++ b/pkg/kubelet/config/http_test.go @@ -42,7 +42,9 @@ func TestExtractFromHttpBadness(t *testing.T) { ch := make(chan interface{}, 1) c := SourceURL{"http://localhost:49575/_not_found_", ch} err := c.extractFromURL() - expectError(t, err) + if err == nil { + t.Errorf("Expected error") + } expectEmptyChannel(t, ch) } @@ -63,7 +65,9 @@ func TestExtractFromHttpSingle(t *testing.T) { c := SourceURL{testServer.URL, ch} err = c.extractFromURL() - expectNoError(t, err) + if err != nil { + t.Errorf("Unexpected error: %v", err) + } update := (<-ch).(kubelet.PodUpdate) expected := CreatePodUpdate(kubelet.SET, kubelet.Pod{Name: "foo", Manifest: manifests[0]}) if !reflect.DeepEqual(expected, update) { @@ -90,7 +94,9 @@ func TestExtractFromHttpMultiple(t *testing.T) { c := SourceURL{testServer.URL, ch} err = c.extractFromURL() - expectNoError(t, err) + if err != nil { + t.Errorf("Unexpected error: %v", err) + } update := (<-ch).(kubelet.PodUpdate) expected := CreatePodUpdate(kubelet.SET, kubelet.Pod{Name: "1", Manifest: manifests[0]}, kubelet.Pod{Name: "bar", Manifest: manifests[1]}) @@ -115,7 +121,9 @@ func TestExtractFromHttpEmptyArray(t *testing.T) { c := SourceURL{testServer.URL, ch} err = c.extractFromURL() - expectNoError(t, err) + if err != nil { + t.Errorf("Unexpected error: %v", err) + } update := (<-ch).(kubelet.PodUpdate) expected := CreatePodUpdate(kubelet.SET) if !reflect.DeepEqual(expected, update) { diff --git a/pkg/tools/fake_etcd_client.go b/pkg/tools/fake_etcd_client.go index 5979e71daee..becf7f3a8d4 100644 --- a/pkg/tools/fake_etcd_client.go +++ b/pkg/tools/fake_etcd_client.go @@ -18,7 +18,6 @@ package tools import ( "fmt" - "testing" "github.com/coreos/go-etcd/etcd" ) @@ -28,13 +27,19 @@ type EtcdResponseWithError struct { E error } +// TestLogger is a type passed to Test functions to support formatted test logs. +type TestLogger interface { + Errorf(format string, args ...interface{}) + Logf(format string, args ...interface{}) +} + type FakeEtcdClient struct { watchCompletedChan chan bool Data map[string]EtcdResponseWithError DeletedKeys []string Err error - t *testing.T + t TestLogger Ix int // Will become valid after Watch is called; tester may write to it. Tester may @@ -45,7 +50,7 @@ type FakeEtcdClient struct { WatchStop chan<- bool } -func MakeFakeEtcdClient(t *testing.T) *FakeEtcdClient { +func MakeFakeEtcdClient(t TestLogger) *FakeEtcdClient { ret := &FakeEtcdClient{ t: t, Data: map[string]EtcdResponseWithError{},