juju-core

Merge lp:~jameinel/juju-core/set-agent-version into lp:~go-bot/juju-core/trunk

  1. set-agent-version
  2. Merge into trunk
Proposed by John A Meinel
Status: Merged
Approved by: John A Meinel
Approved revision: no longer in the source branch.
Merged at revision: 1390
Proposed branch: lp:~jameinel/juju-core/set-agent-version
Merge into: lp:~go-bot/juju-core/trunk
Diff against target: 119 lines (+29/-33)
3 files modified
environs/jujutest/livetests.go (+2/-15)
state/state_test.go (+4/-18)
state/testing/agent.go (+23/-0)
To merge this branch: bzr merge lp:~jameinel/juju-core/set-agent-version
Reviewer Review Type Date Requested Status
Juju Engineering Pending
Review via email: mp+173175@code.launchpad.net

Commit message

state/testing: pull out SetAgentVersion

There were two test cases that wanted SetAgentVersion, and I wanted to add a
third. So this is a trivial "pull it out into a common testing helper".

https://codereview.appspot.com/10962043/

Description of the change

state/testing: pull out SetAgentVersion

There were two test cases that wanted SetAgentVersion, and I wanted to add a
third. So this is a trivial "pull it out into a common testing helper".

https://codereview.appspot.com/10962043/

To post a comment you must log in.
Revision history for this message
John A Meinel (jameinel) wrote :
Download full text (4.7 KiB)

Reviewers: mp+173175_code.launchpad.net,

Message:
Please take a look.

Description:
state/testing: pull out SetAgentVersion

There were two test cases that wanted SetAgentVersion, and I wanted to
add a
third. So this is a trivial "pull it out into a common testing helper".

https://code.launchpad.net/~jameinel/juju-core/set-agent-version/+merge/173175

(do not edit description out of merge proposal)

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

Affected files:
   A [revision details]
   M environs/jujutest/livetests.go
   M state/state_test.go
   A state/testing/agent.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-20130704233548-3tyyqq9g9ojkmbn7
+New revision: <email address hidden>

Index: state/state_test.go
=== modified file 'state/state_test.go'
--- state/state_test.go 2013-07-04 14:12:18 +0000
+++ state/state_test.go 2013-07-05 07:59:53 +0000
@@ -1093,23 +1093,9 @@
   assertChange(attrs{"fancy-new-key": "arbitrary-value"})
  }

-// setAgentVersion sets the current agent version in the state's
-// environment configuration.
-func setAgentVersion(st *state.State, vers version.Number) error {
- cfg, err := st.EnvironConfig()
- if err != nil {
- return err
- }
- cfg, err = cfg.Apply(map[string]interface{}{"agent-version":
vers.String()})
- if err != nil {
- panic(fmt.Errorf("config refused agent-version: %v", err))
- }
- return st.SetEnvironConfig(cfg)
-}
-
  func (s *StateSuite) TestWatchForEnvironConfigChanges(c *C) {
   cur := version.Current.Number
- err := setAgentVersion(s.State, cur)
+ err := statetesting.SetAgentVersion(s.State, cur)
   c.Assert(err, IsNil)
   w := s.State.WatchForEnvironConfigChanges()
   defer statetesting.AssertStop(c, w)
@@ -1121,17 +1107,17 @@
   // Multiple changes will only result in a single change notification
   newVersion := cur
   newVersion.Minor += 1
- err = setAgentVersion(s.State, newVersion)
+ err = statetesting.SetAgentVersion(s.State, newVersion)
   c.Assert(err, IsNil)

   newerVersion := newVersion
   newerVersion.Minor += 1
- err = setAgentVersion(s.State, newerVersion)
+ err = statetesting.SetAgentVersion(s.State, newerVersion)
   c.Assert(err, IsNil)
   wc.AssertOneChange()

   // Setting it to the same value does not trigger a change notification
- err = setAgentVersion(s.State, newerVersion)
+ err = statetesting.SetAgentVersion(s.State, newerVersion)
   c.Assert(err, IsNil)
   wc.AssertNoChange()
  }

Index: environs/jujutest/livetests.go
=== modified file 'environs/jujutest/livetests.go'
--- environs/jujutest/livetests.go 2013-07-04 14:12:18 +0000
+++ environs/jujutest/livetests.go 2013-07-05 08:03:59 +0000
@@ -20,6 +20,7 @@
   "launchpad.net/juju-core/juju/testing"
   "launchpad.net/juju-core/state"
   "launchpad.net/juju-core/state/api"
+ statetesting "launchpad.net/juju-core/state/testing"
   coretesting "launchpad.net/juju-core/testing"
   . "launchpad.net/juju-core/testing/checkers"
   "launchpad.net/juju-core/utils"
@@ -581,7 +582,7 @@

   // Check that the put version reall...

Read more...

Revision history for this message
Raphaël Badin (rvb) wrote :

LGTM trivial

If it compiles, ship it ;)

https://codereview.appspot.com/10962043/

Revision history for this message
Frank Mueller (themue) wrote :

