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

[Andrew Wilkins]

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/

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/

[Andrew Wilkins (axwalk)]

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",...

[Dave Cheney (dave-cheney)]

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/

[Andrew Wilkins (axwalk)]

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/

=== 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 @@
 
 	// Map services to role names we want to delete.
 	serviceInstances := make(map[string]map[string]bool)
+	var serviceNames []string
 	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

=== 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 @@
 	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", ".*"+service1.ServiceName+".*")
-	assertOneRequestMatches(c, *requests, "GET", ".*"+service2.ServiceName+".*")
-	assertOneRequestMatches(c, *requests, "DELETE", ".*("+service1.ServiceName+"|"+service2.ServiceName+").")
+	assertOneRequestMatches(c, *requests, "DELETE", service1.ServiceName)
 }
 
 func (s *environSuite) TestStopInstancesWithZeroInstance(c *gc.C) {
@@ -1525,7 +1525,7 @@
 	cons := constraints.MustParse("arch=amd64 tags=bar cpu-power=10")
 	unsupported, err := validator.Validate(cons)
 	c.Assert(err, gc.IsNil)
-	c.Assert(unsupported, gc.DeepEquals, []string{"cpu-power", "tags"})
+	c.Assert(unsupported, jc.SameContents, []string{"cpu-power", "tags"})
 }
 
 func (s *environSuite) TestConstraintsValidatorVocab(c *gc.C) {