Add tomb to Pinger

state/presence/presence_test.go

 package presence_test
 
 import (
         "fmt"
         . "launchpad.net/gocheck"
         "launchpad.net/gozk/zookeeper"
         "launchpad.net/juju-core/juju/state/presence"
         "testing"
         "time"
 )
@@ skipping 108 matching lines @@
 
         // Watch for life, and check the watch doesn't fire early.
         alive, watch, err := presence.AliveW(s.zkConn, path)
         c.Assert(err, IsNil)
         c.Assert(alive, Equals, false)
         assertNoChange(c, watch)
 
         // Start a Pinger, and check the watch fires.
         p, err := presence.StartPinger(s.zkConn, path, period)
         c.Assert(err, IsNil)
-        defer p.Stop()
         assertChange(c, watch, true)
 
         // Check that Alive agrees.
         alive, err = presence.Alive(s.zkConn, path)
         c.Assert(err, IsNil)
         c.Assert(alive, Equals, true)
 
         // Watch for life again, and check it doesn't change.
         alive, watch, err = presence.AliveW(s.zkConn, path)
         c.Assert(err, IsNil)
         c.Assert(alive, Equals, true)
         assertNoChange(c, watch)
+
+        // Clean up.
+        err = p.Kill()
+        c.Assert(err, IsNil)
 }
 
 func (s *PresenceSuite) TestKillPinger(c *C) {
         // Start a Pinger and a watch, and check sanity.
         p, err := presence.StartPinger(s.zkConn, path, period)
         c.Assert(err, IsNil)
         alive, watch, err := presence.AliveW(s.zkConn, path)
         c.Assert(err, IsNil)
         c.Assert(alive, Equals, true)
         assertNoChange(c, watch)
 
         // Kill the Pinger; check the watch fires and Alive agrees.
-        p.Kill()
+        err = p.Kill()
+        c.Assert(err, IsNil)
         assertChange(c, watch, false)
         alive, err = presence.Alive(s.zkConn, path)
         c.Assert(err, IsNil)
         c.Assert(alive, Equals, false)
 
         // Check that the pinger's node was deleted.
         stat, err := s.zkConn.Exists(path)
         c.Assert(err, IsNil)
         c.Assert(stat, IsNil)
 }
 
 func (s *PresenceSuite) TestStopPinger(c *C) {
         // Start a Pinger and a watch, and check sanity.
         p, err := presence.StartPinger(s.zkConn, path, period)
         c.Assert(err, IsNil)
         alive, watch, err := presence.AliveW(s.zkConn, path)
         c.Assert(err, IsNil)
         c.Assert(alive, Equals, true)
         assertNoChange(c, watch)
 
         // Stop the Pinger; check the watch fires and Alive agrees.
-        p.Stop()
+        err = p.Stop()
+        c.Assert(err, IsNil)
         assertChange(c, watch, false)
         alive, err = presence.Alive(s.zkConn, path)
         c.Assert(err, IsNil)
         c.Assert(alive, Equals, false)
 
         // Check that the pinger's node is still present.
         stat, err := s.zkConn.Exists(path)
         c.Assert(err, IsNil)
         c.Assert(stat, NotNil)
 }
 
 func (s *PresenceSuite) TestWatchDeadnessChange(c *C) {
         // Create a stale node.
         p, err := presence.StartPinger(s.zkConn, path, period)
         c.Assert(err, IsNil)
-        p.Stop()
+        err = p.Stop()
+        c.Assert(err, IsNil)
         time.Sleep(longEnough)
 
         // Start watching for liveness.
         alive, watch, err := presence.AliveW(s.zkConn, path)
         c.Assert(err, IsNil)
         c.Assert(alive, Equals, false)
 
         // Delete the node and check the watch doesn't fire.
         err = s.zkConn.Delete(path, -1)
         c.Assert(err, IsNil)
         assertNoChange(c, watch)
 
         // Start a new Pinger and check the watch does fire.
         p, err = presence.StartPinger(s.zkConn, path, period)
         c.Assert(err, IsNil)
-        defer p.Stop()
         assertChange(c, watch, true)
+
+        // Clean up.
+        err = p.Kill()
+        c.Assert(err, IsNil)
 }
 
 func (s *PresenceSuite) TestBadData(c *C) {
         // Create a node that contains inappropriate data.
         _, err := s.zkConn.Create(path, "roflcopter", 0, zookeeper.WorldACL(zookeeper.PERM_ALL))
         c.Assert(err, IsNil)
 
         // Check it is not interpreted as a presence node by Alive.
         _, err = presence.Alive(s.zkConn, path)
         c.Assert(err, ErrorMatches, `/presence presence node has bad data: "roflcopter"`)
 
         // Check it is not interpreted as a presence node by Watch.
         _, watch, err := presence.AliveW(s.zkConn, path)
         c.Assert(watch, IsNil)
         c.Assert(err, ErrorMatches, `/presence presence node has bad data: "roflcopter"`)
 }
 
 func (s *PresenceSuite) TestDisconnectDeadWatch(c *C) {
         // Create a target node.
         p, err := presence.StartPinger(s.zkConn, path, period)
         c.Assert(err, IsNil)
-        p.Stop()
+        err = p.Stop()
+        c.Assert(err, IsNil)
 
         // Start an alternate connection and ensure the node is stale.
         altConn := s.connect(c)
         time.Sleep(longEnough)
 
         // Start a watch using the alternate connection.
         alive, watch, err := presence.AliveW(altConn, path)
         c.Assert(err, IsNil)
         c.Assert(alive, Equals, false)
 
@@ skipping 13 matching lines @@
 
         // Kill the watch's connection and check it's alerted.
         altConn.Close()
         assertClose(c, watch)
 }
 
 func (s *PresenceSuite) TestDisconnectAliveWatch(c *C) {
         // Start a Pinger on the main connection
         p, err := presence.StartPinger(s.zkConn, path, period)
         c.Assert(err, IsNil)
-        defer p.Stop()
 
         // Start watching on an alternate connection.
         altConn := s.connect(c)
         alive, watch, err := presence.AliveW(altConn, path)
         c.Assert(err, IsNil)
         c.Assert(alive, Equals, true)
 
         // Kill the watch's connection and check it's alerted.
         altConn.Close()
         assertClose(c, watch)
+
+        // Clean up.
+        err = p.Kill()
+        c.Assert(err, IsNil)
 }
 
 func (s *PresenceSuite) TestDisconnectPinger(c *C) {
         // Start a Pinger on an alternate connection.
         altConn := s.connect(c)
         p, err := presence.StartPinger(altConn, path, period)
         c.Assert(err, IsNil)
-        defer p.Stop()
 
         // Watch on the "main" connection.
         alive, watch, err := presence.AliveW(s.zkConn, path)
         c.Assert(err, IsNil)
         c.Assert(alive, Equals, true)
 
         // Kill the pinger connection and check the watch notices.
         altConn.Close()
         assertChange(c, watch, false)
+
+        // Check the pinger already knows it broke.
+        <-p.Dying()

[niemeyer, 2012/06/14 15:50:50]

There's apparently no inverted test of Dying. I'm not excessively worried, but
it could be returning a closed channel right now and everything would pass fine.

+
+        // Stop the pinger anyway; check we get an error.
+        err = p.Stop()
+        c.Assert(err, NotNil)
 }
 
 func (s *PresenceSuite) TestWaitAlive(c *C) {
         err := presence.WaitAlive(s.zkConn, path, longEnough)
         c.Assert(err, ErrorMatches, "presence: still not alive after timeout")
 
-        // Start a pinger with a short delay so that WaitAlive() detects it.
+        dying := make(chan struct{})
+        dead := make(chan struct{})
+
+        // Start a pinger with a short delay so that WaitAlive() has to wait.
         go func() {
                 time.Sleep(period * 2)
                 p, err := presence.StartPinger(s.zkConn, path, period)
                 c.Assert(err, IsNil)
-                defer p.Kill()
+                <-dying
+                err = p.Kill()
+                c.Assert(err, IsNil)
+                close(dead)
         }()
 
+        // Wait for, and check, liveness.
         err = presence.WaitAlive(s.zkConn, path, longEnough)
         c.Assert(err, IsNil)
+        close(dying)
+        <-dead
+}
 
-        // Use alternative connection for closing test.
+func (s *PresenceSuite) TestDisconnectWaitAlive(c *C) {
+        // Start a new connection with a short lifespan.
         altConn := s.connect(c)
-
         go func() {
                 time.Sleep(period * 2)
                 altConn.Close()
         }()
 
-        err = presence.WaitAlive(altConn, path, longEnough)
+        // Check that WaitAlive returns an appropriate error.
+        err := presence.WaitAlive(altConn, path, longEnough)
         c.Assert(err, ErrorMatches, "presence: channel closed while waiting")
 }