Merge lp:~jtv/gwacl/management-test-helpers into lp:gwacl

[Jeroen T. Vermeulen]

Clean up management_test.go a bit, using some test helpers.

In my branch to create ListAllDeployments I had to add a test to management_test.go, and these tests were pretty long and heavy with fixture. So I extracted some of the repetition. There's a lot of little functions now, as is my wont, but the good news is that you can usually ignore these. The test still shrinks overall, and the actual tests shrink by a lot.

Jeroen

[lp:~jtv/gwacl/management-test-helpers]

148. By Jeroen T. Vermeulen on 2013-06-30

Kick off fresh diff.

[Jeroen T. Vermeulen (jtv)]

Strange: the ListDeployments/ListAllDeployments split, already landed in trunk, now shows up as part of this diff. That's why it looks as if I'm increasing line count instead of reducing it. I'll see if kicking off a fresh diff helps.

[Julian Edwards (julian-edwards)]

51 + return []DispatcherResponse{
52 + {
53 + response: &x509Response{
54 + StatusCode: http.StatusOK,
55 + Body: []byte(body),
56 + },
57 + },
58 + }

<whinge type=general>
I wish go fmt would make its mind up whether it prefers ANSI or K&R style braces. This is getting on my nerves.
</whinge>

Also do you fancy making a branch to s/rig/patch/ everywhere? If you don't I will!

review: Approve

=== modified file 'management_test.go'
--- management_test.go	2013-06-28 17:02:53 +0000
+++ management_test.go	2013-06-30 02:20:30 +0000
@@ -4,6 +4,7 @@
 package gwacl
 
 import (
+    "fmt"
     . "launchpad.net/gocheck"
     "net/http"
 )
@@ -12,67 +13,90 @@
 
 var _ = Suite(&managementAPISuite{})
 
