You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@trafficcontrol.apache.org by GitBox <gi...@apache.org> on 2022/05/06 17:00:37 UTC
[GitHub] [trafficcontrol] ericholguin opened a new pull request, #6814: Refactor CacheGroupDeliveryServices tests
ericholguin opened a new pull request, #6814:
URL: https://github.com/apache/trafficcontrol/pull/6814
<!--
Thank you for contributing! Please be sure to read our contribution guidelines: https://github.com/apache/trafficcontrol/blob/master/CONTRIBUTING.md
If this closes or relates to an existing issue, please reference it using one of the following:
Closes: #ISSUE
Related: #ISSUE
If this PR fixes a security vulnerability, DO NOT submit! Instead, contact
the Apache Traffic Control Security Team at security@trafficcontrol.apache.org and follow the
guidelines at https://apache.org/security regarding vulnerability disclosure.
-->
This PR refactors **v4/cachegroupsdeliveryservices_test.go**, **v3/cachegroupsdeliveryservices_test.go**, in order to make tests more granular, providing transparency on what is being tested and reducing the need to dig through test code to confirm what was tested. In addition this new test structure should also make it easier for new tests to be added as well as reduce redundant code.
- Adds data driven testing
- Descriptive test names
- Sub tests for organization
- Use GoLangs test runner
- Use assert functionality
- Provides fundamental assertions
- Streamlines code
- Reduces nested conditionals
- Adds expectation functions
- Test specific expectations in a clear concise way
- Reusable expectations
<!-- **^ Add meaningful description above** --><hr/>
## Which Traffic Control components are affected by this PR?
<!-- Please delete all components from this list that are NOT affected by this PR.
Feel free to add the name of a tool or script that is affected but not on the list.
-->
- Traffic Ops Tests
## What is the best way to verify this PR?
<!-- Please include here ALL the steps necessary to test your PR.
If your PR has tests (and most should), provide the steps needed to run the tests.
If not, please provide step-by-step instructions to test the PR manually and explain why your PR does not need tests. -->
Run TO integration test
## PR submission checklist
- [x] This PR has tests <!-- If not, please delete this text and explain why this PR does not need tests. -->
- [x] This PR **DOES NOT FIX A SERIOUS SECURITY VULNERABILITY** (see [the Apache Software Foundation's security guidelines](https://apache.org/security) for details)
<!--
Licensed to the Apache Software Foundation (ASF) under one
or more contributor license agreements. See the NOTICE file
distributed with this work for additional information
regarding copyright ownership. The ASF licenses this file
to you under the Apache License, Version 2.0 (the
"License"); you may not use this file except in compliance
with the License. You may obtain a copy of the License at
http://www.apache.org/licenses/LICENSE-2.0
Unless required by applicable law or agreed to in writing,
software distributed under the License is distributed on an
"AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
KIND, either express or implied. See the License for the
specific language governing permissions and limitations
under the License.
-->
--
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
[GitHub] [trafficcontrol] ericholguin commented on a diff in pull request #6814: Refactor CacheGroupDeliveryServices tests
Posted by GitBox <gi...@apache.org>.
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: issues-unsubscribe@trafficcontrol.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficcontrol] ocket8888 merged pull request #6814: Refactor CacheGroupDeliveryServices tests
Posted by GitBox <gi...@apache.org>.
ocket8888 merged PR #6814:
URL: https://github.com/apache/trafficcontrol/pull/6814
--
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
[GitHub] [trafficcontrol] rimashah25 commented on a diff in pull request #6814: Refactor CacheGroupDeliveryServices tests
Posted by GitBox <gi...@apache.org>.
rimashah25 commented on code in PR #6814:
URL: https://github.com/apache/trafficcontrol/pull/6814#discussion_r869710293
##########
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:
I don't see this condition check. Do we not need it? Or even the below checks for Topology (L84-90)?
##########
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 {
Review Comment:
Are we not checking for this condition?
##########
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:
Where is this check happening?
##########
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:
Why did we have this check before and not now?
--
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
[GitHub] [trafficcontrol] ericholguin commented on a diff in pull request #6814: Refactor CacheGroupDeliveryServices tests
Posted by GitBox <gi...@apache.org>.
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: issues-unsubscribe@trafficcontrol.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficcontrol] ericholguin commented on a diff in pull request #6814: Refactor CacheGroupDeliveryServices tests
Posted by 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
[GitHub] [trafficcontrol] ericholguin commented on a diff in pull request #6814: Refactor CacheGroupDeliveryServices tests
Posted by GitBox <gi...@apache.org>.
ericholguin commented on code in PR #6814:
URL: https://github.com/apache/trafficcontrol/pull/6814#discussion_r869730728
##########
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 {
Review Comment:
No this got moved. Thats being tested by: `BAD REQUEST assigning TOPOLOGY-BASED DS to CACHEGROUP`. This check was looking for a delivery service belonging to cdn1 and had a topology, which is the delivery service with xmlId: top-ds-in-cdn1.
--
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