ericholguin commented on code in PR #6814:
URL: https://github.com/apache/trafficcontrol/pull/6814#discussion_r869732680
##########
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 {
- t.Fatal("No Topology-based Delivery Services found in CDN
'cdn1', cannot continue.")
- }
-
- _, reqInf, err := TOSession.SetCacheGroupDeliveryServices(cgID,
topologyDsIDs, client.RequestOptions{})
- if err == nil {
- t.Fatal("assigning Topology-based delivery service to
cachegroup - expected: error, actual: nil")
- }
- if reqInf.StatusCode < http.StatusBadRequest || reqInf.StatusCode >=
http.StatusInternalServerError {
- t.Fatalf("assigning Topology-based delivery service to
cachegroup - expected: 400-level status code, actual: %d", reqInf.StatusCode)
- }
-
- resp, _, err := TOSession.SetCacheGroupDeliveryServices(cgID, dsIDs,
client.RequestOptions{})
- if err != nil {
- t.Fatalf("setting cachegroup delivery services returned error:
%v", err)
- }
- if len(resp.Response.ServerNames) == 0 {
- t.Fatal("setting cachegroup delivery services returned success,
but no servers set")
- }
-
- // Note this second post of the same cg-dses specifically tests a
previous bug, where the query
- // failed if any servers with location parameters were already
assigned, due to a foreign key
- // violation. See https://github.com/apache/trafficcontrol/pull/3199
- resp, _, err = TOSession.SetCacheGroupDeliveryServices(cgID, dsIDs,
client.RequestOptions{})
- if err != nil {
- t.Fatalf("setting cachegroup delivery services returned error:
%v", err)
- }
- if len(resp.Response.ServerNames) == 0 {
- t.Fatal("setting cachegroup delivery services returned success,
but no servers set")
- }
-
- opts.QueryParameters.Del("name")
- for _, serverName := range resp.Response.ServerNames {
- opts.QueryParameters.Set("hostName", string(serverName))
Review Comment:
Hm, yeah this is validating that the servers belonging to the cachegroup are
now assigned to the delivery service. I missed this, I will add the validation
function.
--
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]
