Merge lp:~thumper/juju-core/invalid-state-rename into lp:~go-bot/juju-core/trunk

Proposed by Tim Penhey
Status: Merged
Approved by: Tim Penhey
Approved revision: no longer in the source branch.
Merged at revision: 1293
Proposed branch: lp:~thumper/juju-core/invalid-state-rename
Merge into: lp:~go-bot/juju-core/trunk
Diff against target: 174 lines (+30/-29)
6 files modified
environs/ec2/live_test.go (+1/-1)
environs/jujutest/livetests.go (+2/-2)
juju/testing/conn.go (+6/-6)
state/export_test.go (+1/-16)
state/initialize_test.go (+4/-4)
testing/environ.go (+16/-0)
To merge this branch: bzr merge lp:~thumper/juju-core/invalid-state-rename
Reviewer Review Type Date Requested Status
Juju Engineering Pending
Review via email: mp+169962@code.launchpad.net

Commit message

Some testing code refactoring.

Move state.TestingEnvironConfig into testing/environ.go and
rename to EnvironConfig, so usage looks like testing.EnvironConfig.
Also renamed InvalidStateInfo and InvalidAPIInfo to FakeStateInfo
and FakeAIInfo.

Description of the change

Some testing code refactoring.

Move state.TestingEnvironConfig into testing/environ.go and
rename to EnvironConfig, so usage looks like testing.EnvironConfig.
Also renamed InvalidStateInfo and InvalidApiInfo to FakeStateInfo
and FakeApiInfo.

https://codereview.appspot.com/10344044/

To post a comment you must log in.
Revision history for this message
Tim Penhey (thumper) wrote :
Download full text (7.6 KiB)

Reviewers: mp+169962_code.launchpad.net,

Message:
Please take a look.

Description:
Some testing code refactoring.

Move state.TestingEnvironConfig into testing/environ.go and
rename to EnvironConfig, so usage looks like testing.EnvironConfig.
Also renamed InvalidStateInfo and InvalidApiInfo to FakeStateInfo
and FakeApiInfo.

https://code.launchpad.net/~thumper/juju-core/invalid-state-rename/+merge/169962

(do not edit description out of merge proposal)

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

Affected files:
   A [revision details]
   M environs/ec2/live_test.go
   M environs/jujutest/livetests.go
   M juju/testing/conn.go
   M state/export_test.go
   M state/initialize_test.go
   M testing/environ.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: state/export_test.go
=== modified file 'state/export_test.go'
--- state/export_test.go 2013-06-14 00:24:13 +0000
+++ state/export_test.go 2013-06-17 22:52:36 +0000
@@ -15,27 +15,12 @@
   "path/filepath"
  )

-// TestingEnvironConfig returns a default environment configuration.
-func TestingEnvironConfig(c *C) *config.Config {
- cfg, err := config.New(map[string]interface{}{
- "type": "test",
- "name": "test-name",
- "default-series": "test-series",
- "authorized-keys": "test-keys",
- "agent-version": "9.9.9.9",
- "ca-cert": testing.CACert,
- "ca-private-key": "",
- })
- c.Assert(err, IsNil)
- return cfg
-}
-
  // TestingInitialize initializes the state and returns it. If state was not
  // already initialized, and cfg is nil, the minimal default environment
  // configuration will be used.
  func TestingInitialize(c *C, cfg *config.Config) *State {
   if cfg == nil {
- cfg = TestingEnvironConfig(c)
+ cfg = testing.EnvironConfig(c)
   }
   st, err := Initialize(TestingStateInfo(), cfg, TestingDialOpts())
   c.Assert(err, IsNil)

Index: state/initialize_test.go
=== modified file 'state/initialize_test.go'
--- state/initialize_test.go 2013-05-30 00:47:30 +0000
+++ state/initialize_test.go 2013-06-17 22:52:36 +0000
@@ -52,7 +52,7 @@
   _, err = s.State.EnvironConstraints()
   c.Assert(errors.IsNotFoundError(err), Equals, true)

- cfg := state.TestingEnvironConfig(c)
+ cfg := testing.EnvironConfig(c)
   initial := cfg.AllAttrs()
   st, err := state.Initialize(state.TestingStateInfo(), cfg,
state.TestingDialOpts())
   c.Assert(err, IsNil)
@@ -74,7 +74,7 @@
  }

  func (s *InitializeSuite) TestDoubleInitializeConfig(c *C) {
- cfg := state.TestingEnvironConfig(c)
+ cfg := testing.EnvironConfig(c)
   initial := cfg.AllAttrs()
   st := state.TestingInitialize(c, cfg)
   st.Close()
@@ -96,7 +96,7 @@

  func (s *InitializeSuite) TestEnvironConfigWithAdminSecret(c *C) {
   // admin-secret blocks Initialize.
- good := state.TestingEnvironConfig(c)
+ good := testing.EnvironConfig(c)
   bad, err := good.Apply(map[string]interface{}{"admin-secret": "foo"})

   _, err...

Read more...

Revision history for this message
Ian Booth (wallyworld) wrote :

LGTM, niiiice

https://codereview.appspot.com/10344044/diff/1/environs/ec2/live_test.go
File environs/ec2/live_test.go (right):

https://codereview.appspot.com/10344044/diff/1/environs/ec2/live_test.go#newcode133
environs/ec2/live_test.go:133: inst, err := t.Env.StartInstance("31",
"fake_nonce", config.DefaultSeries, cons, testing.FakeStateInfo("31"),
testing.FakeAPIInfo("31"))
On 2013/06/17 23:42:13, dfc wrote:
> s/Fake/Mock, whatever, personal preference.

+1 :-)

https://codereview.appspot.com/10344044/diff/1/state/initialize_test.go
File state/initialize_test.go (left):

https://codereview.appspot.com/10344044/diff/1/state/initialize_test.go#oldcode55
state/initialize_test.go:55: cfg := state.TestingEnvironConfig(c)
On 2013/06/17 23:42:13, dfc wrote:
> excellent, state should know nothing about the test mocks.

+100 \o/

https://codereview.appspot.com/10344044/

Revision history for this message
Tim Penhey (thumper) wrote :

https://codereview.appspot.com/10344044/diff/1/environs/ec2/live_test.go
File environs/ec2/live_test.go (right):

https://codereview.appspot.com/10344044/diff/1/environs/ec2/live_test.go#newcode133
environs/ec2/live_test.go:133: inst, err := t.Env.StartInstance("31",
"fake_nonce", config.DefaultSeries, cons, testing.FakeStateInfo("31"),
testing.FakeAPIInfo("31"))
On 2013/06/18 01:58:25, wallyworld wrote:
> On 2013/06/17 23:42:13, dfc wrote:
> > s/Fake/Mock, whatever, personal preference.

> +1 :-)

Going to stick with Fakes here as Mocks are normally different
implementations with internal assertions.

https://codereview.appspot.com/10344044/diff/1/testing/environ.go
File testing/environ.go (right):

https://codereview.appspot.com/10344044/diff/1/testing/environ.go#newcode14
testing/environ.go:14: // EnvironConfig returns a default environment
configuration.
On 2013/06/17 23:42:13, dfc wrote:
> // EnvironConfig returns a default environment configuration suitable
for
> testing.

Done.

