Merge lp:~axwalk/juju-core/worker-provisioner-test-speedup into lp:~go-bot/juju-core/trunk

[Andrew Wilkins]

Speed up worker/provisioner test

This cuts 10s off the tests, by forcing the
state to synchronise ahead of the 5s periodic
timer.

https://codereview.appspot.com/12767051/

Speed up worker/provisioner test

This cuts 10s off the tests, by forcing the
state to synchronise ahead of the 5s periodic
timer.

https://codereview.appspot.com/12767051/

[Andrew Wilkins (axwalk)]

Reviewers: mp+181222_code.launchpad.net,

Message:
Please take a look.

Description:
Speed up worker/provisioner test

This cuts 10s off the tests, by forcing the
state to synchronise ahead of the 5s periodic
timer.

https://code.launchpad.net/~axwalk/juju-core/worker-provisioner-test-speedup/+merge/181222

(do not edit description out of merge proposal)

Please review this at https://codereview.appspot.com/12767051/

Affected files:
   A [revision details]
   M worker/provisioner/lxc-broker_test.go

Index: [revision details]
=== added file '[revision details]'
--- [revision details] 2012-01-01 00:00:00 +0000
+++ [revision details] 2012-01-01 00:00:00 +0000
@@ -0,0 +1,2 @@
+Old revision: tarmac-20130821051648-ovunbhgihpcax89h
+New revision: <email address hidden>

Index: worker/provisioner/lxc-broker_test.go
=== modified file 'worker/provisioner/lxc-broker_test.go'
--- worker/provisioner/lxc-broker_test.go 2013-07-31 23:12:58 +0000
+++ worker/provisioner/lxc-broker_test.go 2013-08-21 08:32:52 +0000
@@ -264,10 +264,12 @@

   container := s.addContainer(c)

+ s.State.StartSync()
   instId := s.expectStarted(c, container)

   // ...and removed, along with the machine, when the machine is Dead.
   c.Assert(container.EnsureDead(), gc.IsNil)
+ s.State.StartSync()
   s.expectStopped(c, instId)
   s.waitRemoved(c, container)
  }

[John A Meinel (jameinel)]

This LGTM.

Though I have to wonder why we weren't doing it already, and I know that
Roger has a patch which is changing how we call StartSync vs Sync, etc.
So you might want to run it by him to see what makes sense and avoid
logical conflicts. (there won't be a textual conflict, AFAICT)

https://codereview.appspot.com/12767051/

[William Reade (fwereade)]

LGTM either way, but please have a look inside expectStarted and
expectStopped and see whether it would make more sense to put the syncs
therein.

https://codereview.appspot.com/12767051/

[Andrew Wilkins (axwalk)]

On 2013/08/21 15:44:21, fwereade wrote:
> LGTM either way, but please have a look inside expectStarted and
expectStopped
> and see whether it would make more sense to put the syncs therein.

That would be better, thanks. Done.

https://codereview.appspot.com/12767051/

=== modified file 'worker/provisioner/lxc-broker_test.go'
--- worker/provisioner/lxc-broker_test.go	2013-07-31 23:12:58 +0000
+++ worker/provisioner/lxc-broker_test.go	2013-08-22 01:45:20 +0000
@@ -197,6 +197,7 @@
 }
 
 func (s *lxcProvisionerSuite) expectStarted(c *gc.C, machine *state.Machine) string {
+	s.State.StartSync()
 	event := <-s.events
 	c.Assert(event.Action, gc.Equals, mock.Started)
 	err := machine.Refresh()
@@ -206,6 +207,7 @@
 }
 
 func (s *lxcProvisionerSuite) expectStopped(c *gc.C, instId string) {
+	s.State.StartSync()
 	event := <-s.events
 	c.Assert(event.Action, gc.Equals, mock.Stopped)
 	c.Assert(event.InstanceId, gc.Equals, instId)