Reviewers: mp+126536_code.launchpad.net,

Message:
Please take a look.

Description:
cmd/jujud: fix all tests



https://code.launchpad.net/~niemeyer/juju-core/fix-jujud-tests/+merge/126536

(do not edit description out of merge proposal)


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

Affected files:
   A [revision details]
   M cmd/jujud/provisioning_test.go
   M cmd/jujud/unit_test.go
   M cmd/jujud/upgrade_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: <email address hidden>
+New revision: <email address hidden>

Index: cmd/jujud/provisioning_test.go
=== modified file 'cmd/jujud/provisioning_test.go'
--- cmd/jujud/provisioning_test.go	2012-09-25 23:40:11 +0000
+++ cmd/jujud/provisioning_test.go	2012-09-26 20:11:21 +0000
@@ -5,6 +5,7 @@
  	"launchpad.net/juju-core/cmd"
  	"launchpad.net/juju-core/environs/dummy"
  	"launchpad.net/juju-core/juju/testing"
+	"launchpad.net/juju-core/state"
  	"reflect"
  	"time"
  )
@@ -56,7 +57,7 @@
  	c.Assert(err, IsNil)
  	units, err := s.Conn.AddUnits(svc, 1)
  	c.Assert(err, IsNil)
-	c.Check(opRecvTimeout(c, op, dummy.OpStartInstance{}), NotNil)
+	c.Check(s.opRecvTimeout(c, op, dummy.OpStartInstance{}), NotNil)

  	// Wait for the instance id to show up in the state.
  	id, err := units[0].AssignedMachineId()
@@ -64,7 +65,10 @@
  	m, err := s.State.Machine(id)
  	c.Assert(err, IsNil)
  	w := m.Watch()
+	defer w.Stop()
  	for _ = range w.Changes() {
+		err = m.Refresh()
+		c.Assert(err, IsNil)
  		_, err := m.InstanceId()
  		if state.IsNotFound(err) {
  			continue
@@ -75,7 +79,7 @@
  	err = units[0].OpenPort("tcp", 999)
  	c.Assert(err, IsNil)

-	c.Check(opRecvTimeout(c, op, dummy.OpOpenPorts{}), NotNil)
+	c.Check(s.opRecvTimeout(c, op, dummy.OpOpenPorts{}), NotNil)

  	err = a.Stop()
  	c.Assert(err, IsNil)
@@ -83,14 +87,15 @@
  	select {
  	case err := <-done:
  		c.Assert(err, IsNil)
-	case <-time.After(2 * time.Second):
+	case <-time.After(5 * time.Second):
  		c.Fatalf("timed out waiting for agent to terminate")
  	}
  }

  // opRecvTimeout waits for any of the given kinds of operation to
  // be received from ops, and times out if not.
-func opRecvTimeout(c *C, opc <-chan dummy.Operation,  
kinds ...dummy.Operation) dummy.Operation {
+func (s *ProvisioningSuite) opRecvTimeout(c *C, opc <-chan  
dummy.Operation, kinds ...dummy.Operation) dummy.Operation {
+	s.State.StartSync()
  	for {
  		select {
  		case op := <-opc:
@@ -100,7 +105,7 @@
  				}
  			}
  			c.Logf("discarding unknown event %#v", op)
-		case <-time.After(2 * time.Second):
+		case <-time.After(5 * time.Second):
  			c.Fatalf("time out wating for operation")
  		}
  	}


Index: cmd/jujud/unit_test.go
=== modified file 'cmd/jujud/unit_test.go'
--- cmd/jujud/unit_test.go	2012-09-13 20:32:50 +0000
+++ cmd/jujud/unit_test.go	2012-09-26 19:39:30 +0000
@@ -96,6 +96,9 @@
  				continue
  			case state.UnitStarted:
  				c.Logf("started!")
+			case state.UnitDown:
+				s.State.StartSync()
+				c.Logf("unit is still down")
  			default:
  				c.Fatalf("unexpected status %s %s", st, info)
  			}


Index: cmd/jujud/upgrade_test.go
=== modified file 'cmd/jujud/upgrade_test.go'
--- cmd/jujud/upgrade_test.go	2012-09-17 11:21:13 +0000
+++ cmd/jujud/upgrade_test.go	2012-09-26 19:39:30 +0000
@@ -17,42 +17,30 @@
  	"time"
  )

-var _ = Suite(&upgraderSuite{})
+var _ = Suite(&UpgraderSuite{})