LGTM, simple and clean.

https://codereview.appspot.com/10962043/

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'environs/jujutest/livetests.go'
2--- environs/jujutest/livetests.go 2013-07-04 14:12:18 +0000
3+++ environs/jujutest/livetests.go 2013-07-05 10:23:33 +0000
4@@ -20,6 +20,7 @@
5 "launchpad.net/juju-core/juju/testing"
6 "launchpad.net/juju-core/state"
7 "launchpad.net/juju-core/state/api"
8+ statetesting "launchpad.net/juju-core/state/testing"
9 coretesting "launchpad.net/juju-core/testing"
10 . "launchpad.net/juju-core/testing/checkers"
11 "launchpad.net/juju-core/utils"
12@@ -581,7 +582,7 @@
13
14 // Check that the put version really is the version we expect.
15 c.Assert(upgradeTools.Binary, Equals, newVersion)
16- err = setAgentVersion(conn.State, newVersion.Number)
17+ err = statetesting.SetAgentVersion(conn.State, newVersion.Number)
18 c.Assert(err, IsNil)
19
20 for i, w := range waiters {
21@@ -592,20 +593,6 @@
22 }
23 }
24
25-// setAgentVersion sets the current agent version in the state's
26-// environment configuration.
27-func setAgentVersion(st *state.State, vers version.Number) error {
28- cfg, err := st.EnvironConfig()
29- if err != nil {
30- return err
31- }
32- cfg, err = cfg.Apply(map[string]interface{}{"agent-version": vers.String()})
33- if err != nil {
34- panic(fmt.Errorf("config refused agent-version: %v", err))
35- }
36- return st.SetEnvironConfig(cfg)
37-}
38-
39 var waitAgent = utils.AttemptStrategy{
40 Total: 30 * time.Second,
41 Delay: 1 * time.Second,
42
43=== modified file 'state/state_test.go'
44--- state/state_test.go 2013-07-04 14:12:18 +0000
45+++ state/state_test.go 2013-07-05 10:23:33 +0000
46@@ -1093,23 +1093,9 @@
47 assertChange(attrs{"fancy-new-key": "arbitrary-value"})
48 }
49
50-// setAgentVersion sets the current agent version in the state's
51-// environment configuration.
52-func setAgentVersion(st *state.State, vers version.Number) error {
53- cfg, err := st.EnvironConfig()
54- if err != nil {
55- return err
56- }
57- cfg, err = cfg.Apply(map[string]interface{}{"agent-version": vers.String()})
58- if err != nil {
59- panic(fmt.Errorf("config refused agent-version: %v", err))
60- }
61- return st.SetEnvironConfig(cfg)
62-}
63-
64 func (s *StateSuite) TestWatchForEnvironConfigChanges(c *C) {
65 cur := version.Current.Number
66- err := setAgentVersion(s.State, cur)
67+ err := statetesting.SetAgentVersion(s.State, cur)
68 c.Assert(err, IsNil)
69 w := s.State.WatchForEnvironConfigChanges()
70 defer statetesting.AssertStop(c, w)
71@@ -1121,17 +1107,17 @@
72 // Multiple changes will only result in a single change notification
73 newVersion := cur
74 newVersion.Minor += 1
75- err = setAgentVersion(s.State, newVersion)
76+ err = statetesting.SetAgentVersion(s.State, newVersion)
77 c.Assert(err, IsNil)
78
79 newerVersion := newVersion
80 newerVersion.Minor += 1
81- err = setAgentVersion(s.State, newerVersion)
82+ err = statetesting.SetAgentVersion(s.State, newerVersion)
83 c.Assert(err, IsNil)
84 wc.AssertOneChange()
85
86 // Setting it to the same value does not trigger a change notification
87- err = setAgentVersion(s.State, newerVersion)
88+ err = statetesting.SetAgentVersion(s.State, newerVersion)
89 c.Assert(err, IsNil)
90 wc.AssertNoChange()
91 }
92
93=== added file 'state/testing/agent.go'
94--- state/testing/agent.go 1970-01-01 00:00:00 +0000
95+++ state/testing/agent.go 2013-07-05 10:23:33 +0000
96@@ -0,0 +1,23 @@
97+// Copyright 2012, 2013 Canonical Ltd.
98+// Licensed under the AGPLv3, see LICENCE file for details.
99+
100+package testing
101+
102+import (
103+ "launchpad.net/juju-core/state"
104+ "launchpad.net/juju-core/version"
105+)
106+
107+// SetAgentVersion sets the current agent version in the state's
108+// environment configuration.
109+func SetAgentVersion(st *state.State, vers version.Number) error {
110+ cfg, err := st.EnvironConfig()
111+ if err != nil {
112+ return err
113+ }
114+ cfg, err = cfg.Apply(map[string]interface{}{"agent-version": vers.String()})
115+ if err != nil {
116+ return err
117+ }
118+ return st.SetEnvironConfig(cfg)
119+}

Subscribers

People subscribed via source and target branches

to status/vote changes: