Merge pull request #312 from weaveworks/311-flaky

Use poll in TestRegistryEvents.

probe/docker/container_test.go

@@ -61,8 +61,7 @@ func TestContainer(t *testing.T) {
 	}
 
 	// Now see if we go them
-	test.Poll(t, 10*time.Millisecond, func() bool {
-		nmd := c.GetNodeMetadata()
-		return nmd[docker.MemoryUsage] == "12345"
-	}, "Failed to get stats")
+	test.Poll(t, 10*time.Millisecond, "12345", func() interface{} {
+		return c.GetNodeMetadata()[docker.MemoryUsage]
+	})
 }

probe/docker/registry_test.go

@@ -46,7 +46,7 @@ func (c *mockContainer) GetNodeMetadata() report.NodeMetadata {
 }
 
 type mockDockerClient struct {
-	sync.Mutex
+	sync.RWMutex
 	apiContainers []client.APIContainers
 	containers    map[string]*client.Container
 	apiImages     []client.APIImages
@@ -54,16 +54,20 @@ type mockDockerClient struct {
 }
 
 func (m *mockDockerClient) ListContainers(client.ListContainersOptions) ([]client.APIContainers, error) {
+	m.RLock()
+	defer m.RUnlock()
 	return m.apiContainers, nil
 }
 
 func (m *mockDockerClient) InspectContainer(id string) (*client.Container, error) {
+	m.RLock()
+	defer m.RUnlock()
 	return m.containers[id], nil
 }
 
 func (m *mockDockerClient) ListImages(client.ListImagesOptions) ([]client.APIImages, error) {
-	m.Lock()
-	defer m.Unlock()
+	m.RLock()
+	defer m.RUnlock()
 	return m.apiImages, nil
 }
 
@@ -86,8 +90,8 @@ func (m *mockDockerClient) RemoveEventListener(events chan *client.APIEvents) er
 }
 
 func (m *mockDockerClient) send(event *client.APIEvents) {
-	m.Lock()
-	defer m.Unlock()
+	m.RLock()
+	defer m.RUnlock()
 	for _, c := range m.events {
 		c <- event
 	}
@@ -160,11 +164,12 @@ func TestRegistry(t *testing.T) {
 		defer registry.Stop()
 		runtime.Gosched()
 
-		test.Poll(t, 10*time.Millisecond, func() bool {
-			have := allContainers(registry)
+		{
 			want := []docker.Container{&mockContainer{container1}}
-			return reflect.DeepEqual(want, have)
-		}, "Didn't get containers!")
+			test.Poll(t, 10*time.Millisecond, want, func() interface{} {
+				return allContainers(registry)
+			})
+		}
 
 		{
 			have := allImages(registry)
@@ -183,6 +188,12 @@ func TestRegistryEvents(t *testing.T) {
 		defer registry.Stop()
 		runtime.Gosched()
 
+		check := func(want []docker.Container) {
+			test.Poll(t, 10*time.Millisecond, want, func() interface{} {
+				return allContainers(registry)
+			})
+		}
+
 		{
 			mdc.Lock()
 			mdc.containers["wiff"] = container2
@@ -190,11 +201,8 @@ func TestRegistryEvents(t *testing.T) {
 			mdc.send(&client.APIEvents{Status: docker.StartEvent, ID: "wiff"})
 			runtime.Gosched()
 
-			have := allContainers(registry)
 			want := []docker.Container{&mockContainer{container1}, &mockContainer{container2}}
-			if !reflect.DeepEqual(want, have) {
-				t.Errorf("%s", test.Diff(want, have))
-			}
+			check(want)
 		}
 
 		{
@@ -204,11 +212,8 @@ func TestRegistryEvents(t *testing.T) {
 			mdc.send(&client.APIEvents{Status: docker.DieEvent, ID: "wiff"})
 			runtime.Gosched()
 
-			have := allContainers(registry)
 			want := []docker.Container{&mockContainer{container1}}
-			if !reflect.DeepEqual(want, have) {
-				t.Errorf("%s", test.Diff(want, have))
-			}
+			check(want)
 		}
 
 		{
@@ -218,22 +223,16 @@ func TestRegistryEvents(t *testing.T) {
 			mdc.send(&client.APIEvents{Status: docker.DieEvent, ID: "ping"})
 			runtime.Gosched()
 
-			have := allContainers(registry)
 			want := []docker.Container{}
-			if !reflect.DeepEqual(want, have) {
-				t.Errorf("%s", test.Diff(want, have))
-			}
+			check(want)
 		}
 
 		{
 			mdc.send(&client.APIEvents{Status: docker.DieEvent, ID: "doesntexist"})
 			runtime.Gosched()
 
-			have := allContainers(registry)
 			want := []docker.Container{}
-			if !reflect.DeepEqual(want, have) {
-				t.Errorf("%s", test.Diff(want, have))
-			}
+			check(want)
 		}
 	})
 }

test/poll.go

@@ -1,20 +1,25 @@
 package test
 
 import (
+	"reflect"
 	"testing"
 	"time"
 )
 
 // Poll repeatedly evaluates condition until we either timeout, or it suceeds.
-func Poll(t *testing.T, d time.Duration, condition func() bool, msg string) {
+func Poll(t *testing.T, d time.Duration, want interface{}, have func() interface{}) {
 	deadline := time.Now().Add(d)
 	for {
 		if time.Now().After(deadline) {
-			t.Fatal(msg)
+			break
 		}
-		if condition() {
+		if reflect.DeepEqual(want, have()) {
 			return
 		}
 		time.Sleep(d / 10)
 	}
+	h := have()
+	if reflect.DeepEqual(want, h) {
+		t.Fatal(Diff(want, h))
+	}
 }

xfer/collector_internal_test.go

@@ -53,9 +53,9 @@ func TestCollector(t *testing.T) {
 	}
 
 	reports <- r
-	test.Poll(t, 100*time.Millisecond, func() bool {
-		return len(concreteCollector.peek().Address.NodeMetadatas) == 1
-	}, "missed the report")
+	test.Poll(t, 100*time.Millisecond, 1, func() interface{} {
+		return len(concreteCollector.peek().Address.NodeMetadatas)
+	})
 
 	go func() { publish <- time.Now() }()
 	collected := <-collector.Reports()