Full tests for TrackLocalListeners()

Author: relistan

catalog/services_state.go

@@ -404,22 +404,17 @@ func (state *ServicesState) TrackLocalListeners(fn func() []Listener, looper dir
 		// Add new listeners
 		for _, listener := range discovered {
 			state.RLock()
-			if _, ok := state.listeners[listener.Name()]; !ok {
-				// We fire off a goroutine to add it, which will block until we
-				// (and anyone else) release the read lock.  The only use case
-				// where this is likely to be a problem is where we're adding
-				// and removing listeners quickly, which is explicitly not a
-				// good idea, anyway. Since listener name is built around
-				// service ID, even in that scenario this should not cause
-				// issues.
+			_, ok := state.listeners[listener.Name()]
+			state.RUnlock()
+
+			if !ok {
 				log.Infof("Adding listener %s because it was just discovered", listener.Name())
-				go state.AddListener(listener)
+				state.AddListener(listener)
 			}
-			state.RUnlock()
 		}
-
 		// Remove old ones
-		for _, listener := range state.listeners {
+		listeners := state.listeners
+		for _, listener := range listeners {
 			if listener.Managed() && !containsListener(discovered, listener.Name()) {
 				log.Infof("Removing listener %s because the service appears to be gone", listener.Name())
 				state.RemoveListener(listener.Name())

catalog/services_state_test.go

@@ -487,8 +487,49 @@ func Test_Listeners(t *testing.T) {
 			So(result2.Service.Hostname, ShouldEqual, hostname)
 		})
 
-		Reset(func() {
-			state = NewServicesState()
+		Convey("GetListeners() returns all the listeners", func() {
+			state.AddListener(listener)
+			state.AddListener(listener2)
+
+			So(len(state.GetListeners()), ShouldEqual, 2)
+		})
+
+		Convey("containsListener() finds a listener if present", func() {
+			listeners := []Listener{listener, listener2}
+			So(containsListener(listeners, "listener1"), ShouldBeTrue)
+		})
+
+		Convey("Tracking dynamic listeners", func() {
+			Convey("Adds new listeners that are discovered", func() {
+				looper := director.NewFreeLooper(director.ONCE, nil)
+				listeners := []Listener{listener, listener2}
+				state.TrackLocalListeners(func() []Listener { return listeners }, looper)
+
+				So(len(state.listeners), ShouldEqual, 2)
+			})
+
+			Convey("Removes listeners that have gone away", func() {
+				// Add some and track them
+				looper := director.NewFreeLooper(director.ONCE, nil)
+				listeners := []Listener{listener, listener2}
+				listenFunc := func() []Listener { return listeners }
+				listener.managed = true
+				listener2.managed = true
+
+				state.TrackLocalListeners(listenFunc, looper)
+				So(len(state.listeners), ShouldEqual, 2)
+
+				// Discovery now returns only the first one
+				listeners = []Listener{listener}
+				looper = director.NewFreeLooper(director.ONCE, nil)
+
+				state.TrackLocalListeners(listenFunc, looper)
+				So(len(state.listeners), ShouldEqual, 1)
+
+				found, ok := state.listeners[listener.Name()]
+				So(ok, ShouldBeTrue)
+				So(found, ShouldResemble, listener)
+			})
 		})
 	})
 }