juju-core

Merge lp:~axwalk/juju-core/lp1303583-provider-azure-tests-gccgo into lp:~go-bot/juju-core/trunk

  1. lp1303583-provider-azure-tests-gccgo
  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: 2765
Proposed branch: lp:~axwalk/juju-core/lp1303583-provider-azure-tests-gccgo
Merge into: lp:~go-bot/juju-core/trunk
Diff against target: 67 lines (+10/-6)
2 files modified
provider/azure/environ.go (+5/-1)
provider/azure/environ_test.go (+5/-5)
To merge this branch: bzr merge lp:~axwalk/juju-core/lp1303583-provider-azure-tests-gccgo
Reviewer Review Type Date Requested Status
Juju Engineering Pending
Review via email: mp+220552@code.launchpad.net

Commit message

provider/azure: fix tests for go1.3/gccgo

Fix a test to use SameContents instead of DeepEquals.
Also, make StopInstances deterministic; it now stops
instances in the order of IDs specified. One of the
StopInstances tests was sending responses in the wrong
order, and that has been rectified.

Fixes lp:1303583

https://codereview.appspot.com/93520047/

Description of the change

provider/azure: fix tests for go1.3/gccgo

Fix a test to use SameContents instead of DeepEquals.
Also, make StopInstances deterministic; it now stops
instances in the order of IDs specified. One of the
StopInstances tests was sending responses in the wrong
order, and that has been rectified.

Fixes lp:1303583

https://codereview.appspot.com/93520047/

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

Reviewers: mp+220552_code.launchpad.net,

Message:
Please take a look.

Description:
provider/azure: fix tests for go1.3/gccgo

Fix a test to use SameContents instead of DeepEquals.
Also, make StopInstances deterministic; it now stops
instances in the order of IDs specified. One of the
StopInstances tests was sending responses in the wrong
order, and that has been rectified.

Fixes lp:1303583

https://code.launchpad.net/~axwalk/juju-core/lp1303583-provider-azure-tests-gccgo/+merge/220552

(do not edit description out of merge proposal)

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

Affected files (+12, -6 lines):
   A [revision details]
   M provider/azure/environ.go
   M provider/azure/environ_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-20140522020148-mkgjwj28zqzfxcad
+New revision: <email address hidden>

Index: provider/azure/environ.go
=== modified file 'provider/azure/environ.go'
--- provider/azure/environ.go 2014-05-22 01:35:12 +0000
+++ provider/azure/environ.go 2014-05-22 03:48:17 +0000
@@ -769,15 +769,18 @@

   // Map services to role names we want to delete.
   serviceInstances := make(map[string]map[string]bool)
