juju-core

Merge lp:~axwalk/juju-core/lp1298133-peergrouper-tests-maporder into lp:~go-bot/juju-core/trunk

  1. lp1298133-peergrouper-tests-maporder
  2. Merge into trunk
Proposed by Andrew Wilkins
Status: Merged
Approved by: Andrew Wilkins
Approved revision: no longer in the source branch.
Merged at revision: 2496
Proposed branch: lp:~axwalk/juju-core/lp1298133-peergrouper-tests-maporder
Merge into: lp:~go-bot/juju-core/trunk
Diff against target: 101 lines (+16/-9)
2 files modified
worker/peergrouper/desired_test.go (+7/-0)
worker/peergrouper/worker_test.go (+9/-9)
To merge this branch: bzr merge lp:~axwalk/juju-core/lp1298133-peergrouper-tests-maporder
Reviewer Review Type Date Requested Status
Juju Engineering Pending
Review via email: mp+213002@code.launchpad.net

Commit message

worker/peergrouper: don't depend on map order

Tests were depending on map order being
predictable. Changed code to sort members
before comparing.

Fixes lp:1298133

https://codereview.appspot.com/81360043/

Description of the change

worker/peergrouper: don't depend on map order

Tests were depending on map order being
predictable. Changed code to sort members
before comparing.

Fixes lp:1298133

https://codereview.appspot.com/81360043/

To post a comment you must log in.
Revision history for this message
Andrew Wilkins (axwalk) wrote :
Download full text (4.5 KiB)

Reviewers: mp+213002_code.launchpad.net,

Message:
Please take a look.

Description:
worker/peergrouper: don't depend on map order

Tests were depending on map order being
predictable. Changed code to sort members
before comparing.

Fixes lp:1298133

https://code.launchpad.net/~axwalk/juju-core/lp1298133-peergrouper-tests-maporder/+merge/213002

(do not edit description out of merge proposal)

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

Affected files (+18, -9 lines):
   A [revision details]
   M worker/peergrouper/desired_test.go
   M worker/peergrouper/worker_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-20140327045522-cukovtfmelnxty3x
+New revision: <email address hidden>

Index: worker/peergrouper/desired_test.go
=== modified file 'worker/peergrouper/desired_test.go'
--- worker/peergrouper/desired_test.go 2014-03-18 18:07:38 +0000
+++ worker/peergrouper/desired_test.go 2014-03-27 08:22:09 +0000
@@ -404,6 +404,13 @@
   return descrs
  }

+func assertMembers(c *gc.C, obtained interface{}, expected
[]replicaset.Member) {
+ c.Assert(obtained, gc.FitsTypeOf, []replicaset.Member{})
+ sort.Sort(membersById(obtained.([]replicaset.Member)))
+ sort.Sort(membersById(expected))
+ c.Assert(obtained, jc.DeepEquals, expected)
+}
+
  type membersById []replicaset.Member

  func (l membersById) Len() int { return len(l) }

Index: worker/peergrouper/worker_test.go
=== modified file 'worker/peergrouper/worker_test.go'
--- worker/peergrouper/worker_test.go 2014-03-25 16:24:47 +0000
+++ worker/peergrouper/worker_test.go 2014-03-27 08:22:09 +0000
@@ -124,7 +124,7 @@

   memberWatcher := st.session.members.Watch()
   mustNext(c, memberWatcher)
- c.Assert(memberWatcher.Value(), jc.DeepEquals, mkMembers("0v"))
+ assertMembers(c, memberWatcher.Value(), mkMembers("0v"))

   logger.Infof("starting worker")
   w := newWorker(st, noPublisher{})
@@ -134,14 +134,14 @@

   // Wait for the worker to set the initial members.
   mustNext(c, memberWatcher)
- c.Assert(memberWatcher.Value(), jc.DeepEquals, mkMembers("0v 1 2"))
+ assertMembers(c, memberWatcher.Value(), mkMembers("0v 1 2"))

   // Update the status of the new members
   // and check that they become voting.
   c.Logf("updating new member status")
   st.session.setStatus(mkStatuses("0p 1s 2s"))
   mustNext(c, memberWatcher)
- c.Assert(memberWatcher.Value(), jc.DeepEquals, mkMembers("0v 1v 2v"))
+ assertMembers(c, memberWatcher.Value(), mkMembers("0v 1v 2v"))

   c.Logf("adding another machine")
   // Add another machine.
