TaylorCFrey commented on a change in pull request #6690:
URL: https://github.com/apache/trafficcontrol/pull/6690#discussion_r838952223
##########
File path: traffic_ops/testing/api/v4/deliveryservices_test.go
##########
@@ -17,2896 +17,772 @@ package v4
import (
"encoding/json"
- "fmt"
"net/http"
"net/url"
- "reflect"
"strconv"
- "strings"
"testing"
"time"
"github.com/apache/trafficcontrol/lib/go-rfc"
"github.com/apache/trafficcontrol/lib/go-tc"
- "github.com/apache/trafficcontrol/lib/go-util"
-
"github.com/apache/trafficcontrol/traffic_ops/traffic_ops_golang/deliveryservice"
+ "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"
)
func TestDeliveryServices(t *testing.T) {
Review comment:
This is just a general observation and question, but not a requirement
for any change.
I don't believe this PR changes how these tests are necessarily run. Even
though the previous tests were written as individual tests, they still had a
hard requirement and dependency on `WithObjs()`. It appeared like we could have
run those tests individually, but with their dependency on the objects being
initialized ahead of time, they couldn't actually be done that way.
Since these are now table tests, and the dependency on `WithObjs` is still
there, we still don't really have the ability to run these individually, is
that correct?
##########
File path: traffic_ops/testing/api/v3/deliveryservices_test.go
##########
@@ -17,1180 +17,499 @@ package v3
import (
"encoding/json"
- "fmt"
"net/http"
"net/url"
- "reflect"
"strconv"
- "strings"
"testing"
"time"
"github.com/apache/trafficcontrol/lib/go-rfc"
"github.com/apache/trafficcontrol/lib/go-tc"
- "github.com/apache/trafficcontrol/lib/go-util"
- toclient "github.com/apache/trafficcontrol/traffic_ops/v3-client"
+ "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"
)
func TestDeliveryServices(t *testing.T) {
- WithObjs(t, []TCObj{CDNs, Types, Tenants, Users, Parameters, Profiles,
Statuses, Divisions, Regions, PhysLocations, CacheGroups, Servers, Topologies,
ServerCapabilities, DeliveryServices}, func() {
- currentTime := time.Now().UTC().Add(-5 * time.Second)
- ti := currentTime.Format(time.RFC1123)
- var header http.Header
- header = make(map[string][]string)
- header.Set(rfc.IfModifiedSince, ti)
- header.Set(rfc.IfUnmodifiedSince, ti)
- if includeSystemTests {
- SSLDeliveryServiceCDNUpdateTest(t)
- GetTestDeliveryServicesURLSigKeys(t)
+ WithObjs(t, []TCObj{CDNs, Types, Tenants, Users, Parameters, Profiles,
Statuses, Divisions, Regions, PhysLocations, CacheGroups, Servers, Topologies,
ServerCapabilities, DeliveryServices, DeliveryServicesRequiredCapabilities,
DeliveryServiceServerAssignments}, func() {
+
+ tomorrow := time.Now().AddDate(0, 0, 1).Format(time.RFC1123)
+ currentTime := time.Now().UTC().Add(-15 * time.Second)
+ currentTimeRFC := currentTime.Format(time.RFC1123)
+
+ tenant4UserSession := utils.CreateV3Session(t,
Config.TrafficOps.URL, "tenant4user", "pa$$word",
Config.Default.Session.TimeoutInSecs)
+
+ methodTests := utils.V3TestCase{
+ "GET": {
+ "NOT MODIFIED when NO CHANGES made": {
+ ClientSession: TOSession,
RequestHeaders: http.Header{rfc.IfModifiedSince: {tomorrow}},
+ Expectations:
utils.CkRequest(utils.NoError(), utils.HasStatus(http.StatusNotModified)),
+ },
+ "OK when VALID request": {
+ ClientSession: TOSession, Expectations:
utils.CkRequest(utils.NoError(), utils.HasStatus(http.StatusOK)),
+ },
+ "OK when ACTIVE=TRUE": {
+ ClientSession: TOSession,
RequestParams: url.Values{"active": {"true"}},
+ Expectations:
utils.CkRequest(utils.NoError(), utils.HasStatus(http.StatusOK),
utils.ResponseLengthGreaterOrEqual(1),
+
validateGetDSExpectedFields(map[string]interface{}{"Active": true})),
+ },
+ "OK when ACTIVE=FALSE": {
+ ClientSession: TOSession,
RequestParams: url.Values{"active": {"false"}},
+ Expectations:
utils.CkRequest(utils.NoError(), utils.HasStatus(http.StatusOK),
utils.ResponseLengthGreaterOrEqual(1),
+
validateGetDSExpectedFields(map[string]interface{}{"Active": false})),
+ },
+ "OK when VALID ACCESSIBLETO parameter": {
+ ClientSession: TOSession,
RequestParams: url.Values{"accessibleTo": {"1"}},
+ Expectations:
utils.CkRequest(utils.NoError(), utils.HasStatus(http.StatusOK),
utils.ResponseLengthGreaterOrEqual(1)),
+ },
+ "EMPTY RESPONSE when TENANT attempts reading DS
OUTSIDE TENANCY": {
+ ClientSession: tenant4UserSession,
RequestParams: url.Values{"xmlId": {"ds3"}},
+ Expectations:
utils.CkRequest(utils.NoError(), utils.HasStatus(http.StatusOK),
utils.ResponseHasLength(0)),
+ },
+ },
+ "POST": {
Review comment:
This applies to both v3 and v4 test suites:
Does the order of these tests matter? Especially for endpoints that aren't
idempotent (`CUD`, but not `R`)? Granted it wouldn't necessarily be prudent to
call `DELETE` randomly, but I mean does the order matter in, say, all the
`POST` or `PUT` requests?
##########
File path: traffic_ops/testing/api/v3/deliveryservices_test.go
##########
@@ -17,1180 +17,499 @@ package v3
import (
"encoding/json"
- "fmt"
"net/http"
"net/url"
- "reflect"
"strconv"
- "strings"
"testing"
"time"
"github.com/apache/trafficcontrol/lib/go-rfc"
"github.com/apache/trafficcontrol/lib/go-tc"
- "github.com/apache/trafficcontrol/lib/go-util"
- toclient "github.com/apache/trafficcontrol/traffic_ops/v3-client"
+ "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"
)
func TestDeliveryServices(t *testing.T) {
- WithObjs(t, []TCObj{CDNs, Types, Tenants, Users, Parameters, Profiles,
Statuses, Divisions, Regions, PhysLocations, CacheGroups, Servers, Topologies,
ServerCapabilities, DeliveryServices}, func() {
- currentTime := time.Now().UTC().Add(-5 * time.Second)
- ti := currentTime.Format(time.RFC1123)
- var header http.Header
- header = make(map[string][]string)
- header.Set(rfc.IfModifiedSince, ti)
- header.Set(rfc.IfUnmodifiedSince, ti)
- if includeSystemTests {
- SSLDeliveryServiceCDNUpdateTest(t)
- GetTestDeliveryServicesURLSigKeys(t)
+ WithObjs(t, []TCObj{CDNs, Types, Tenants, Users, Parameters, Profiles,
Statuses, Divisions, Regions, PhysLocations, CacheGroups, Servers, Topologies,
ServerCapabilities, DeliveryServices, DeliveryServicesRequiredCapabilities,
DeliveryServiceServerAssignments}, func() {
+
+ tomorrow := time.Now().AddDate(0, 0, 1).Format(time.RFC1123)
+ currentTime := time.Now().UTC().Add(-15 * time.Second)
+ currentTimeRFC := currentTime.Format(time.RFC1123)
+
+ tenant4UserSession := utils.CreateV3Session(t,
Config.TrafficOps.URL, "tenant4user", "pa$$word",
Config.Default.Session.TimeoutInSecs)
+
+ methodTests := utils.V3TestCase{
+ "GET": {
Review comment:
It might behove us to make these HTTP verbs and switch strings `const`
somewhere
```go
const (
Get = "GET"
Post = "POST"
GetAfterChanges = "GET AFTER CHANGES"
// ...
)
```
Go's capitalization results in scope changes so the convention of ALL CAPS
may not be appropriate, but these could be put somewhere so they are accessible
everywhere? I will leave that decision up to you. Maybe the HTTP verbs could be
someone globally accessible, but the delivery service specific (`DELIVERY
SERVICES CAPACITY`) could be local?
I won't put this comment on every verb, but it applies for `GET` `POST`
`PUT` `DELETE` etc.
##########
File path: traffic_ops/testing/api/v3/deliveryservices_test.go
##########
@@ -17,1180 +17,490 @@ package v3
import (
"encoding/json"
- "fmt"
"net/http"
"net/url"
- "reflect"
"strconv"
- "strings"
"testing"
"time"
"github.com/apache/trafficcontrol/lib/go-rfc"
"github.com/apache/trafficcontrol/lib/go-tc"
- "github.com/apache/trafficcontrol/lib/go-util"
- toclient "github.com/apache/trafficcontrol/traffic_ops/v3-client"
+ "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"
)
func TestDeliveryServices(t *testing.T) {
- WithObjs(t, []TCObj{CDNs, Types, Tenants, Users, Parameters, Profiles,
Statuses, Divisions, Regions, PhysLocations, CacheGroups, Servers, Topologies,
ServerCapabilities, DeliveryServices}, func() {
- currentTime := time.Now().UTC().Add(-5 * time.Second)
- ti := currentTime.Format(time.RFC1123)
- var header http.Header
- header = make(map[string][]string)
- header.Set(rfc.IfModifiedSince, ti)
- header.Set(rfc.IfUnmodifiedSince, ti)
- if includeSystemTests {
- SSLDeliveryServiceCDNUpdateTest(t)
- GetTestDeliveryServicesURLSigKeys(t)
+ WithObjs(t, []TCObj{CDNs, Types, Tenants, Users, Parameters, Profiles,
Statuses, Divisions, Regions, PhysLocations, CacheGroups, Servers, Topologies,
ServerCapabilities, DeliveryServices, DeliveryServicesRequiredCapabilities,
DeliveryServiceServerAssignments}, func() {
+
+ tomorrow := time.Now().AddDate(0, 0, 1).Format(time.RFC1123)
+ currentTime := time.Now().UTC().Add(-15 * time.Second)
+ currentTimeRFC := currentTime.Format(time.RFC1123)
Review comment:
In general, when I'm writing tests that deal with time.Now() I try to
grab it once, then reuse it to ensure it doesn't actually change on me with how
fast some processing can happen. Calls that are immediately executed could
still have different _now_ times. So I'll do something like:
```go
currentTime := time.Now().UTC()
currentTimeRFC := currentTime.Format(time.RFC1123)
tomorrow := currentTime.AddDate(0,0,1).Format(time.RFC1123) // Ensures there
aren't nanoseconds worth of time that throws off the +1 day or equality later
on, if need be
...
```
--
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]
