Merge pull request #550 from yurishkuro/ys-fix-549

Simplify and fix unstable test

sd/internal/instance/cache_test.go

@@ -12,50 +12,33 @@ var _ sd.Instancer = &Cache{} // API check
 
 // The test verifies the following:
 //   registering causes initial notification of the current state
-//   notifications delivered to two receivers
-//   identical notifications cause no updates
-//   different update causes new notification
 //   instances are sorted
+//   different update causes new notification
+//   identical notifications cause no updates
 //   no updates after de-registering
 func TestCache(t *testing.T) {
 	e1 := sd.Event{Instances: []string{"y", "x"}} // not sorted
 	e2 := sd.Event{Instances: []string{"c", "a", "b"}}
 
-	c := NewCache()
-	if want, have := 0, len(c.State().Instances); want != have {
+	cache := NewCache()
+	if want, have := 0, len(cache.State().Instances); want != have {
 		t.Fatalf("want %v instances, have %v", want, have)
 	}
 
-	c.Update(e1) // sets initial state
-	if want, have := 2, len(c.State().Instances); want != have {
+	cache.Update(e1) // sets initial state
+	if want, have := 2, len(cache.State().Instances); want != have {
 		t.Fatalf("want %v instances, have %v", want, have)
 	}
 
 	r1 := make(chan sd.Event)
-	go c.Register(r1)
+	go cache.Register(r1)
 	expectUpdate(t, r1, []string{"x", "y"})
 
-	r2 := make(chan sd.Event)
-	go c.Register(r2)
-	expectUpdate(t, r2, []string{"x", "y"})
-
-	// send the same instances but in different order.
-	// because it's a duplicate it should not cause new notification.
-	// if it did, this call would deadlock trying to send to channels with no readers
-	c.Update(sd.Event{Instances: []string{"x", "y"}})
-	expectNoUpdate(t, r1)
-	expectNoUpdate(t, r2)
-
-	go c.Update(e2) // different set
+	go cache.Update(e2) // different set
 	expectUpdate(t, r1, []string{"a", "b", "c"})
-	expectUpdate(t, r2, []string{"a", "b", "c"})
 
-	c.Deregister(r1)
-	c.Deregister(r2)
+	cache.Deregister(r1)
 	close(r1)
-	close(r2)
-	// if deregister didn't work, Update would panic on closed channels
-	c.Update(e1)
 }
 
 func expectUpdate(t *testing.T, r chan sd.Event, expect []string) {
@@ -65,15 +48,30 @@ func expectUpdate(t *testing.T, r chan sd.Event, expect []string) {
 			t.Fatalf("want: %v, have: %v", want, have)
 		}
 	case <-time.After(time.Second):
-		t.Fatalf("did not receive expected update")
+		t.Fatalf("did not receive expected update %v", expect)
 	}
 }
 
-func expectNoUpdate(t *testing.T, r chan sd.Event) {
-	select {
-	case e := <-r:
-		t.Errorf("received unexpected update %v", e)
-	case <-time.After(time.Millisecond):
-		return // as expected
+func TestRegistry(t *testing.T) {
+	reg := make(registry)
+	c1 := make(chan sd.Event, 1)
+	c2 := make(chan sd.Event, 1)
+	reg.register(c1)
+	reg.register(c2)
+
+	// validate that both channels receive the update
+	reg.broadcast(sd.Event{Instances: []string{"x", "y"}})
+	if want, have := []string{"x", "y"}, (<-c1).Instances; !reflect.DeepEqual(want, have) {
+		t.Fatalf("want: %v, have: %v", want, have)
 	}
+	if want, have := []string{"x", "y"}, (<-c2).Instances; !reflect.DeepEqual(want, have) {
+		t.Fatalf("want: %v, have: %v", want, have)
+	}
+
+	reg.deregister(c1)
+	reg.deregister(c2)
+	close(c1)
+	close(c2)
+	// if deregister didn't work, broadcast would panic on closed channels
+	reg.broadcast(sd.Event{Instances: []string{"x", "y"}})
 }