@@ -151,7 +151,7 @@

   c.Logf("waiting for new member to be added")
   mustNext(c, memberWatcher)
- c.Assert(memberWatcher.Value(), jc.DeepEquals, mkMembers("0v 1v 2v 3"))
+ assertMembers(c, memberWatcher.Value(), mkMembers("0v 1v 2v 3"))

   // Remove vote from an existing member;
   // and give it to the new machine.
@@ -167,7 +167,7 @@
   // old machine loses it.
   c.Logf("waiting for vote switch")
   mustNext(c, memberWatcher)
- c.Assert(memberWatcher.Value(), ...

Read more...

Revision history for this message
Dave Cheney (dave-cheney) wrote :
Download full text (5.3 KiB)

LGTM.

On Thu, Mar 27, 2014 at 7:30 PM, Andrew Wilkins <
<email address hidden>> wrote:

> Reviewers: mp+213002_code.launchpad.net,
>
> Message:
> Please take a look.
>
> Description:
> worker/peergrouper: don't depend on map order
>
> Tests were depending on map order being
> predictable. Changed code to sort members
> before comparing.
>
> Fixes lp:1298133
>
>
> https://code.launchpad.net/~axwalk/juju-core/lp1298133-peergrouper-tests-maporder/+merge/213002
>
> (do not edit description out of merge proposal)
>
>
> Please review this at https://codereview.appspot.com/81360043/
>
> Affected files (+18, -9 lines):
> A [revision details]
> M worker/peergrouper/desired_test.go
> M worker/peergrouper/worker_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-20140327045522-cukovtfmelnxty3x
> +New revision: <email address hidden>
>
> Index: worker/peergrouper/desired_test.go
> === modified file 'worker/peergrouper/desired_test.go'
> --- worker/peergrouper/desired_test.go 2014-03-18 18:07:38 +0000
> +++ worker/peergrouper/desired_test.go 2014-03-27 08:22:09 +0000
> @@ -404,6 +404,13 @@
> return descrs
> }
>
> +func assertMembers(c *gc.C, obtained interface{}, expected
> []replicaset.Member) {
> + c.Assert(obtained, gc.FitsTypeOf, []replicaset.Member{})
> + sort.Sort(membersById(obtained.([]replicaset.Member)))
> + sort.Sort(membersById(expected))
> + c.Assert(obtained, jc.DeepEquals, expected)
> +}
> +
> type membersById []replicaset.Member
>
> func (l membersById) Len() int { return len(l) }
>
>
> Index: worker/peergrouper/worker_test.go
> === modified file 'worker/peergrouper/worker_test.go'
> --- worker/peergrouper/worker_test.go 2014-03-25 16:24:47 +0000
> +++ worker/peergrouper/worker_test.go 2014-03-27 08:22:09 +0000
> @@ -124,7 +124,7 @@
>
> memberWatcher := st.session.members.Watch()
> mustNext(c, memberWatcher)
> - c.Assert(memberWatcher.Value(), jc.DeepEquals, mkMembers("0v"))
> + assertMembers(c, memberWatcher.Value(), mkMembers("0v"))
>
> logger.Infof("starting worker")
> w := newWorker(st, noPublisher{})
> @@ -134,14 +134,14 @@
>
> // Wait for the worker to set the initial members.
> mustNext(c, memberWatcher)
> - c.Assert(memberWatcher.Value(), jc.DeepEquals, mkMembers("0v 1 2"))
> + assertMembers(c, memberWatcher.Value(), mkMembers("0v 1 2"))
>
> // Update the status of the new members
> // and check that they become voting.
> c.Logf("updating new member status")
> st.session.setStatus(mkStatuses("0p 1s 2s"))
> mustNext(c, memberWatcher)
> - c.Assert(memberWatcher.Value(), jc.DeepEquals, mkMembers("0v 1v
> 2v"))
> + assertMembers(c, memberWatcher.Value(), mkMembers("0v 1v 2v"))
>
> c.Logf("adding another machine")
> // Add another machine.
> @@ -151,7 +151,7 @@
>
> c.Logf("waiting for new member to be added")
> ...

Read more...

Revision history for this message
Go Bot (go-bot) wrote :
Download full text (10.1 KiB)

The attempt to merge lp:~axwalk/juju-core/lp1298133-peergrouper-tests-maporder into lp:juju-core failed. Below is the output from the failed tests.

ok launchpad.net/juju-core 0.013s
ok launchpad.net/juju-core/agent 1.118s
ok launchpad.net/juju-core/agent/mongo 0.548s
ok launchpad.net/juju-core/agent/tools 0.191s
ok launchpad.net/juju-core/bzr 5.206s
ok launchpad.net/juju-core/cert 2.383s
ok launchpad.net/juju-core/charm 0.421s
? launchpad.net/juju-core/charm/hooks [no test files]
? launchpad.net/juju-core/charm/testing [no test files]
ok launchpad.net/juju-core/cloudinit 0.027s
ok launchpad.net/juju-core/cloudinit/sshinit 0.780s
ok launchpad.net/juju-core/cmd 0.167s
ok launchpad.net/juju-core/cmd/charm-admin 0.737s
? launchpad.net/juju-core/cmd/charmd [no test files]
? launchpad.net/juju-core/cmd/charmload [no test files]
ok launchpad.net/juju-core/cmd/juju 203.389s
ok launchpad.net/juju-core/cmd/jujud 63.633s
ok launchpad.net/juju-core/cmd/plugins/juju-metadata 9.398s
? launchpad.net/juju-core/cmd/plugins/juju-restore [no test files]
ok launchpad.net/juju-core/cmd/plugins/local 0.156s
? launchpad.net/juju-core/cmd/plugins/local/juju-local [no test files]
ok launchpad.net/juju-core/constraints 0.028s
ok launchpad.net/juju-core/container 0.046s
ok launchpad.net/juju-core/container/factory 0.045s
ok launchpad.net/juju-core/container/kvm 0.227s
ok launchpad.net/juju-core/container/kvm/mock 0.051s
? launchpad.net/juju-core/container/kvm/testing [no test files]
ok launchpad.net/juju-core/container/lxc 4.347s
? launchpad.net/juju-core/container/lxc/mock [no test files]
? launchpad.net/juju-core/container/lxc/testing [no test files]
? launchpad.net/juju-core/container/testing [no test files]
ok launchpad.net/juju-core/downloader 5.229s
ok launchpad.net/juju-core/environs 2.195s
ok launchpad.net/juju-core/environs/bootstrap 10.134s
ok launchpad.net/juju-core/environs/cloudinit 0.504s
ok launchpad.net/juju-core/environs/config 1.832s
ok launchpad.net/juju-core/environs/configstore 0.038s
ok launchpad.net/juju-core/environs/filestorage 0.025s
ok launchpad.net/juju-core/environs/httpstorage 0.674s
ok launchpad.net/juju-core/environs/imagemetadata 0.461s
? launchpad.net/juju-core/environs/imagemetadata/testing [no test files]
ok launchpad.net/juju-core/environs/instances 0.052s
ok launchpad.net/juju-core/environs/jujutest 0.161s
ok launchpad.net/juju-core/environs/manual 13.150s
ok launchpad.net/juju-core/environs/simplestreams 0.274s
? launchpad.net/juju-core/environs/simplestreams/testing [no test files]
ok launchpad.net/juju-core/environs/sshstorage 0.991s
ok launchpad.net/juju-core/environs/storage 0.800s
ok launchpad.net/juju-core/environs/sync 44.009s
ok launchpad.net/juju-core/environs/testing 0.141s
ok launchpad.net/juju-core/environs/tools 4.561s
? launchpad.net/juju-core/environs/tools/testing [no test files]
ok launchpad.net/juju-core/errors 0.011s
ok launchpad.net/juju-core/instance 0.024s
? launchpad.net/juju-core/instance/testing [no test files]
ok launchpad.net/juju-core/juju 19.874s
ok launchpad.net/juju-core/ju...

Revision history for this message
Wayne Witzel III (wwitzel3) wrote :

LGTM

https://codereview.appspot.com/81360043/

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'worker/peergrouper/desired_test.go'
2--- worker/peergrouper/desired_test.go 2014-03-18 18:07:38 +0000
3+++ worker/peergrouper/desired_test.go 2014-03-27 08:34:16 +0000
4@@ -404,6 +404,13 @@
5 return descrs
6 }
7
8+func assertMembers(c *gc.C, obtained interface{}, expected []replicaset.Member) {
9+ c.Assert(obtained, gc.FitsTypeOf, []replicaset.Member{})
10+ sort.Sort(membersById(obtained.([]replicaset.Member)))
11+ sort.Sort(membersById(expected))
12+ c.Assert(obtained, jc.DeepEquals, expected)
13+}
14+
15 type membersById []replicaset.Member
16
17 func (l membersById) Len() int { return len(l) }
18
19=== modified file 'worker/peergrouper/worker_test.go'
20--- worker/peergrouper/worker_test.go 2014-03-25 16:24:47 +0000
21+++ worker/peergrouper/worker_test.go 2014-03-27 08:34:16 +0000
22@@ -124,7 +124,7 @@
23
24 memberWatcher := st.session.members.Watch()
25 mustNext(c, memberWatcher)
26- c.Assert(memberWatcher.Value(), jc.DeepEquals, mkMembers("0v"))
27+ assertMembers(c, memberWatcher.Value(), mkMembers("0v"))
28
29 logger.Infof("starting worker")
30 w := newWorker(st, noPublisher{})
31@@ -134,14 +134,14 @@
32
33 // Wait for the worker to set the initial members.
34 mustNext(c, memberWatcher)
35- c.Assert(memberWatcher.Value(), jc.DeepEquals, mkMembers("0v 1 2"))
36+ assertMembers(c, memberWatcher.Value(), mkMembers("0v 1 2"))
37
38 // Update the status of the new members
39 // and check that they become voting.
40 c.Logf("updating new member status")
41 st.session.setStatus(mkStatuses("0p 1s 2s"))
42 mustNext(c, memberWatcher)
43- c.Assert(memberWatcher.Value(), jc.DeepEquals, mkMembers("0v 1v 2v"))
44+ assertMembers(c, memberWatcher.Value(), mkMembers("0v 1v 2v"))
45
46 c.Logf("adding another machine")
47 // Add another machine.
48@@ -151,7 +151,7 @@
49
50 c.Logf("waiting for new member to be added")
51 mustNext(c, memberWatcher)
52- c.Assert(memberWatcher.Value(), jc.DeepEquals, mkMembers("0v 1v 2v 3"))
53+ assertMembers(c, memberWatcher.Value(), mkMembers("0v 1v 2v 3"))
54
55 // Remove vote from an existing member;
56 // and give it to the new machine.
57@@ -167,7 +167,7 @@
58 // old machine loses it.
59 c.Logf("waiting for vote switch")
60 mustNext(c, memberWatcher)
61- c.Assert(memberWatcher.Value(), jc.DeepEquals, mkMembers("0 1v 2v 3v"))
62+ assertMembers(c, memberWatcher.Value(), mkMembers("0 1v 2v 3v"))
63
64 c.Logf("removing old machine")
65 // Remove the old machine.
66@@ -177,7 +177,7 @@
67 // Check that it's removed from the members.
68 c.Logf("waiting for removal")
69 mustNext(c, memberWatcher)
70- c.Assert(memberWatcher.Value(), jc.DeepEquals, mkMembers("1v 2v 3v"))
71+ assertMembers(c, memberWatcher.Value(), mkMembers("1v 2v 3v"))
72 }
73
74 func (s *workerSuite) TestAddressChange(c *gc.C) {
75@@ -186,7 +186,7 @@
76
77 memberWatcher := st.session.members.Watch()
78 mustNext(c, memberWatcher)
79- c.Assert(memberWatcher.Value(), jc.DeepEquals, mkMembers("0v"))
80+ assertMembers(c, memberWatcher.Value(), mkMembers("0v"))
81
82 logger.Infof("starting worker")
83 w := newWorker(st, noPublisher{})
84@@ -196,7 +196,7 @@
85
86 // Wait for the worker to set the initial members.
87 mustNext(c, memberWatcher)
88- c.Assert(memberWatcher.Value(), jc.DeepEquals, mkMembers("0v 1 2"))
89+ assertMembers(c, memberWatcher.Value(), mkMembers("0v 1 2"))
90
91 // Change an address and wait for it to be changed in the
92 // members.
93@@ -205,7 +205,7 @@
94 mustNext(c, memberWatcher)
95 expectMembers := mkMembers("0v 1 2")
96 expectMembers[1].Address = "0.1.99.99:9876"
97- c.Assert(memberWatcher.Value(), jc.DeepEquals, expectMembers)
98+ assertMembers(c, memberWatcher.Value(), expectMembers)
99 }
100
101 var fatalErrorsTests = []struct {

Subscribers

People subscribed via source and target branches

to status/vote changes: