juju-core

Merge lp:~dave-cheney/juju-core/172-fix-constraints-test-failure into lp:~go-bot/juju-core/trunk

  1. 172-fix-constraints-test-failure
  2. Merge into trunk
Proposed by Dave Cheney
Status: Merged
Approved by: Dave Cheney
Approved revision: no longer in the source branch.
Merged at revision: 2756
Proposed branch: lp:~dave-cheney/juju-core/172-fix-constraints-test-failure
Merge into: lp:~go-bot/juju-core/trunk
Diff against target: 117 lines (+14/-14)
6 files modified
cmd/juju/bootstrap_test.go (+1/-1)
constraints/validation.go (+6/-6)
constraints/validation_test.go (+4/-4)
state/machine_test.go (+1/-1)
state/service_test.go (+1/-1)
state/state_test.go (+1/-1)
To merge this branch: bzr merge lp:~dave-cheney/juju-core/172-fix-constraints-test-failure
Reviewer Review Type Date Requested Status
Juju Engineering Pending
Review via email: mp+220156@code.launchpad.net

Commit message

constraints: fix lp 1320738

Fixes lp 1320738

Although the red and blue constraints sets are sets, for consistency sort them before looking for the intersection to return predictable error messages. For no particular reason constraints will be sorted by name.

Description of the change

constraints: fix lp 1320738

Fixes lp 1320738

Although the red and blue constraints sets are sets, for consistency sort them before looking for the intersection to return predictable error messages. For no particular reason constraints will be sorted by name.

https://codereview.appspot.com/91580043/

To post a comment you must log in.
Revision history for this message
Dave Cheney (dave-cheney) wrote :
Download full text (4.1 KiB)

Reviewers: mp+220156_code.launchpad.net,

Message:
Please take a look.

Description:
constraints: fix lp 1320738

Fixes lp 1320738

Although the red and blue constraints sets are sets, for consistency
sort them before looking for the intersection to return predictable
error messages. For no particular reason constraints will be sorted by
name.

https://code.launchpad.net/~dave-cheney/juju-core/172-fix-constraints-test-failure/+merge/220156

(do not edit description out of merge proposal)

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

Affected files (+13, -11 lines):
   A [revision details]
   M cmd/juju/bootstrap_test.go
   M constraints/validation.go
   M constraints/validation_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-20140520044449-ixp17q9u2jbdx2ct
+New revision: <email address hidden>