-type upgraderSuite struct {
+type UpgraderSuite struct {
  	oldVersion version.Binary
  	testing.JujuConnSuite
  }

-func (s *upgraderSuite) SetUpTest(c *C) {
+func (s *UpgraderSuite) SetUpTest(c *C) {
  	s.JujuConnSuite.SetUpTest(c)
  	s.oldVersion = version.Current
  }

-func (s *upgraderSuite) TearDownTest(c *C) {
+func (s *UpgraderSuite) TearDownTest(c *C) {
  	version.Current = s.oldVersion
  	s.JujuConnSuite.TearDownTest(c)
  }

-func (s *upgraderSuite) TestUpgraderError(c *C) {
-	st, err := state.Open(s.StateInfo(c))
-	c.Assert(err, IsNil)
-	// We have no installed tools, so the logic should set the agent
-	// tools anyway, but with no URL.
-	u := startUpgrader(c, st, c.MkDir(), &state.Tools{Binary:  
version.Current})
-
-	// Close the state under the watcher and check that the upgrader dies.
-	st.Close()
-	waitDeath(c, u, nil, "watcher: cannot get content of node.*")
-}
-
-func (s *upgraderSuite) TestUpgraderStop(c *C) {
+func (s *UpgraderSuite) TestUpgraderStop(c *C) {
  	u := startUpgrader(c, s.State, c.MkDir(), &state.Tools{Binary:  
version.Current})
  	err := u.Stop()
  	c.Assert(err, IsNil)
  }

-func (s *upgraderSuite) proposeVersion(c *C, vers version.Number,  
development bool) {
+func (s *UpgraderSuite) proposeVersion(c *C, vers version.Number,  
development bool) {
  	cfg, err := s.State.EnvironConfig()
  	c.Assert(err, IsNil)
  	attrs := cfg.AllAttrs()
@@ -64,7 +52,7 @@
  	c.Assert(err, IsNil)
  }

-func (s *upgraderSuite) uploadTools(c *C, vers version.Binary)  
*state.Tools {
+func (s *UpgraderSuite) uploadTools(c *C, vers version.Binary)  
*state.Tools {
  	tgz := coretesting.TarGz(
  		coretesting.NewTarFile("juju", 0777, "juju contents "+vers.String()),
  		coretesting.NewTarFile("jujuc", 0777, "jujuc contents "+vers.String()),
@@ -131,7 +119,7 @@
  },
  }

-func (s *upgraderSuite) TestUpgrader(c *C) {
+func (s *UpgraderSuite) TestUpgrader(c *C) {
  	dataDir, currentTools := s.primeTools(c,  
version.MustParseBinary("2.0.0-foo-bar"))
  	// Remove the tools from the storage so that we're sure that the
  	// uploader isn't trying to fetch them.
@@ -171,8 +159,10 @@
  		}
  		if test.propose != "" {
  			s.proposeVersion(c, version.MustParse(test.propose), test.devVersion)
+			s.State.StartSync()
  		}
  		if test.upgradeTo == "" {
+			s.State.StartSync()
  			assertNothingHappens(c, upgraderDone)
  		} else {
  			tools := uploaded[version.MustParse(test.upgradeTo)]
@@ -229,7 +219,7 @@
  },
  }

-func (s *upgraderSuite) TestDelayedStop(c *C) {
+func (s *UpgraderSuite) TestDelayedStop(c *C) {
  	defer dummy.SetStorageDelay(0)
  	dataDir, tools := s.primeTools(c,  
version.MustParseBinary("2.0.3-foo-bar"))
  	s.uploadTools(c, version.MustParseBinary("2.0.5-foo-bar"))
@@ -259,12 +249,12 @@
  	}
  }

-func (s *upgraderSuite) poisonVersion(vers version.Binary) {
+func (s *UpgraderSuite) poisonVersion(vers version.Binary) {
  	path := environs.ToolsStoragePath(vers)
  	dummy.Poison(s.Conn.Environ.Storage(), path, fmt.Errorf("poisoned file"))
  }

-func (s *upgraderSuite) removeVersion(c *C, vers version.Binary) {
+func (s *UpgraderSuite) removeVersion(c *C, vers version.Binary) {
  	path := environs.ToolsStoragePath(vers)
  	err := s.Conn.Environ.Storage().Remove(path)
  	c.Assert(err, IsNil)
@@ -272,7 +262,7 @@

  // primeTools sets up the current version of the tools to vers and
  // makes sure that they're available in the returned dataDir.
-func (s *upgraderSuite) primeTools(c *C, vers version.Binary) (dataDir  
string, tools *state.Tools) {
+func (s *UpgraderSuite) primeTools(c *C, vers version.Binary) (dataDir  
string, tools *state.Tools) {
  	dataDir = c.MkDir()
  	// Set up the current version and tools.
  	version.Current = vers