https://codereview.appspot.com/10344044/

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'environs/ec2/live_test.go'
2--- environs/ec2/live_test.go 2013-06-16 23:07:00 +0000
3+++ environs/ec2/live_test.go 2013-06-18 03:17:26 +0000
4@@ -130,7 +130,7 @@
5
6 func (t *LiveTests) TestStartInstanceConstraints(c *C) {
7 cons := constraints.MustParse("mem=2G")
8- inst, err := t.Env.StartInstance("31", "fake_nonce", config.DefaultSeries, cons, testing.InvalidStateInfo("31"), testing.InvalidAPIInfo("31"))
9+ inst, err := t.Env.StartInstance("31", "fake_nonce", config.DefaultSeries, cons, testing.FakeStateInfo("31"), testing.FakeAPIInfo("31"))
10 c.Assert(err, IsNil)
11 defer t.Env.StopInstances([]instance.Instance{inst})
12 ec2inst := ec2.InstanceEC2(inst)
13
14=== modified file 'environs/jujutest/livetests.go'
15--- environs/jujutest/livetests.go 2013-06-17 21:38:52 +0000
16+++ environs/jujutest/livetests.go 2013-06-18 03:17:26 +0000
17@@ -713,7 +713,7 @@
18 // available platform. The first thing start instance should do is find
19 // appropriate tools.
20 func (t *LiveTests) TestStartInstanceOnUnknownPlatform(c *C) {
21- inst, err := t.Env.StartInstance("4", "fake_nonce", "unknownseries", constraints.Value{}, testing.InvalidStateInfo("4"), testing.InvalidAPIInfo("4"))
22+ inst, err := t.Env.StartInstance("4", "fake_nonce", "unknownseries", constraints.Value{}, testing.FakeStateInfo("4"), testing.FakeAPIInfo("4"))
23 if inst != nil {
24 err := t.Env.StopInstances([]instance.Instance{inst})
25 c.Check(err, IsNil)
26@@ -726,7 +726,7 @@
27
28 // Check that we can't start an instance with an empty nonce value.
29 func (t *LiveTests) TestStartInstanceWithEmptyNonceFails(c *C) {
30- inst, err := t.Env.StartInstance("4", "", config.DefaultSeries, constraints.Value{}, testing.InvalidStateInfo("4"), testing.InvalidAPIInfo("4"))
31+ inst, err := t.Env.StartInstance("4", "", config.DefaultSeries, constraints.Value{}, testing.FakeStateInfo("4"), testing.FakeAPIInfo("4"))
32 if inst != nil {
33 err := t.Env.StopInstances([]instance.Instance{inst})
34 c.Check(err, IsNil)
35
36=== modified file 'juju/testing/conn.go'
37--- juju/testing/conn.go 2013-06-14 01:02:53 +0000
38+++ juju/testing/conn.go 2013-06-18 03:17:26 +0000
39@@ -52,10 +52,10 @@
40 oldJujuHome string
41 }
42
43-// InvalidStateInfo holds information about no state - it will always
44+// FakeStateInfo holds information about no state - it will always
45 // give an error when connected to. The machine id gives the machine id
46 // of the machine to be started.
47-func InvalidStateInfo(machineId string) *state.Info {
48+func FakeStateInfo(machineId string) *state.Info {
49 return &state.Info{
50 Addrs: []string{"0.1.2.3:1234"},
51 Tag: state.MachineTag(machineId),
52@@ -64,10 +64,10 @@
53 }
54 }
55
56-// InvalidAPIInfo holds information about no state - it will always
57+// FakeAPIInfo holds information about no state - it will always
58 // give an error when connected to. The machine id gives the machine id
59 // of the machine to be started.
60-func InvalidAPIInfo(machineId string) *api.Info {
61+func FakeAPIInfo(machineId string) *api.Info {
62 return &api.Info{
63 Addrs: []string{"0.1.2.3:1234"},
64 Tag: state.MachineTag(machineId),
65@@ -85,8 +85,8 @@
66 "fake_nonce",
67 series,
68 constraints.Value{},
69- InvalidStateInfo(machineId),
70- InvalidAPIInfo(machineId),
71+ FakeStateInfo(machineId),
72+ FakeAPIInfo(machineId),
73 )
74 c.Assert(err, IsNil)
75 return inst
76
77=== modified file 'state/export_test.go'
78--- state/export_test.go 2013-06-14 00:24:13 +0000
79+++ state/export_test.go 2013-06-18 03:17:26 +0000
80@@ -15,27 +15,12 @@
81 "path/filepath"
82 )
83
84-// TestingEnvironConfig returns a default environment configuration.
85-func TestingEnvironConfig(c *C) *config.Config {
86- cfg, err := config.New(map[string]interface{}{
87- "type": "test",
88- "name": "test-name",
89- "default-series": "test-series",
90- "authorized-keys": "test-keys",
91- "agent-version": "9.9.9.9",
92- "ca-cert": testing.CACert,
93- "ca-private-key": "",
94- })
95- c.Assert(err, IsNil)
96- return cfg
97-}
98-
99 // TestingInitialize initializes the state and returns it. If state was not
100 // already initialized, and cfg is nil, the minimal default environment
101 // configuration will be used.
102 func TestingInitialize(c *C, cfg *config.Config) *State {
103 if cfg == nil {
104- cfg = TestingEnvironConfig(c)
105+ cfg = testing.EnvironConfig(c)
106 }
107 st, err := Initialize(TestingStateInfo(), cfg, TestingDialOpts())
108 c.Assert(err, IsNil)
109
110=== modified file 'state/initialize_test.go'
111--- state/initialize_test.go 2013-05-30 00:47:30 +0000
112+++ state/initialize_test.go 2013-06-18 03:17:26 +0000
113@@ -52,7 +52,7 @@
114 _, err = s.State.EnvironConstraints()
115 c.Assert(errors.IsNotFoundError(err), Equals, true)
116
117- cfg := state.TestingEnvironConfig(c)
118+ cfg := testing.EnvironConfig(c)
119 initial := cfg.AllAttrs()
120 st, err := state.Initialize(state.TestingStateInfo(), cfg, state.TestingDialOpts())
121 c.Assert(err, IsNil)
122@@ -74,7 +74,7 @@
123 }
124
125 func (s *InitializeSuite) TestDoubleInitializeConfig(c *C) {
126- cfg := state.TestingEnvironConfig(c)
127+ cfg := testing.EnvironConfig(c)
128 initial := cfg.AllAttrs()
129 st := state.TestingInitialize(c, cfg)
130 st.Close()
131@@ -96,7 +96,7 @@
132
133 func (s *InitializeSuite) TestEnvironConfigWithAdminSecret(c *C) {
134 // admin-secret blocks Initialize.
135- good := state.TestingEnvironConfig(c)
136+ good := testing.EnvironConfig(c)
137 bad, err := good.Apply(map[string]interface{}{"admin-secret": "foo"})
138
139 _, err = state.Initialize(state.TestingStateInfo(), bad, state.TestingDialOpts())
140@@ -116,7 +116,7 @@
141
142 func (s *InitializeSuite) TestEnvironConfigWithoutAgentVersion(c *C) {
143 // admin-secret blocks Initialize.
144- good := state.TestingEnvironConfig(c)
145+ good := testing.EnvironConfig(c)
146 attrs := good.AllAttrs()
147 delete(attrs, "agent-version")
148 bad, err := config.New(attrs)
149
150=== modified file 'testing/environ.go'
151--- testing/environ.go 2013-05-10 20:55:57 +0000
152+++ testing/environ.go 2013-06-18 03:17:26 +0000
153@@ -11,6 +11,22 @@
154 "path/filepath"
155 )
156
157+// EnvironConfig returns a default environment configuration suitable for
158+// testing.
159+func EnvironConfig(c *C) *config.Config {
160+ cfg, err := config.New(map[string]interface{}{
161+ "type": "test",
162+ "name": "test-name",
163+ "default-series": "test-series",
164+ "authorized-keys": "test-keys",
165+ "agent-version": "9.9.9.9",
166+ "ca-cert": CACert,
167+ "ca-private-key": "",
168+ })
169+ c.Assert(err, IsNil)
170+ return cfg
171+}
172+
173 const SampleEnvName = "erewhemos"
174 const EnvDefault = "default:\n " + SampleEnvName + "\n"
175

Subscribers

People subscribed via source and target branches

to status/vote changes: