juju-core

Merge lp:~niemeyer/juju-core/fix-jujud-tests into lp:~juju/juju-core/trunk

  1. fix-jujud-tests
  2. Merge into trunk
Proposed by Gustavo Niemeyer
Status: Merged
Merged at revision: 583
Proposed branch: lp:~niemeyer/juju-core/fix-jujud-tests
Merge into: lp:~juju/juju-core/trunk
Diff against target: 196 lines (+27/-29)
3 files modified
cmd/jujud/provisioning_test.go (+10/-5)
cmd/jujud/unit_test.go (+3/-0)
cmd/jujud/upgrade_test.go (+14/-24)
To merge this branch: bzr merge lp:~niemeyer/juju-core/fix-jujud-tests
Reviewer Review Type Date Requested Status
The Go Language Gophers Pending
Review via email: mp+126536@code.launchpad.net

Description of the change

cmd/jujud: fix all tests

https://codereview.appspot.com/6565052/

To post a comment you must log in.
Revision history for this message
Gustavo Niemeyer (niemeyer) wrote :
Download full text (7.1 KiB)

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 i...

Read more...

Revision history for this message
William Reade (fwereade) wrote :

LGTM

https://codereview.appspot.com/6565052/

Revision history for this message
Gustavo Niemeyer (niemeyer) wrote :

*** Submitted:

cmd/jujud: fix all tests

R=fwereade
CC=
https://codereview.appspot.com/6565052