+// makeNamedRoleInstances creates an array of RoleInstance objects, each with
+// the respective given name.
+func makeNamedRoleInstances(names ...string) []RoleInstance {
+    instances := make([]RoleInstance, 0)
+    for _, name := range names {
+        instances = append(instances, RoleInstance{RoleName: name})
+    }
+    return instances
+}
+
+// makeNamedDeployments creates an array of Deployment objects, each with
+// the respective given name.
+func makeNamedDeployments(names ...string) []Deployment {
+    deployments := make([]Deployment, 0)
+    for _, name := range names {
+        deployments = append(deployments, Deployment{Name: name})
+    }
+    return deployments
+}
+
+// makeHostedService creates a HostedService with the given deployments.
+func makeHostedService(deployments []Deployment) HostedService {
+    return HostedService{
+        ServiceName: "S1",
+        Deployments: deployments,
+    }
+}
+
+// makeOKXMLResponse creates a DispatcherResponse with status code OK, and
+// an XML-serialized version of the given object.
+// The response is wrapped in a slice because that's slightly easier for
+// the callers.
+func makeOKXMLResponse(c *C, bodyObject AzureObject) []DispatcherResponse {
+    body, err := bodyObject.Serialize()
+    c.Assert(err, IsNil)
+    return []DispatcherResponse{
+        {
+            response: &x509Response{
+                StatusCode: http.StatusOK,
+                Body:       []byte(body),
+            },
+        },
+    }
+}
+
 // TestListInstances goes through the happy path for ListInstances.
 func (suite *managementAPISuite) TestListInstances(c *C) {
-    // The list of properties for "S1".
-    propertiesS1 := HostedService{
-        ServiceName: "S1",
-        Deployments: []Deployment{
-            {
-                RoleInstanceList: []RoleInstance{
-                    {RoleName: "one"},
-                    {RoleName: "two"},
-                },
-            },
-            {
-                RoleInstanceList: []RoleInstance{
-                    {RoleName: "three"},
-                    {RoleName: "four"},
-                },
-            },
-        },
-    }
-    propertiesS1XML, err := propertiesS1.Serialize()
-    c.Assert(err, IsNil)
-    propertiesS1Response := DispatcherResponse{
-        response: &x509Response{
-            Body:       []byte(propertiesS1XML),
-            StatusCode: http.StatusOK,
-        },
-    }
-    // Rig the prepared response in.
+    service := makeHostedService(
+        []Deployment{
+            {RoleInstanceList: makeNamedRoleInstances("one", "two")},
+            {RoleInstanceList: makeNamedRoleInstances("three", "four")},
+        })
     record := []*X509Request{}
-    rigRecordingPreparedResponseDispatcher(
-        &record, []DispatcherResponse{propertiesS1Response},
-    )
-    // Finally, exercise ListInstances.
+    rigRecordingPreparedResponseDispatcher(&record, makeOKXMLResponse(c, service))
+
+    // Exercise ListInstances.
     api := makeAPI(c)
-    request := &ListInstancesRequest{ServiceName: propertiesS1.ServiceName}
+    request := &ListInstancesRequest{ServiceName: service.ServiceName}
     instances, err := api.ListInstances(request)
     c.Assert(err, IsNil)
+
+    // We get the expected instances back.
     c.Check(instances, DeepEquals, []RoleInstance{
-        propertiesS1.Deployments[0].RoleInstanceList[0],
-        propertiesS1.Deployments[0].RoleInstanceList[1],
-        propertiesS1.Deployments[1].RoleInstanceList[0],
-        propertiesS1.Deployments[1].RoleInstanceList[1],
+        service.Deployments[0].RoleInstanceList[0],
+        service.Deployments[0].RoleInstanceList[1],
+        service.Deployments[1].RoleInstanceList[0],
+        service.Deployments[1].RoleInstanceList[1],
     })
-    // The only request is for S1's properties
+
+    // The only request is for the service's properties
     c.Assert(len(record), Not(Equals), 0)
+    expectedURL := fmt.Sprintf(
+        "%ssubscriptionId/services/hostedservices/%s?embed-detail=true",
+        AZURE_URL, service.ServiceName)
     c.Check(record[0], DeepEquals, &X509Request{
         APIVersion: baseAPIVersion,
-        URL:        AZURE_URL + "subscriptionId/services/hostedservices/S1?embed-detail=true",
+        URL:        expectedURL,
         Method:     "GET",
     })
 }
 
 func (suite *managementAPISuite) TestListInstancesFailsGettingDetails(c *C) {
-    propertiesS1Response := DispatcherResponse{
-        response: &x509Response{
-            StatusCode: http.StatusNotFound,
-        },
-    }
     rigPreparedResponseDispatcher(
-        []DispatcherResponse{propertiesS1Response},
+        []DispatcherResponse{{response: &x509Response{StatusCode: http.StatusNotFound}}},
     )
     api := makeAPI(c)
     request := &ListInstancesRequest{ServiceName: "SomeService"}
@@ -84,101 +108,57 @@
 
 // TestListAllDeployments goes through the happy path for ListAllDeployments.
 func (suite *managementAPISuite) TestListAllDeployments(c *C) {
-    // The list of properties for "S1".
-    propertiesS1 := HostedService{
-        ServiceName: "S1",
-        Deployments: []Deployment{
-            {Name: "one"}, {Name: "two"},
-        },
-    }
-    propertiesS1XML, err := propertiesS1.Serialize()
-    c.Assert(err, IsNil)
-    propertiesS1Response := DispatcherResponse{
-        response: &x509Response{
-            Body:       []byte(propertiesS1XML),
-            StatusCode: http.StatusOK,
-        },
-    }
-    // Rig the prepared response in.
+    service := makeHostedService(makeNamedDeployments("one", "two"))
     record := []*X509Request{}
-    rigRecordingPreparedResponseDispatcher(
-        &record, []DispatcherResponse{propertiesS1Response},
-    )
-    // Finally, exercise ListDeployments.
+    rigRecordingPreparedResponseDispatcher(&record, makeOKXMLResponse(c, service))
+
+    // Exercise ListDeployments.
     api := makeAPI(c)
-    request := &ListAllDeploymentsRequest{ServiceName: propertiesS1.ServiceName}
+    request := &ListAllDeploymentsRequest{ServiceName: service.ServiceName}
     deployments, err := api.ListAllDeployments(request)
     c.Assert(err, IsNil)
-    c.Check(deployments, DeepEquals, propertiesS1.Deployments)
+
+    // We get the complete set of deployments back.
+    c.Check(deployments, DeepEquals, service.Deployments)
+
     // Only one request to the API is made.
     c.Assert(len(record), Equals, 1)
 }
 
 // TestListDeployments tests ListDeployments, including filtering by name.
 func (suite *managementAPISuite) TestListDeployments(c *C) {
-    // The list of properties for "S1".
-    propertiesS1 := HostedService{
-        ServiceName: "S1",
-        Deployments: []Deployment{
-            {Name: "Arthur"}, {Name: "Bobby"},
-        },
-    }
-    propertiesS1XML, err := propertiesS1.Serialize()
-    c.Assert(err, IsNil)
-    propertiesS1Response := DispatcherResponse{
-        response: &x509Response{
-            Body:       []byte(propertiesS1XML),
-            StatusCode: http.StatusOK,
-        },
-    }
-    // Rig the prepared response in.
+    service := makeHostedService(makeNamedDeployments("Arthur", "Bobby"))
     record := []*X509Request{}
-    rigRecordingPreparedResponseDispatcher(
-        &record, []DispatcherResponse{propertiesS1Response},
-    )
-    // Finally, exercise ListDeployments.
+    rigRecordingPreparedResponseDispatcher(&record, makeOKXMLResponse(c, service))
+
+    // Exercise ListDeployments.
     api := makeAPI(c)
     request := &ListDeploymentsRequest{
-        ServiceName:     propertiesS1.ServiceName,
+        ServiceName:     service.ServiceName,
         DeploymentNames: []string{"Arthur"},
     }
     deployments, err := api.ListDeployments(request)
     c.Assert(err, IsNil)
+
     // Only the first deployment - named "Arthur" - is returned.
-    c.Check(deployments, DeepEquals, []Deployment{propertiesS1.Deployments[0]})
+    c.Check(deployments, DeepEquals, []Deployment{service.Deployments[0]})
     // Only one request to the API is made.
     c.Assert(len(record), Equals, 1)
 }
 
 func (suite *managementAPISuite) TestListDeploymentsWithoutNamesReturnsNothing(c *C) {
-    // The list of properties for "S1".
-    propertiesS1 := HostedService{
-        ServiceName: "S1",
-        Deployments: []Deployment{
-            {Name: "Arthur"}, {Name: "Bobby"},
-        },
-    }
-    propertiesS1XML, err := propertiesS1.Serialize()
-    c.Assert(err, IsNil)
-    propertiesS1Response := DispatcherResponse{
-        response: &x509Response{
-            Body:       []byte(propertiesS1XML),
-            StatusCode: http.StatusOK,
-        },
-    }
-    // Rig the prepared response in.
+    service := makeHostedService(makeNamedDeployments("One", "Two"))
     record := []*X509Request{}
-    rigRecordingPreparedResponseDispatcher(
-        &record, []DispatcherResponse{propertiesS1Response},
-    )
-    // Finally, exercise ListDeployments.
+    rigRecordingPreparedResponseDispatcher(&record, makeOKXMLResponse(c, service))
+    // Exercise ListDeployments.
     api := makeAPI(c)
     request := &ListDeploymentsRequest{
-        ServiceName:     propertiesS1.ServiceName,
+        ServiceName:     service.ServiceName,
         DeploymentNames: []string{},
     }
     deployments, err := api.ListDeployments(request)
     c.Assert(err, IsNil)
+
     // No deployments are returned.
     c.Check(len(deployments), Equals, 0)
 }