Index: constraints/validation.go
=== modified file 'constraints/validation.go'
--- constraints/validation.go 2014-04-30 23:18:40 +0000
+++ constraints/validation.go 2014-05-20 05:33:59 +0000
@@ -44,10 +44,10 @@

  // NewValidator returns a new constraints Validator instance.
  func NewValidator() Validator {
- c := validator{}
- c.conflicts = make(map[string]set.Strings)
- c.vocab = make(map[string][]interface{})
- return &c
+ return &validator{
+ conflicts: make(map[string]set.Strings),
+ vocab: make(map[string][]interface{}),
+ }
  }

  type validator struct {
@@ -93,12 +93,12 @@
   for attrTag := range attrValues {
    attrSet.Add(attrTag)
   }
- for attrTag := range attrValues {
+ for _, attrTag := range attrSet.SortedValues() {
    conflicts, ok := v.conflicts[attrTag]
    if !ok {
     continue
    }
- for _, conflict := range conflicts.Values() {
+ for _, conflict := range conflicts.SortedValues() {
     if attrSet.Contains(conflict) {
      return fmt.Errorf("ambiguous constraints: %q overlaps with %q",
attrTag, conflict)
     }

Index: constraints/validation_test.go
=== modified file 'constraints/validation_test.go'
--- constraints/validation_test.go 2014-05-19 02:53:32 +0000
+++ constraints/validation_test.go 2014-05-20 05:33:59 +0000
@@ -39,7 +39,7 @@
    reds: []string{"mem", "arch"},
    blues: []string{"instance-type"},
    unsupported: []string{"cpu-cores"},
- err: `ambiguous constraints: "mem" overlaps
with "instance-type"`,
+ err: `ambiguous constraints: "instance-type" overlaps
with "mem"`,
   },
   {
    cons: "root-disk=8G mem=4G arch=amd64 cpu-cores=4 instance-type=foo",
@@ -67,7 +67,7 @@
    cons: "root-disk=8G mem=4G cpu-cores=4 instance-type=foo",
    reds: []string{"mem", "arch"},
    blues: []string{"instance-type"},
- err: `ambiguous constraints: "mem" overlaps with "instance-type"`,
+ err: `ambiguous constraints: "instance-type" overlaps with "mem"`,
   },
   {
    cons: "arch=amd64 mem=4G cpu-cores=4",
@@ -336,7 +336,7 @@
   consFallback := constraints.MustParse("instance-type=foo mem=4G")
   cons := constraints.MustParse("cpu-cores=2")
   _, err := validato...

Read more...

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

LGTM

https://codereview.appspot.com/91580043/

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

The attempt to merge lp:~dave-cheney/juju-core/172-fix-constraints-test-failure into lp:juju-core failed. Below is the output from the failed tests.

ok launchpad.net/juju-core 0.014s
ok launchpad.net/juju-core/agent 0.948s
ok launchpad.net/juju-core/agent/mongo 0.550s
ok launchpad.net/juju-core/agent/tools 0.175s
ok launchpad.net/juju-core/bzr 5.536s
ok launchpad.net/juju-core/cert 2.534s
ok launchpad.net/juju-core/charm 0.372s
? 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.163s
ok launchpad.net/juju-core/cloudinit/sshinit 0.660s
ok launchpad.net/juju-core/cmd 0.211s
ok launchpad.net/juju-core/cmd/charm-admin 0.333s
? launchpad.net/juju-core/cmd/charmd [no test files]
? launchpad.net/juju-core/cmd/charmload [no test files]
ok launchpad.net/juju-core/cmd/envcmd 0.200s
ok launchpad.net/juju-core/cmd/juju 224.710s
ok launchpad.net/juju-core/cmd/jujud 67.917s
ok launchpad.net/juju-core/cmd/plugins/juju-metadata 11.588s
? launchpad.net/juju-core/cmd/plugins/juju-restore [no test files]
ok launchpad.net/juju-core/cmd/plugins/local 0.163s
? launchpad.net/juju-core/cmd/plugins/local/juju-local [no test files]
ok launchpad.net/juju-core/constraints 0.025s
ok launchpad.net/juju-core/container 0.133s
ok launchpad.net/juju-core/container/factory 0.180s
ok launchpad.net/juju-core/container/kvm 0.181s
ok launchpad.net/juju-core/container/kvm/mock 0.174s
? launchpad.net/juju-core/container/kvm/testing [no test files]
ok launchpad.net/juju-core/container/lxc 4.287s
? 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.198s
ok launchpad.net/juju-core/environs 2.355s
ok launchpad.net/juju-core/environs/bootstrap 11.930s
ok launchpad.net/juju-core/environs/cloudinit 0.432s
ok launchpad.net/juju-core/environs/config 1.617s
ok launchpad.net/juju-core/environs/configstore 0.147s
ok launchpad.net/juju-core/environs/filestorage 0.024s
ok launchpad.net/juju-core/environs/httpstorage 0.639s
ok launchpad.net/juju-core/environs/imagemetadata 0.410s
? launchpad.net/juju-core/environs/imagemetadata/testing [no test files]
ok launchpad.net/juju-core/environs/instances 0.156s
ok launchpad.net/juju-core/environs/jujutest 0.169s
ok launchpad.net/juju-core/environs/manual 10.318s
? launchpad.net/juju-core/environs/network [no test files]
ok launchpad.net/juju-core/environs/simplestreams 0.314s
? launchpad.net/juju-core/environs/simplestreams/testing [no test files]
ok launchpad.net/juju-core/environs/sshstorage 1.046s
ok launchpad.net/juju-core/environs/storage 0.910s
ok launchpad.net/juju-core/environs/sync 49.011s
ok launchpad.net/juju-core/environs/testing 0.138s
ok launchpad.net/juju-core/environs/tools 4.651s
? launchpad.net/juju-core/environs/tools/testing [no test files]
ok launchpad.net/juju-core/instance 0.155s
? launchpad.net/juju-core/instance/testing [no test files]
ok ...

Revision history for this message
Dave Cheney (dave-cheney) wrote :

Please take a look.

https://codereview.appspot.com/91580043/

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'cmd/juju/bootstrap_test.go'
2--- cmd/juju/bootstrap_test.go 2014-05-16 01:33:13 +0000
3+++ cmd/juju/bootstrap_test.go 2014-05-21 00:21:28 +0000
4@@ -293,7 +293,7 @@
5 }, {
6 info: "conflicting --constraints",
7 args: []string{"--constraints", "instance-type=foo mem=4G"},
8- err: `ambiguous constraints: "mem" overlaps with "instance-type"`,
9+ err: `ambiguous constraints: "instance-type" overlaps with "mem"`,
10 }, {
11 info: "bad --series",
12 args: []string{"--series", "1bad1"},
13
14=== modified file 'constraints/validation.go'
15--- constraints/validation.go 2014-04-30 23:18:40 +0000
16+++ constraints/validation.go 2014-05-21 00:21:28 +0000
17@@ -44,10 +44,10 @@
18
19 // NewValidator returns a new constraints Validator instance.
20 func NewValidator() Validator {
21- c := validator{}
22- c.conflicts = make(map[string]set.Strings)
23- c.vocab = make(map[string][]interface{})
24- return &c
25+ return &validator{
26+ conflicts: make(map[string]set.Strings),
27+ vocab: make(map[string][]interface{}),
28+ }
29 }
30
31 type validator struct {
32@@ -93,12 +93,12 @@
33 for attrTag := range attrValues {
34 attrSet.Add(attrTag)
35 }
36- for attrTag := range attrValues {
37+ for _, attrTag := range attrSet.SortedValues() {
38 conflicts, ok := v.conflicts[attrTag]
39 if !ok {
40 continue
41 }
42- for _, conflict := range conflicts.Values() {
43+ for _, conflict := range conflicts.SortedValues() {
44 if attrSet.Contains(conflict) {
45 return fmt.Errorf("ambiguous constraints: %q overlaps with %q", attrTag, conflict)
46 }
47
48=== modified file 'constraints/validation_test.go'
49--- constraints/validation_test.go 2014-05-19 02:53:32 +0000
50+++ constraints/validation_test.go 2014-05-21 00:21:28 +0000
51@@ -39,7 +39,7 @@
52 reds: []string{"mem", "arch"},
53 blues: []string{"instance-type"},
54 unsupported: []string{"cpu-cores"},
55- err: `ambiguous constraints: "mem" overlaps with "instance-type"`,
56+ err: `ambiguous constraints: "instance-type" overlaps with "mem"`,
57 },
58 {
59 cons: "root-disk=8G mem=4G arch=amd64 cpu-cores=4 instance-type=foo",
60@@ -67,7 +67,7 @@
61 cons: "root-disk=8G mem=4G cpu-cores=4 instance-type=foo",
62 reds: []string{"mem", "arch"},
63 blues: []string{"instance-type"},
64- err: `ambiguous constraints: "mem" overlaps with "instance-type"`,
65+ err: `ambiguous constraints: "instance-type" overlaps with "mem"`,
66 },
67 {
68 cons: "arch=amd64 mem=4G cpu-cores=4",
69@@ -336,7 +336,7 @@
70 consFallback := constraints.MustParse("instance-type=foo mem=4G")
71 cons := constraints.MustParse("cpu-cores=2")
72 _, err := validator.Merge(consFallback, cons)
73- c.Assert(err, gc.ErrorMatches, `ambiguous constraints: "mem" overlaps with "instance-type"`)
74+ c.Assert(err, gc.ErrorMatches, `ambiguous constraints: "instance-type" overlaps with "mem"`)
75 _, err = validator.Merge(cons, consFallback)
76- c.Assert(err, gc.ErrorMatches, `ambiguous constraints: "mem" overlaps with "instance-type"`)
77+ c.Assert(err, gc.ErrorMatches, `ambiguous constraints: "instance-type" overlaps with "mem"`)
78 }
79
80=== modified file 'state/machine_test.go'
81--- state/machine_test.go 2014-05-13 04:30:48 +0000
82+++ state/machine_test.go 2014-05-21 00:21:28 +0000
83@@ -1265,7 +1265,7 @@
84 c.Assert(err, gc.IsNil)
85 cons := constraints.MustParse("mem=4G instance-type=foo")
86 err = machine.SetConstraints(cons)
87- c.Assert(err, gc.ErrorMatches, `cannot set constraints: ambiguous constraints: "mem" overlaps with "instance-type"`)
88+ c.Assert(err, gc.ErrorMatches, `cannot set constraints: ambiguous constraints: "instance-type" overlaps with "mem"`)
89 }
90
91 func (s *MachineSuite) TestSetUnsupportedConstraintsWarning(c *gc.C) {
92
93=== modified file 'state/service_test.go'
94--- state/service_test.go 2014-05-13 04:30:48 +0000
95+++ state/service_test.go 2014-05-21 00:21:28 +0000
96@@ -1164,7 +1164,7 @@
97 func (s *ServiceSuite) TestSetInvalidConstraints(c *gc.C) {
98 cons := constraints.MustParse("mem=4G instance-type=foo")
99 err := s.mysql.SetConstraints(cons)
100- c.Assert(err, gc.ErrorMatches, `ambiguous constraints: "mem" overlaps with "instance-type"`)
101+ c.Assert(err, gc.ErrorMatches, `ambiguous constraints: "instance-type" overlaps with "mem"`)
102 }
103
104 func (s *ServiceSuite) TestSetUnsupportedConstraintsWarning(c *gc.C) {
105
106=== modified file 'state/state_test.go'
107--- state/state_test.go 2014-05-13 04:30:48 +0000
108+++ state/state_test.go 2014-05-21 00:21:28 +0000
109@@ -1403,7 +1403,7 @@
110 func (s *StateSuite) TestSetInvalidConstraints(c *gc.C) {
111 cons := constraints.MustParse("mem=4G instance-type=foo")
112 err := s.State.SetEnvironConstraints(cons)
113- c.Assert(err, gc.ErrorMatches, `ambiguous constraints: "mem" overlaps with "instance-type"`)
114+ c.Assert(err, gc.ErrorMatches, `ambiguous constraints: "instance-type" overlaps with "mem"`)
115 }
116
117 func (s *StateSuite) TestSetUnsupportedConstraintsWarning(c *gc.C) {

Subscribers

People subscribed via source and target branches

to status/vote changes: