Merge lp:~chipaca/ubuntu-push/watchticker into lp:ubuntu-push

[John Lenton]

Made bus.Endpoint's WatchSignal use an (exported) channel for waiting between sending values, if the channel is not nil.

made bus.Endpoint's WatchSignal use an (exported) channel for waiting between sending values, if the channel is not nil.

[Samuele Pedroni (pedronis)]

66 +var WatchTicker chan rune

using a rune here is a bit confusing, maybe

type TickT rune

const Tick TickT = 'x'

chan TickT

37 + for _, b := range expected.ticks {
38 + testingbus.WatchTicker <- b
39 + }

that's cute but also fairly obscure, also b sounds like a byte or boolean, not rune, expected ticks cool be just a bool no? or are always ticking once or not

[John Lenton (chipaca)]

or it could be an int, and we send over that many bools.

[John Lenton (chipaca)]

Re-proposed against new branch that does all the old stuff and more! Well, just the more bit discussed with Samuele. And it doesn't depend on client.

[Samuele Pedroni (pedronis)]

76 + <-time.Tick(10 * time.Millisecond)

this can just be a Sleep again right? it's a fresh ticker each time

104 + // wait for the destructor to be called
105 +Out:
106 + for {
107 + select {
108 + case <-ch:
109 + break Out
110 + case <-time.Tick(time.Millisecond):
111 + }
112 + }

I don't understand this code, shouldn't it just be a:

select {
  case <- ch:
    break
  case <- time.After(TIMEOUT):
    FAIL!
}

without the busy loop

[Samuele Pedroni (pedronis)]

review: Needs Information

[John Lenton (chipaca)]

Umm... right on both counts. Fixed.

[Samuele Pedroni (pedronis)]

looks good

review: Approve

[Ubuntu One Auto Pilot (otto-pilot)]

The attempt to merge lp:~chipaca/ubuntu-push/watchticker into lp:ubuntu-push failed. Below is the output from the failed tests.

mkdir -p /mnt/tarmac/cache/ubuntu-push/go-ws/bin
mkdir -p /mnt/tarmac/cache/ubuntu-push/go-ws/pkg
go get -u launchpad.net/godeps
go get -d -u launchpad.net/gocheck launchpad.net/go-dbus/v1
/mnt/tarmac/cache/ubuntu-push/go-ws/bin/godeps -u dependencies.tsv
"/mnt/tarmac/cache/ubuntu-push/go-ws/src/launchpad.net/gocheck" now at <email address hidden>
go install launchpad.net/gocheck launchpad.net/go-dbus/v1
go test launchpad.net/ubuntu-push/...
ok launchpad.net/ubuntu-push/bus 0.056s
ok launchpad.net/ubuntu-push/bus/connectivity 2.689s
ok launchpad.net/ubuntu-push/bus/networkmanager 1.387s
ok launchpad.net/ubuntu-push/bus/notifications 1.139s
ok launchpad.net/ubuntu-push/bus/testing 1.158s
ok launchpad.net/ubuntu-push/bus/urldispatcher 1.175s
ok launchpad.net/ubuntu-push/config 0.648s
ok launchpad.net/ubuntu-push/logger 0.108s
ok launchpad.net/ubuntu-push/protocol 0.402s
ok launchpad.net/ubuntu-push/server 1.488s
ok launchpad.net/ubuntu-push/server/acceptance 0.359s
? launchpad.net/ubuntu-push/server/acceptance/cmd [no test files]
ok launchpad.net/ubuntu-push/server/api 0.671s
ok launchpad.net/ubuntu-push/server/broker 0.650s
ok launchpad.net/ubuntu-push/server/broker/simple 0.933s
? launchpad.net/ubuntu-push/server/broker/testing [no test files]
? launchpad.net/ubuntu-push/server/broker/testsuite [no test files]
? launchpad.net/ubuntu-push/server/dev [no test files]
ok launchpad.net/ubuntu-push/server/listener 11.924s

----------------------------------------------------------------------
FAIL: session_test.go:412: sessionSuite.TestSessionLoopExchangeNextPing

session_test.go:430:
    c.Check(time.Since(tack) < (3+5)*time.Millisecond, Equals, true)
... obtained bool = false
... expected bool = true

OOPS: 22 passed, 1 FAILED
--- FAIL: TestSession (2.49 seconds)
FAIL
FAIL launchpad.net/ubuntu-push/server/session 3.182s
ok launchpad.net/ubuntu-push/server/store 3.959s
? launchpad.net/ubuntu-push/testing [no test files]
ok launchpad.net/ubuntu-push/testing/condition 0.695s
ok launchpad.net/ubuntu-push/util 1.332s
ok launchpad.net/ubuntu-push/whoopsie/identifier 0.994s
ok launchpad.net/ubuntu-push/whoopsie/identifier/testing 2.002s

make: *** [check] Error 1

[lp:~chipaca/ubuntu-push/watchticker]

30. By John Lenton on 2014-01-29

[r=pedronis] Introducing AutoRetry, a generic AutoRetry.

31. By John Lenton on 2014-01-30

[r=pedronis] Made bus.Endpoint's WatchSignal use an (exported) channel for waiting between sending values, if the channel is not nil.

=== modified file 'bus/connectivity/connectivity_test.go'
--- bus/connectivity/connectivity_test.go	2014-01-28 22:18:39 +0000
+++ bus/connectivity/connectivity_test.go	2014-01-29 11:33:47 +0000
@@ -196,29 +196,39 @@
 		uint32(networkmanager.Disconnected),
 	)
 
+	testingbus.WatchTicker = make(chan bool)
+	defer func() { testingbus.WatchTicker = nil }()
+
 	out := make(chan bool)
 	dt := time.Second / 10
 	timer := time.NewTimer(dt)
 	go ConnectedState(endp, cfg, nullog, out)
 	var v bool
-	expecteds := []bool{
-		false, // first state is always false
-		true,  // then it should be true as per ConnectedGlobal above
-		false, // then, false (upon receiving the next ConnectedGlobal)
-		true,  // then it should be true (webcheck passed)
-		false, // then it should be false (Disconnected)
-		false, // then it should be false again because it's restarted
+	expecteds := []struct {
+		p bool
+		s string
+		n int
+	}{
+		{false, "first state is always false", 0},
+		{true, "then it should be true as per ConnectedGlobal above", 0},
+		{false, "then, false (upon receiving the next ConnectedGlobal)", 1},
+		{true, "then it should be true (webcheck passed)", 0},
+		{false, "then it should be false (Disconnected)", 1},
+		{false, "then it should be false again because it's restarted", 1},
 	}
 
 	for i, expected := range expecteds {
+		for j := 0; j < expected.n; j++ {
+			testingbus.WatchTicker <- true
+		}
 		timer.Reset(dt)
 		select {
 		case v = <-out:
 			break
 		case <-timer.C:
-			c.Fatalf("Timed out before getting value (#%d)", i+1)
+			c.Fatalf("Timed out before getting value (#%d: %s)", i+1, expected.s)
 		}
 
-		c.Check(v, Equals, expected)
+		c.Check(v, Equals, expected.p, Commentf(expected.s))
 	}
 }

=== modified file 'bus/testing/testing_endpoint.go'
--- bus/testing/testing_endpoint.go	2014-01-27 12:41:54 +0000
+++ bus/testing/testing_endpoint.go	2014-01-29 11:33:47 +0000
@@ -49,6 +49,10 @@
 	return &testingEndpoint{dialCond, callCond, retvalses}
 }
 
+// if WatchTickeris not nil, it is used instead of the default timeout
+// to wait while sending values over WatchSignal
+var WatchTicker chan bool
+
 // See Endpoint's WatchSignal. This WatchSignal will check its condition to
 // decide whether to return an error, or provide each of its return values
 func (tc *testingEndpoint) WatchSignal(member string, f func(...interface{}), d func()) error {
@@ -56,7 +60,11 @@
 		go func() {
 			for _, v := range tc.retvals {
 				f(v...)
-				time.Sleep(10 * time.Millisecond)
+				if WatchTicker != nil {
+					<-WatchTicker
+				} else {
+					time.Sleep(10 * time.Millisecond)
+				}
 			}
 			d()
 		}()

=== modified file 'bus/testing/testing_endpoint_test.go'
--- bus/testing/testing_endpoint_test.go	2014-01-27 12:41:54 +0000
+++ bus/testing/testing_endpoint_test.go	2014-01-29 11:33:47 +0000
@@ -101,6 +101,32 @@
 	c.Check(e, NotNil)
 }
 
+// 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)
+
+	endp := NewTestingEndpoint(nil, condition.Work(true), 0, 0)
+	ch := make(chan int)
+	e := endp.WatchSignal("what", func(us ...interface{}) {}, func() { close(ch) })
+	c.Check(e, IsNil)
+
+	// wait for the destructor to be called
+	select {
+	case <-time.Tick(10 * time.Millisecond):
+		c.Fatal("timed out waiting for close on channel")
+	case <-ch:
+	}
+
+	// now if all went well, the ticker will have been tuck twice.
+	c.Assert(len(WatchTicker), Equals, 1)
+}
+
 // Tests that GetProperty() works
 func (s *TestingEndpointSuite) TestGetProperty(c *C) {
 	var m uint32 = 42