+ serviceNames := make([]string, 0)
   for _, id := range ids {
    serviceName, roleName := env.splitInstanceId(id)
    if roleName == "" {
     serviceInstances[serviceName] = nil
+ serviceNames = append(serviceNames, serviceName)
    } else {
     deleteRoleNames, ok := serviceInstances[serviceName]
     if !ok {
      deleteRoleNames = make(map[string]bool)
      serviceInstances[serviceName] = deleteRoleNames
+ serviceNames = append(serviceNames, serviceName)
     }
     deleteRoleNames[roleName] = true
    }
@@ -788,7 +791,8 @@
   //
   // Note: concurrent operations on Affinity Groups have been
   // found to cause conflict responses, so we do everything serially.
- for serviceName, deleteRoleNames := range serviceInstances {
+ for _, serviceName := range serviceNames {
+ deleteRoleNames := serviceInstances[serviceName]
    service, err := context.GetHostedServiceProperties(serviceName, true)
    if err != nil {
     return err

Index: provider/azure/environ_test.go
=== modified file 'provider/azure/environ_test.go'
--- provider/azure/environ_test.go 2014-05-14 02:59:53 +0000
+++ provider/azure/environ_test.go 2014-05-22 03:48:17 +0000
@@ -828,8 +828,9 @@
   inst2, err := env.getInstance(service2, service2Role1Name)
   c.Assert(err, gc.IsNil)

- responses := buildGetServicePropertiesResponses(c, service1, service2)
- // Failed to delete one of the services.
+ responses := buildGetServicePropertiesResponses(c, service1)
+ // Failed to delete one of the services. This will cause StopInstances to
stop
+ // immediately.
   responses = append(responses, gwacl.NewDispatcherResponse(nil,
http.StatusConflict, nil))
   requests := gwacl.PatchManagementAPIResponses(responses)

@@ -838,8 +839,7 @@

   c.Check(len(*requests), gc.Equals, len(responses))
   assertOneRequestMatches(c,
*requests, "GET",...

Read more...

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

LGTM.

https://codereview.appspot.com/93520047/diff/1/provider/azure/environ.go
File provider/azure/environ.go (right):

https://codereview.appspot.com/93520047/diff/1/provider/azure/environ.go#newcode772
provider/azure/environ.go:772: serviceNames := make([]string, 0)
var serviceNames []string

https://codereview.appspot.com/93520047/

Revision history for this message
Andrew Wilkins (axwalk) wrote :

Please take a look.

https://codereview.appspot.com/93520047/diff/1/provider/azure/environ.go
File provider/azure/environ.go (right):

https://codereview.appspot.com/93520047/diff/1/provider/azure/environ.go#newcode772
provider/azure/environ.go:772: serviceNames := make([]string, 0)
On 2014/05/22 04:30:03, dfc wrote:
> var serviceNames []string

Done.

https://codereview.appspot.com/93520047/

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'provider/azure/environ.go'
--- provider/azure/environ.go 2014-05-22 01:35:12 +0000
+++ provider/azure/environ.go 2014-05-22 04:37:15 +0000
@@ -769,15 +769,18 @@
769769
770 // Map services to role names we want to delete.770 // Map services to role names we want to delete.
771 serviceInstances := make(map[string]map[string]bool)771 serviceInstances := make(map[string]map[string]bool)
772 var serviceNames []string
772 for _, id := range ids {773 for _, id := range ids {
773 serviceName, roleName := env.splitInstanceId(id)774 serviceName, roleName := env.splitInstanceId(id)
774 if roleName == "" {775 if roleName == "" {
775 serviceInstances[serviceName] = nil776 serviceInstances[serviceName] = nil
777 serviceNames = append(serviceNames, serviceName)
776 } else {778 } else {
777 deleteRoleNames, ok := serviceInstances[serviceName]779 deleteRoleNames, ok := serviceInstances[serviceName]
778 if !ok {780 if !ok {
779 deleteRoleNames = make(map[string]bool)781 deleteRoleNames = make(map[string]bool)
780 serviceInstances[serviceName] = deleteRoleNames782 serviceInstances[serviceName] = deleteRoleNames
783 serviceNames = append(serviceNames, serviceName)
781 }784 }
782 deleteRoleNames[roleName] = true785 deleteRoleNames[roleName] = true
783 }786 }
@@ -788,7 +791,8 @@
788 //791 //
789 // Note: concurrent operations on Affinity Groups have been792 // Note: concurrent operations on Affinity Groups have been
790 // found to cause conflict responses, so we do everything serially.793 // found to cause conflict responses, so we do everything serially.
791 for serviceName, deleteRoleNames := range serviceInstances {794 for _, serviceName := range serviceNames {
795 deleteRoleNames := serviceInstances[serviceName]
792 service, err := context.GetHostedServiceProperties(serviceName, true)796 service, err := context.GetHostedServiceProperties(serviceName, true)
793 if err != nil {797 if err != nil {
794 return err798 return err
795799
=== modified file 'provider/azure/environ_test.go'
--- provider/azure/environ_test.go 2014-05-14 02:59:53 +0000
+++ provider/azure/environ_test.go 2014-05-22 04:37:15 +0000
@@ -828,8 +828,9 @@
828 inst2, err := env.getInstance(service2, service2Role1Name)828 inst2, err := env.getInstance(service2, service2Role1Name)
829 c.Assert(err, gc.IsNil)829 c.Assert(err, gc.IsNil)
830830
831 responses := buildGetServicePropertiesResponses(c, service1, service2)831 responses := buildGetServicePropertiesResponses(c, service1)
832 // Failed to delete one of the services.832 // Failed to delete one of the services. This will cause StopInstances to stop
833 // immediately.
833 responses = append(responses, gwacl.NewDispatcherResponse(nil, http.StatusConflict, nil))834 responses = append(responses, gwacl.NewDispatcherResponse(nil, http.StatusConflict, nil))
834 requests := gwacl.PatchManagementAPIResponses(responses)835 requests := gwacl.PatchManagementAPIResponses(responses)
835836
@@ -838,8 +839,7 @@
838839
839 c.Check(len(*requests), gc.Equals, len(responses))840 c.Check(len(*requests), gc.Equals, len(responses))
840 assertOneRequestMatches(c, *requests, "GET", ".*"+service1.ServiceName+".*")841 assertOneRequestMatches(c, *requests, "GET", ".*"+service1.ServiceName+".*")
841 assertOneRequestMatches(c, *requests, "GET", ".*"+service2.ServiceName+".*")842 assertOneRequestMatches(c, *requests, "DELETE", service1.ServiceName)
842 assertOneRequestMatches(c, *requests, "DELETE", ".*("+service1.ServiceName+"|"+service2.ServiceName+").")
843}843}
844844
845func (s *environSuite) TestStopInstancesWithZeroInstance(c *gc.C) {845func (s *environSuite) TestStopInstancesWithZeroInstance(c *gc.C) {
@@ -1525,7 +1525,7 @@
1525 cons := constraints.MustParse("arch=amd64 tags=bar cpu-power=10")1525 cons := constraints.MustParse("arch=amd64 tags=bar cpu-power=10")
1526 unsupported, err := validator.Validate(cons)1526 unsupported, err := validator.Validate(cons)
1527 c.Assert(err, gc.IsNil)1527 c.Assert(err, gc.IsNil)
1528 c.Assert(unsupported, gc.DeepEquals, []string{"cpu-power", "tags"})1528 c.Assert(unsupported, jc.SameContents, []string{"cpu-power", "tags"})
1529}1529}
15301530
1531func (s *environSuite) TestConstraintsValidatorVocab(c *gc.C) {1531func (s *environSuite) TestConstraintsValidatorVocab(c *gc.C) {

Subscribers

People subscribed via source and target branches

to status/vote changes: