Merge lp:~chipaca/ubuntu-push/watchticker-goes-to-the-races into lp:ubuntu-push

[John Lenton]

get rid of a race in the bus/testing watchticker thing

Get rid of a race in the bus/testing watchticker thing.

[Samuele Pedroni (pedronis)]

58 +func SetWatchTicker(tc bus.Endpoint, watchTicker chan bool) {

this guy need a description now that the global has gone

should we experiment putting make check-race in the tarmac_tests.sh (after plain make check):

=== modified file 'tarmac_tests.sh'
--- tarmac_tests.sh 2014-01-14 15:35:20 +0000
+++ tarmac_tests.sh 2014-01-31 13:42:33 +0000
@@ -2,4 +2,5 @@
 # For running tests in Jenkins by Tarmac
 set -e
 make bootstrap && make check
+make check-race
 make check-format

[John Lenton (chipaca)]

Adding a description.

If tarmac is 64 bits, yes!

[lp:~chipaca/ubuntu-push/watchticker-goes-to-the-races]

36. By John Lenton on 2014-01-31

docs for SetWatchTicker

[Samuele Pedroni (pedronis)]

review: Approve

[lp:~chipaca/ubuntu-push/watchticker-goes-to-the-races]

37. By John Lenton on 2014-01-31

changed the tarmac script to call check-race

=== modified file 'bus/connectivity/connectivity_test.go'
--- bus/connectivity/connectivity_test.go	2014-01-28 22:41:55 +0000
+++ bus/connectivity/connectivity_test.go	2014-01-31 13:56:32 +0000
@@ -196,8 +196,8 @@
 		uint32(networkmanager.Disconnected),
 	)
 
-	testingbus.WatchTicker = make(chan bool)
-	defer func() { testingbus.WatchTicker = nil }()
+	watchTicker := make(chan bool)
+	testingbus.SetWatchTicker(endp, watchTicker)
 
 	out := make(chan bool)
 	dt := time.Second / 10
@@ -219,7 +219,7 @@
 
 	for i, expected := range expecteds {
 		for j := 0; j < expected.n; j++ {
-			testingbus.WatchTicker <- true
+			watchTicker <- true
 		}
 		timer.Reset(dt)
 		select {

=== modified file 'bus/testing/testing_endpoint.go'
--- bus/testing/testing_endpoint.go	2014-01-29 11:33:36 +0000
+++ bus/testing/testing_endpoint.go	2014-01-31 13:56:32 +0000
@@ -27,9 +27,10 @@
 )
 
 type testingEndpoint struct {
-	dialCond condition.Interface
-	callCond condition.Interface
-	retvals  [][]interface{}
+	dialCond    condition.Interface
+	callCond    condition.Interface
+	retvals     [][]interface{}
+	watchTicker chan bool
 }
 
 // Build a bus.Endpoint that calls OK() on its condition before returning
@@ -38,7 +39,7 @@
 // NOTE: Call() always returns the first return value; Watch() will provide
 // each of them in turn, irrespective of whether Call has been called.
 func NewMultiValuedTestingEndpoint(dialCond condition.Interface, callCond condition.Interface, retvalses ...[]interface{}) bus.Endpoint {
-	return &testingEndpoint{dialCond, callCond, retvalses}
+	return &testingEndpoint{dialCond, callCond, retvalses, nil}
 }
 
 func NewTestingEndpoint(dialCond condition.Interface, callCond condition.Interface, retvals ...interface{}) bus.Endpoint {
@@ -46,12 +47,15 @@
 	for i, x := range retvals {
 		retvalses[i] = []interface{}{x}
 	}
-	return &testingEndpoint{dialCond, callCond, retvalses}
+	return &testingEndpoint{dialCond, callCond, retvalses, nil}
 }
 
-// if WatchTickeris not nil, it is used instead of the default timeout
-// to wait while sending values over WatchSignal
-var WatchTicker chan bool
+// If SetWatchTicker is called with a non-nil watchTicker, it is used
+// instead of the default timeout to wait while sending values over
+// WatchSignal. Set it to nil again to restore default behaviour.
+func SetWatchTicker(tc bus.Endpoint, watchTicker chan bool) {
+	tc.(*testingEndpoint).watchTicker = watchTicker
+}
 
 // See Endpoint's WatchSignal. This WatchSignal will check its condition to
 // decide whether to return an error, or provide each of its return values
@@ -60,8 +64,8 @@
 		go func() {
 			for _, v := range tc.retvals {
 				f(v...)
-				if WatchTicker != nil {
-					<-WatchTicker
+				if tc.watchTicker != nil {
+					<-tc.watchTicker
 				} else {
 					time.Sleep(10 * time.Millisecond)
 				}

=== modified file 'bus/testing/testing_endpoint_test.go'
--- bus/testing/testing_endpoint_test.go	2014-01-29 11:33:36 +0000
+++ bus/testing/testing_endpoint_test.go	2014-01-31 13:56:32 +0000
@@ -104,14 +104,14 @@
 // Test WatchSignal can use the WatchTicker instead of a timeout (if
 // the former is not nil)
 func (s *TestingEndpointSuite) TestWatchTicker(c *C) {
-	WatchTicker = make(chan bool, 3)
-	defer func() { WatchTicker = nil }()
-	WatchTicker <- true
-	WatchTicker <- true
-	WatchTicker <- true
-	c.Assert(len(WatchTicker), Equals, 3)
+	watchTicker := make(chan bool, 3)
+	watchTicker <- true
+	watchTicker <- true
+	watchTicker <- true
+	c.Assert(len(watchTicker), Equals, 3)
 
 	endp := NewTestingEndpoint(nil, condition.Work(true), 0, 0)
+	SetWatchTicker(endp, watchTicker)
 	ch := make(chan int)
 	e := endp.WatchSignal("what", func(us ...interface{}) {}, func() { close(ch) })
 	c.Check(e, IsNil)
@@ -124,7 +124,7 @@
 	}
 
 	// now if all went well, the ticker will have been tuck twice.
-	c.Assert(len(WatchTicker), Equals, 1)
+	c.Assert(len(watchTicker), Equals, 1)
 }
 
 // Tests that GetProperty() works

=== modified file 'tarmac_tests.sh'
--- tarmac_tests.sh	2014-01-14 15:35:20 +0000
+++ tarmac_tests.sh	2014-01-31 13:56:32 +0000
@@ -1,5 +1,7 @@
 #!/bin/bash
 # For running tests in Jenkins by Tarmac
 set -e
-make bootstrap && make check
+make bootstrap
+make check
+make check-race
 make check-format