rimashah25 commented on code in PR #6758:
URL: https://github.com/apache/trafficcontrol/pull/6758#discussion_r861384058


##########
traffic_ops/testing/api/v4/deliveryservice_requests_test.go:
##########
@@ -16,672 +16,356 @@ package v4
 */
 
 import (
+       "encoding/json"
        "net/http"
+       "net/url"
        "strconv"
        "strings"
        "testing"
        "time"
 
        "github.com/apache/trafficcontrol/lib/go-rfc"
-       tc "github.com/apache/trafficcontrol/lib/go-tc"
-       "github.com/apache/trafficcontrol/lib/go-util"
+       "github.com/apache/trafficcontrol/lib/go-tc"
+       "github.com/apache/trafficcontrol/traffic_ops/testing/api/assert"
+       "github.com/apache/trafficcontrol/traffic_ops/testing/api/utils"
+       "github.com/apache/trafficcontrol/traffic_ops/toclientlib"
        client "github.com/apache/trafficcontrol/traffic_ops/v4-client"
 )
 
-const (
-       dsrGood      = 0
-       dsrBadTenant = 1
-       dsrRequired  = 2
-       dsrDraft     = 3
-)
-
-// this resets the IDs of things attached to a DS, which needs to be done
-// because the WithObjs flow destroys and recreates those object IDs
-// non-deterministically with each test - BUT, the client method permanently
-// alters the DSR structures by adding these referential IDs. Older clients
-// got away with it by not making 'DeliveryService' a pointer, but to add
-// original/requested fields you need to sometimes allow each to be nil, so
-// this is a problem that needs to be solved at some point.
-// A better solution _might_ be to reload all the test fixtures every time
-// to wipe any and all referential modifications made to any test data, but
-// for now that's overkill.
-func resetDS(ds *tc.DeliveryServiceV4) {
-       if ds == nil {
-               return
-       }
-       ds.CDNID = nil
-       ds.ID = nil
-       ds.ProfileID = nil
-       ds.TenantID = nil
-       ds.TypeID = nil
-}
-
 func TestDeliveryServiceRequests(t *testing.T) {
        WithObjs(t, []TCObj{CDNs, Types, Parameters, Tenants, 
DeliveryServiceRequests}, func() {
-               GetTestDeliveryServiceRequestsIMS(t)
-               GetTestDeliveryServiceRequests(t)
-               currentTime := time.Now().UTC().Add(-5 * time.Second)
-               time := currentTime.Format(time.RFC1123)
-               var header http.Header
-               header = make(map[string][]string)
-               header.Set(rfc.IfModifiedSince, time)
-               header.Set(rfc.IfUnmodifiedSince, time)
-               UpdateTestDeliveryServiceRequestsWithLongDescFields(t)
-               UpdateTestDeliveryServiceRequests(t)
-               UpdateTestDeliveryServiceRequestsWithHeaders(t, header)
-               GetTestDeliveryServiceRequestsIMSAfterChange(t, header)
-               header = make(map[string][]string)
-               etag := rfc.ETag(currentTime)
-               header.Set(rfc.IfMatch, etag)
-               UpdateTestDeliveryServiceRequestsWithHeaders(t, header)
-       })
-}
-
-func UpdateTestDeliveryServiceRequestsWithLongDescFields(t *testing.T) {
-       if len(testData.DeliveryServiceRequests) < dsrGood+1 {
-               t.Fatalf("Need at least %d Delivery Service Requests to test 
updating them", dsrGood+1)
-       }
-
-       // Retrieve the DeliveryServiceRequest by name so we can get the id for 
the Update
-       dsr := testData.DeliveryServiceRequests[dsrGood]
-       var ds *tc.DeliveryServiceV4
-       if dsr.ChangeType == tc.DSRChangeTypeDelete {
-               dsr.Original.LongDesc1 = util.StrPtr("long desc 1")
-               dsr.Original.LongDesc2 = util.StrPtr("long desc 2")
-               ds = dsr.Original
-       } else {
-               dsr.Requested.LongDesc1 = util.StrPtr("long desc 1")
-               dsr.Requested.LongDesc2 = util.StrPtr("long desc 2")
-               ds = dsr.Requested
-       }
-
-       if ds == nil || ds.XMLID == nil {
-               t.Fatalf("the %dth DSR in the test data had no DeliveryService 
- or that DeliveryService had no XMLID", dsrGood)
-       }
-
-       opts := client.NewRequestOptions()
-       opts.QueryParameters.Set("xmlId", *ds.XMLID)
-       resp, _, err := TOSession.GetDeliveryServiceRequests(opts)
-       if err != nil {
-               t.Errorf("cannot get Delivery Service Request with XMLID '%s': 
%v - alerts: %+v", *ds.XMLID, err, resp.Alerts)
-       }
-       if len(resp.Response) == 0 {
-               t.Fatalf("Expected at least one Deliver Service Request to 
exist with XMLID '%s', but none were found in Traffic Ops", *ds.XMLID)
-       }
-       respDSR := resp.Response[0]
-       if respDSR.ID == nil {
-               t.Fatalf("got a DSR by XMLID '%s' with a null or undefined ID", 
*ds.XMLID)
-       }
-       var respDS *tc.DeliveryServiceV4
-       if dsr.ChangeType == tc.DSRChangeTypeDelete {
-               respDS = dsr.Original
-               respDSR.Original = respDS
-       } else {
-               respDS = dsr.Requested
-               respDSR.Requested = respDS
-       }
-       expDisplayName := "new display name"
-       respDS.DisplayName = &expDisplayName
-       id := *respDSR.ID
-       _, reqInf, err := TOSession.UpdateDeliveryServiceRequest(id, respDSR, 
client.RequestOptions{})
-       if err == nil {
-               t.Errorf("expected an error stating that Long Desc 1 and Long 
Desc 2 fields are not supported in api version 4.0 onwards, but got nothing")
-       }
-       if reqInf.StatusCode != http.StatusBadRequest {
-               t.Errorf("expected a 400 status code, but got %d", 
reqInf.StatusCode)
-       }
-}
-
-// Note that this test is susceptible to breaking if the structure of the test
-// data's DSRs is altered at all.
-func UpdateTestDeliveryServiceRequestsWithHeaders(t *testing.T, header 
http.Header) {
-       if len(testData.DeliveryServiceRequests) < dsrGood+1 {
-               t.Fatalf("Need at least %d Delivery Service Requests to test 
updating them using headers", dsrGood+1)
-       }
-       // Retrieve the DeliveryServiceRequest by name so we can get the id for 
the Update
-       dsr := testData.DeliveryServiceRequests[dsrGood]
-       var ds *tc.DeliveryServiceV4
-       if dsr.ChangeType == tc.DSRChangeTypeDelete {
-               ds = dsr.Original
-       } else {
-               ds = dsr.Requested
-       }
-       resetDS(ds)
-       if ds == nil || ds.XMLID == nil {
-               t.Fatalf("the %dth DSR in the test data had no Delivery 
Service, or that Delivery Service had a null or undefined XMLID", dsrGood)
-       }
-       opts := client.NewRequestOptions()
-       opts.Header = header
-       opts.QueryParameters.Set("xmlId", *ds.XMLID)
-       resp, _, err := TOSession.GetDeliveryServiceRequests(opts)
-       if err != nil {
-               t.Errorf("cannot get Delivery Service Request by XMLID '%s': %v 
- alerts: %+v", *ds.XMLID, err, resp.Alerts)
-       }
-       if len(resp.Response) == 0 {
-               t.Fatal("Length of GET DeliveryServiceRequest is 0")
-       }
-       respDSR := resp.Response[0]
-       if respDSR.ID == nil {
-               t.Fatalf("Got a DSR for XML ID '%s' that had a nil ID", 
*ds.XMLID)
-       }
-       if respDSR.ChangeType != dsr.ChangeType {
-               t.Fatalf("remote representation of DSR with XMLID '%s' differed 
from stored data", *ds.XMLID)
-       }
-       var respDS *tc.DeliveryServiceV4
-       if respDSR.ChangeType == tc.DSRChangeTypeDelete {
-               respDS = respDSR.Original
-       } else {
-               respDS = respDSR.Requested
-       }
-
-       respDS.DisplayName = new(string)
-       *respDS.DisplayName = "new display name"
-       opts.QueryParameters.Del("xmlId")
-       _, reqInf, err := TOSession.UpdateDeliveryServiceRequest(*respDSR.ID, 
respDSR, opts)
-       if err == nil {
-               t.Errorf("Expected error about precondition failed, but got 
none")
-       }
-       if reqInf.StatusCode != http.StatusPreconditionFailed {
-               t.Errorf("Expected status code 412, got %v", reqInf.StatusCode)
-       }
-}
-
-// Note that this test is susceptible to breaking if the structure of the test
-// data's DSRs is altered at all.
-func GetTestDeliveryServiceRequestsIMSAfterChange(t *testing.T, header 
http.Header) {
-       if len(testData.DeliveryServiceRequests) < dsrGood+1 {
-               t.Fatalf("Need at least %d Delivery Service Requests to test 
updating them with IMS", dsrGood+1)
-       }
-       dsr := testData.DeliveryServiceRequests[dsrGood]
-       var ds *tc.DeliveryServiceV4
-       if dsr.ChangeType == tc.DSRChangeTypeDelete {
-               ds = dsr.Original
-       } else {
-               ds = dsr.Requested
-       }
-
-       resetDS(ds)
-       if ds == nil || ds.XMLID == nil {
-               t.Fatalf("the %dth DSR in the test data had no Delivery 
Service, or that Delivery Service had a null or undefined XMLID", dsrGood)
-       }
-
-       opts := client.NewRequestOptions()
-       opts.Header = header
-       opts.QueryParameters.Set("xmlId", *ds.XMLID)
-       resp, reqInf, err := TOSession.GetDeliveryServiceRequests(opts)
-       if err != nil {
-               t.Fatalf("Expected no error, but got: %v - alerts: %+v", err, 
resp.Alerts)
-       }
-       if reqInf.StatusCode != http.StatusOK {
-               t.Fatalf("Expected 200 status code, got %v", reqInf.StatusCode)
-       }
-       currentTime := time.Now().UTC()
-       currentTime = currentTime.Add(1 * time.Second)
-       timeStr := currentTime.Format(time.RFC1123)
-       opts.Header.Set(rfc.IfModifiedSince, timeStr)
-       resp, reqInf, err = TOSession.GetDeliveryServiceRequests(opts)
-       if err != nil {
-               t.Fatalf("Expected no error, but got: %v - alerts: %+v", err, 
resp.Alerts)
-       }
-       if reqInf.StatusCode != http.StatusNotModified {
-               t.Fatalf("Expected 304 status code, got %v", reqInf.StatusCode)
-       }
-}
 
-// Note that this test is susceptible to breaking if the structure of the test
-// data's DSRs is altered at all.
-func CreateTestDeliveryServiceRequests(t *testing.T) {
-       if len(testData.DeliveryServiceRequests) < dsrGood+1 {
-               t.Fatalf("Need at least %d Delivery Service Requests to test 
creating Delivery Service Requests", dsrGood+1)
-       }
-       dsr := testData.DeliveryServiceRequests[dsrGood]
-       resetDS(dsr.Original)
-       resetDS(dsr.Requested)
-       respDSR, _, err := TOSession.CreateDeliveryServiceRequest(dsr, 
client.RequestOptions{})
-       if err != nil {
-               t.Errorf("could not create Delivery Service Requests: %v - 
alerts: %+v", err, respDSR.Alerts)
-       }
-
-}
-
-// Note that this test is susceptible to breaking if the structure of the test
-// data's DSRs is altered at all.
-func TestDeliveryServiceRequestRequired(t *testing.T) {
-       WithObjs(t, []TCObj{CDNs, Types, Parameters, Tenants}, func() {
-               if len(testData.DeliveryServiceRequests) < dsrRequired+1 {
-                       t.Fatalf("Need at least %d Delivery Service Requests to 
test creating a Delivery Service Request missing required fields", 
dsrRequired+1)
+               currentTime := time.Now().UTC().Add(-15 * time.Second)
+               currentTimeRFC := currentTime.Format(time.RFC1123)
+               tomorrow := currentTime.AddDate(0, 0, 1).Format(time.RFC1123)
+
+               methodTests := utils.V4TestCase{
+                       "GET": {
+                               "NOT MODIFIED when NO CHANGES made": {
+                                       ClientSession: TOSession,
+                                       RequestOpts:   
client.RequestOptions{Header: http.Header{rfc.IfModifiedSince: {tomorrow}}},
+                                       Expectations:  
utils.CkRequest(utils.NoError(), utils.HasStatus(http.StatusNotModified)),
+                               },
+                               "OK when VALID XMLID parameter": {
+                                       ClientSession: TOSession,
+                                       RequestOpts:   
client.RequestOptions{QueryParameters: url.Values{"xmlId": {"test-ds1"}}},
+                                       Expectations: 
utils.CkRequest(utils.NoError(), utils.HasStatus(http.StatusOK), 
utils.ResponseLengthGreaterOrEqual(1),
+                                               
validateGetDSRequestFields(map[string]interface{}{"XMLID": "test-ds1"})),
+                               },
+                       },
+                       "PUT": {
+                               "OK when VALID request": {
+                                       EndpointId:    
GetDeliveryServiceRequestId(t, "test-ds1"),
+                                       ClientSession: TOSession,
+                                       RequestBody: map[string]interface{}{
+                                               "changeType": "create",

Review Comment:
   I am a bit confused. Why is the changeType not `update` but `create` in PUT 
requests?



-- 
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