[GitHub] [trafficcontrol] ericholguin commented on a diff in pull request #6814: Refactor CacheGroupDeliveryServices tests

[GitBox <gi...@apache.org>]

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


##########
traffic_ops/testing/api/v3/cachegroupsdeliveryservices_test.go:
##########
@@ -19,178 +19,111 @@ import (
 	"net/http"
 	"net/url"
 	"strconv"
-	"strings"
 	"testing"
+
+	"github.com/apache/trafficcontrol/traffic_ops/testing/api/assert"
+	"github.com/apache/trafficcontrol/traffic_ops/testing/api/utils"
 )
 
 func TestCacheGroupsDeliveryServices(t *testing.T) {
-	WithObjs(t, []TCObj{CDNs, Types, Tenants, Parameters, Profiles, Statuses, Divisions, Regions, PhysLocations, CacheGroups, Servers, Topologies, DeliveryServices, CacheGroupsDeliveryServices}, func() {})
-}
+	WithObjs(t, []TCObj{CDNs, Types, Tenants, Parameters, Profiles, Statuses, Divisions, Regions, PhysLocations, CacheGroups, Servers, Topologies, DeliveryServices, CacheGroupsDeliveryServices}, func() {
+
+		methodTests := utils.V3TestCase{
+			"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")(),
+						},
+					},
+					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))
+							for _, check := range testCase.Expectations {
+								check(t, reqInf, resp.Response, resp.Alerts, err)
+							}
+						})
+					}
+				}
+			})
+		}
 
-func CreateTestCachegroupsDeliveryServices(t *testing.T) {
-	dss, _, err := TOSession.GetDeliveryServiceServers()

Review Comment:
   This check was not needed. The tests run against a clean database and since we decide what prerequisites we are using per test we know there are no existing ds server assignments. In addition to that, IF there were any ds server assignments it would have no affect on anything tested here.



-- 
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: issues-unsubscribe@trafficcontrol.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org