https://codereview.appspot.com/6565052/

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== 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:13:27 +0000
@@ -5,6 +5,7 @@
5 "launchpad.net/juju-core/cmd"5 "launchpad.net/juju-core/cmd"
6 "launchpad.net/juju-core/environs/dummy"6 "launchpad.net/juju-core/environs/dummy"
7 "launchpad.net/juju-core/juju/testing"7 "launchpad.net/juju-core/juju/testing"
8 "launchpad.net/juju-core/state"
8 "reflect"9 "reflect"
9 "time"10 "time"
10)11)
@@ -56,7 +57,7 @@
56 c.Assert(err, IsNil)57 c.Assert(err, IsNil)
57 units, err := s.Conn.AddUnits(svc, 1)58 units, err := s.Conn.AddUnits(svc, 1)
58 c.Assert(err, IsNil)59 c.Assert(err, IsNil)
59 c.Check(opRecvTimeout(c, op, dummy.OpStartInstance{}), NotNil)60 c.Check(s.opRecvTimeout(c, op, dummy.OpStartInstance{}), NotNil)
6061
61 // Wait for the instance id to show up in the state.62 // Wait for the instance id to show up in the state.
62 id, err := units[0].AssignedMachineId()63 id, err := units[0].AssignedMachineId()
@@ -64,7 +65,10 @@
64 m, err := s.State.Machine(id)65 m, err := s.State.Machine(id)
65 c.Assert(err, IsNil)66 c.Assert(err, IsNil)
66 w := m.Watch()67 w := m.Watch()
68 defer w.Stop()
67 for _ = range w.Changes() {69 for _ = range w.Changes() {
70 err = m.Refresh()
71 c.Assert(err, IsNil)
68 _, err := m.InstanceId()72 _, err := m.InstanceId()
69 if state.IsNotFound(err) {73 if state.IsNotFound(err) {
70 continue74 continue
@@ -75,7 +79,7 @@
75 err = units[0].OpenPort("tcp", 999)79 err = units[0].OpenPort("tcp", 999)
76 c.Assert(err, IsNil)80 c.Assert(err, IsNil)
7781
78 c.Check(opRecvTimeout(c, op, dummy.OpOpenPorts{}), NotNil)82 c.Check(s.opRecvTimeout(c, op, dummy.OpOpenPorts{}), NotNil)
7983
80 err = a.Stop()84 err = a.Stop()
81 c.Assert(err, IsNil)85 c.Assert(err, IsNil)
@@ -83,14 +87,15 @@
83 select {87 select {
84 case err := <-done:88 case err := <-done:
85 c.Assert(err, IsNil)89 c.Assert(err, IsNil)
86 case <-time.After(2 * time.Second):90 case <-time.After(5 * time.Second):
87 c.Fatalf("timed out waiting for agent to terminate")91 c.Fatalf("timed out waiting for agent to terminate")
88 }92 }
89}93}
9094
91// opRecvTimeout waits for any of the given kinds of operation to95// opRecvTimeout waits for any of the given kinds of operation to
92// be received from ops, and times out if not.96// be received from ops, and times out if not.
93func opRecvTimeout(c *C, opc <-chan dummy.Operation, kinds ...dummy.Operation) dummy.Operation {97func (s *ProvisioningSuite) opRecvTimeout(c *C, opc <-chan dummy.Operation, kinds ...dummy.Operation) dummy.Operation {
98 s.State.StartSync()
94 for {99 for {
95 select {100 select {
96 case op := <-opc:101 case op := <-opc:
@@ -100,7 +105,7 @@
100 }105 }
101 }106 }
102 c.Logf("discarding unknown event %#v", op)107 c.Logf("discarding unknown event %#v", op)
103 case <-time.After(2 * time.Second):108 case <-time.After(5 * time.Second):
104 c.Fatalf("time out wating for operation")109 c.Fatalf("time out wating for operation")
105 }110 }
106 }111 }
107112
=== 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 20:13:27 +0000
@@ -96,6 +96,9 @@
96 continue96 continue
97 case state.UnitStarted:97 case state.UnitStarted:
98 c.Logf("started!")98 c.Logf("started!")
99 case state.UnitDown:
100 s.State.StartSync()
101 c.Logf("unit is still down")
99 default:102 default:
100 c.Fatalf("unexpected status %s %s", st, info)103 c.Fatalf("unexpected status %s %s", st, info)
101 }104 }
102105
=== 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 20:13:27 +0000
@@ -17,42 +17,30 @@
17 "time"17 "time"
18)18)
1919
20var _ = Suite(&upgraderSuite{})20var _ = Suite(&UpgraderSuite{})
2121
22type upgraderSuite struct {22type UpgraderSuite struct {
23 oldVersion version.Binary23 oldVersion version.Binary
24 testing.JujuConnSuite24 testing.JujuConnSuite
25}25}
2626
27func (s *upgraderSuite) SetUpTest(c *C) {27func (s *UpgraderSuite) SetUpTest(c *C) {
28 s.JujuConnSuite.SetUpTest(c)28 s.JujuConnSuite.SetUpTest(c)
29 s.oldVersion = version.Current29 s.oldVersion = version.Current
30}30}
3131
32func (s *upgraderSuite) TearDownTest(c *C) {32func (s *UpgraderSuite) TearDownTest(c *C) {
33 version.Current = s.oldVersion33 version.Current = s.oldVersion
34 s.JujuConnSuite.TearDownTest(c)34 s.JujuConnSuite.TearDownTest(c)
35}35}
3636
37func (s *upgraderSuite) TestUpgraderError(c *C) {37func (s *UpgraderSuite) TestUpgraderStop(c *C) {
38 st, err := state.Open(s.StateInfo(c))
39 c.Assert(err, IsNil)
40 // We have no installed tools, so the logic should set the agent
41 // tools anyway, but with no URL.
42 u := startUpgrader(c, st, c.MkDir(), &state.Tools{Binary: version.Current})
43
44 // Close the state under the watcher and check that the upgrader dies.
45 st.Close()
46 waitDeath(c, u, nil, "watcher: cannot get content of node.*")
47}
48
49func (s *upgraderSuite) TestUpgraderStop(c *C) {
50 u := startUpgrader(c, s.State, c.MkDir(), &state.Tools{Binary: version.Current})38 u := startUpgrader(c, s.State, c.MkDir(), &state.Tools{Binary: version.Current})
51 err := u.Stop()39 err := u.Stop()
52 c.Assert(err, IsNil)40 c.Assert(err, IsNil)
53}41}
5442
55func (s *upgraderSuite) proposeVersion(c *C, vers version.Number, development bool) {43func (s *UpgraderSuite) proposeVersion(c *C, vers version.Number, development bool) {
56 cfg, err := s.State.EnvironConfig()44 cfg, err := s.State.EnvironConfig()
57 c.Assert(err, IsNil)45 c.Assert(err, IsNil)
58 attrs := cfg.AllAttrs()46 attrs := cfg.AllAttrs()
@@ -64,7 +52,7 @@
64 c.Assert(err, IsNil)52 c.Assert(err, IsNil)
65}53}
6654
67func (s *upgraderSuite) uploadTools(c *C, vers version.Binary) *state.Tools {55func (s *UpgraderSuite) uploadTools(c *C, vers version.Binary) *state.Tools {
68 tgz := coretesting.TarGz(56 tgz := coretesting.TarGz(
69 coretesting.NewTarFile("juju", 0777, "juju contents "+vers.String()),57 coretesting.NewTarFile("juju", 0777, "juju contents "+vers.String()),
70 coretesting.NewTarFile("jujuc", 0777, "jujuc contents "+vers.String()),58 coretesting.NewTarFile("jujuc", 0777, "jujuc contents "+vers.String()),
@@ -131,7 +119,7 @@
131},119},
132}120}
133121
134func (s *upgraderSuite) TestUpgrader(c *C) {122func (s *UpgraderSuite) TestUpgrader(c *C) {
135 dataDir, currentTools := s.primeTools(c, version.MustParseBinary("2.0.0-foo-bar"))123 dataDir, currentTools := s.primeTools(c, version.MustParseBinary("2.0.0-foo-bar"))
136 // Remove the tools from the storage so that we're sure that the124 // Remove the tools from the storage so that we're sure that the
137 // uploader isn't trying to fetch them.125 // uploader isn't trying to fetch them.
@@ -171,8 +159,10 @@
171 }159 }
172 if test.propose != "" {160 if test.propose != "" {
173 s.proposeVersion(c, version.MustParse(test.propose), test.devVersion)161 s.proposeVersion(c, version.MustParse(test.propose), test.devVersion)
162 s.State.StartSync()
174 }163 }
175 if test.upgradeTo == "" {164 if test.upgradeTo == "" {
165 s.State.StartSync()
176 assertNothingHappens(c, upgraderDone)166 assertNothingHappens(c, upgraderDone)
177 } else {167 } else {
178 tools := uploaded[version.MustParse(test.upgradeTo)]168 tools := uploaded[version.MustParse(test.upgradeTo)]
@@ -229,7 +219,7 @@
229},219},
230}220}
231221
232func (s *upgraderSuite) TestDelayedStop(c *C) {222func (s *UpgraderSuite) TestDelayedStop(c *C) {
233 defer dummy.SetStorageDelay(0)223 defer dummy.SetStorageDelay(0)
234 dataDir, tools := s.primeTools(c, version.MustParseBinary("2.0.3-foo-bar"))224 dataDir, tools := s.primeTools(c, version.MustParseBinary("2.0.3-foo-bar"))
235 s.uploadTools(c, version.MustParseBinary("2.0.5-foo-bar"))225 s.uploadTools(c, version.MustParseBinary("2.0.5-foo-bar"))
@@ -259,12 +249,12 @@
259 }249 }
260}250}
261251
262func (s *upgraderSuite) poisonVersion(vers version.Binary) {252func (s *UpgraderSuite) poisonVersion(vers version.Binary) {
263 path := environs.ToolsStoragePath(vers)253 path := environs.ToolsStoragePath(vers)
264 dummy.Poison(s.Conn.Environ.Storage(), path, fmt.Errorf("poisoned file"))254 dummy.Poison(s.Conn.Environ.Storage(), path, fmt.Errorf("poisoned file"))
265}255}
266256
267func (s *upgraderSuite) removeVersion(c *C, vers version.Binary) {257func (s *UpgraderSuite) removeVersion(c *C, vers version.Binary) {
268 path := environs.ToolsStoragePath(vers)258 path := environs.ToolsStoragePath(vers)
269 err := s.Conn.Environ.Storage().Remove(path)259 err := s.Conn.Environ.Storage().Remove(path)
270 c.Assert(err, IsNil)260 c.Assert(err, IsNil)
@@ -272,7 +262,7 @@
272262
273// primeTools sets up the current version of the tools to vers and263// primeTools sets up the current version of the tools to vers and
274// makes sure that they're available in the returned dataDir.264// makes sure that they're available in the returned dataDir.
275func (s *upgraderSuite) primeTools(c *C, vers version.Binary) (dataDir string, tools *state.Tools) {265func (s *UpgraderSuite) primeTools(c *C, vers version.Binary) (dataDir string, tools *state.Tools) {
276 dataDir = c.MkDir()266 dataDir = c.MkDir()
277 // Set up the current version and tools.267 // Set up the current version and tools.
278 version.Current = vers268 version.Current = vers

Subscribers

People subscribed via source and target branches