ericholguin commented on code in PR #6814:
URL: https://github.com/apache/trafficcontrol/pull/6814#discussion_r869731025


##########
traffic_ops/testing/api/v4/cachegroupsdeliveryservices_test.go:
##########
@@ -20,143 +20,88 @@ import (
        "strconv"
        "testing"
 
+       "github.com/apache/trafficcontrol/traffic_ops/testing/api/assert"
+       "github.com/apache/trafficcontrol/traffic_ops/testing/api/utils"
        client "github.com/apache/trafficcontrol/traffic_ops/v4-client"
 )
 
 func TestCacheGroupsDeliveryServices(t *testing.T) {
-       WithObjs(t, []TCObj{CDNs, Types, Tenants, Parameters, Profiles, 
Statuses, Divisions, Regions, PhysLocations, CacheGroups, Servers, Topologies, 
ServiceCategories, DeliveryServices, CacheGroupsDeliveryServices}, func() {})
-}
+       WithObjs(t, []TCObj{CDNs, Types, Tenants, Parameters, Profiles, 
Statuses, Divisions, Regions, PhysLocations, CacheGroups, Servers, Topologies, 
ServiceCategories, DeliveryServices, CacheGroupsDeliveryServices}, func() {
+               methodTests := utils.V4TestCase{
+                       "POST": {
+                               "BAD REQUEST assigning TOPOLOGY-BASED DS to 
CACHEGROUP": {
+                                       EndpointId:    GetCacheGroupId(t, 
"cachegroup3"),
+                                       ClientSession: TOSession,
+                                       RequestBody: map[string]interface{}{
+                                               "deliveryServices": 
[]int{GetDeliveryServiceId(t, "top-ds-in-cdn1")()},
+                                       },
+                                       Expectations: 
utils.CkRequest(utils.HasError(), utils.HasStatus(http.StatusBadRequest)),
+                               },
+                               "OK when valid request": {
+                                       EndpointId:    GetCacheGroupId(t, 
"cachegroup3"),
+                                       ClientSession: TOSession,
+                                       RequestBody: map[string]interface{}{
+                                               "deliveryServices": []int{
+                                                       GetDeliveryServiceId(t, 
"ds1")(),
+                                                       GetDeliveryServiceId(t, 
"ds2")(),
+                                                       GetDeliveryServiceId(t, 
"ds3")(),
+                                                       GetDeliveryServiceId(t, 
"ds3")(),
+                                                       GetDeliveryServiceId(t, 
"DS5")(),
+                                               },
+                                       },
+                                       Expectations: 
utils.CkRequest(utils.NoError(), utils.HasStatus(http.StatusOK)),
+                               },
+                       },
+               }
 
-// TODO this is the name hard-coded in the create servers test; change to be 
dynamic
-// TODO this test assumes that a CDN named "cdn1" exists, has at least one 
Delivery Service, and also
-// assumes that ALL SERVERS IN "cachegroup3" ARE EDGE-TIER CACHE SERVERS IN 
"cdn1". If that EVER changes,
-// this WILL break.
-const TestEdgeServerCacheGroupName = "cachegroup3"
+               for method, testCases := range methodTests {
+                       t.Run(method, func(t *testing.T) {
+                               for name, testCase := range testCases {
+                                       switch method {
+                                       case "POST":
+                                               t.Run(name, func(t *testing.T) {
+                                                       resp, reqInf, err := 
testCase.ClientSession.SetCacheGroupDeliveryServices(testCase.EndpointId(), 
testCase.RequestBody["deliveryServices"].([]int), testCase.RequestOpts)
+                                                       for _, check := range 
testCase.Expectations {
+                                                               check(t, 
reqInf, resp.Response, resp.Alerts, err)
+                                                       }
+                                               })
+                                       }
+                               }
+                       })
+               }
 
-func CreateTestCachegroupsDeliveryServices(t *testing.T) {
-       dss, _, err := 
TOSession.GetDeliveryServiceServers(client.RequestOptions{})
-       if err != nil {
-               t.Fatalf("Unexpected error retrieving 
server-to-Delivery-Service assignments: %v - alerts: %+v", err, dss.Alerts)
-       }
-       if len(dss.Response) > 0 {
-               t.Fatalf("cannot test cachegroups delivery services: expected 
no initial delivery service servers, actual %v", len(dss.Response))
-       }
+       })
+}
 
+func CreateTestCachegroupsDeliveryServices(t *testing.T) {
        dses, _, err := TOSession.GetDeliveryServices(client.RequestOptions{})
-       if err != nil {
-               t.Fatalf("cannot GET DeliveryServices: %v - %v", err, dses)
-       }
+       assert.RequireNoError(t, err, "Cannot GET DeliveryServices: %v - %v", 
err, dses)
 
        opts := client.NewRequestOptions()
-       opts.QueryParameters.Set("name", TestEdgeServerCacheGroupName)
+       opts.QueryParameters.Set("name", "cachegroup3")
        clientCGs, _, err := TOSession.GetCacheGroups(opts)
-       if err != nil {
-               t.Fatalf("getting cachegroup: %v", err)
-       }
-       if len(clientCGs.Response) != 1 {
-               t.Fatalf("getting cachegroup expected 1, got %v", 
len(clientCGs.Response))
-       }
-
-       clientCG := clientCGs.Response[0]
-
-       if clientCG.ID == nil {
-               t.Fatalf("Cachegroup has a nil ID")
-       }
-       cgID := *clientCG.ID
+       assert.RequireNoError(t, err, "Cannot GET cachegroup: %v", err)
+       assert.RequireEqual(t, len(clientCGs.Response), 1, "Getting cachegroup 
expected 1, got %v", len(clientCGs.Response))
+       assert.RequireNotNil(t, clientCGs.Response[0].ID, "Cachegroup has a nil 
ID")
 
        dsIDs := []int{}
-       topologyDsIDs := []int{}
        for _, ds := range dses.Response {
                if *ds.CDNName == "cdn1" && ds.Topology == nil {
                        dsIDs = append(dsIDs, *ds.ID)
-               } else if *ds.CDNName == "cdn1" && ds.Topology != nil {
-                       topologyDsIDs = append(topologyDsIDs, *ds.ID)
-               }
-       }
-       if len(dsIDs) < 1 {
-               t.Fatal("No Delivery Services found in CDN 'cdn1', cannot 
continue.")
-       }
-
-       if len(topologyDsIDs) < 1 {

Review Comment:
   Same as above, those checks aren't needed since the test above handles it 



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to