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/26 20:52:31 UTC
[GitHub] [trafficcontrol] ericholguin opened a new pull request, #6864: Refactor coordinates test
ericholguin opened a new pull request, #6864:
URL: https://github.com/apache/trafficcontrol/pull/6864
<!--
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/coordinates_test.go**, **v3/coordinates_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 Tests
## 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] mattjackson220 merged pull request #6864: Refactor coordinates test
Posted by GitBox <gi...@apache.org>.
mattjackson220 merged PR #6864:
URL: https://github.com/apache/trafficcontrol/pull/6864
--
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] mattjackson220 commented on a diff in pull request #6864: Refactor coordinates test
Posted by GitBox <gi...@apache.org>.
mattjackson220 commented on code in PR #6864:
URL: https://github.com/apache/trafficcontrol/pull/6864#discussion_r888435991
##########
traffic_ops/testing/api/v3/coordinates_test.go:
##########
@@ -16,185 +16,211 @@ package v3
*/
import (
+ "encoding/json"
"net/http"
+ "net/url"
"sort"
"testing"
"time"
"github.com/apache/trafficcontrol/lib/go-rfc"
- tc "github.com/apache/trafficcontrol/lib/go-tc"
+ "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"
)
func TestCoordinates(t *testing.T) {
WithObjs(t, []TCObj{Parameters, Coordinates}, func() {
- GetTestCoordinatesIMS(t)
- GetTestCoordinates(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)
- SortTestCoordinates(t)
- UpdateTestCoordinates(t)
- UpdateTestCoordinatesWithHeaders(t, header)
- header = make(map[string][]string)
- etag := rfc.ETag(currentTime)
- header.Set(rfc.IfMatch, etag)
- UpdateTestCoordinatesWithHeaders(t, header)
- GetTestCoordinatesIMSAfterChange(t, header)
- })
-}
-func UpdateTestCoordinatesWithHeaders(t *testing.T, header http.Header) {
- firstCoord := testData.Coordinates[0]
- resp, _, err := TOSession.GetCoordinateByNameWithHdr(firstCoord.Name, header)
- if err != nil {
- t.Errorf("cannot GET Coordinate by name: %v - %v", firstCoord.Name, err)
- }
- if len(resp) > 0 {
- coord := resp[0]
- expectedLat := 12.34
- coord.Latitude = expectedLat
-
- _, reqInf, err := TOSession.UpdateCoordinateByIDWithHdr(coord.ID, coord, header)
- 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)
+ currentTime := time.Now().UTC().Add(-15 * time.Second)
+ currentTimeRFC := currentTime.Format(time.RFC1123)
+ tomorrow := currentTime.AddDate(0, 0, 1).Format(time.RFC1123)
+
+ 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), validateCoordinateSort()),
+ },
+ "OK when VALID NAME parameter": {
+ ClientSession: TOSession,
+ RequestParams: url.Values{"name": {"coordinate1"}},
+ Expectations: utils.CkRequest(utils.NoError(), utils.HasStatus(http.StatusOK), utils.ResponseHasLength(1),
+ validateCoordinateFields(map[string]interface{}{"Name": "coordinate1"})),
+ },
+ },
+ "PUT": {
+ "OK when VALID request": {
+ EndpointId: GetCoordinateID(t, "coordinate2"),
+ ClientSession: TOSession,
+ RequestBody: map[string]interface{}{
+ "latitude": 7.7,
+ "longitude": 8.8,
+ "name": "coordinate2",
+ },
+ Expectations: utils.CkRequest(utils.NoError(), utils.HasStatus(http.StatusOK),
+ validateCoordinateUpdateCreateFields("coordinate2", map[string]interface{}{"Latitude": 7.7, "Longitude": 8.8})),
+ },
+ "PRECONDITION FAILED when updating with IMS & IUS Headers": {
+ EndpointId: GetCoordinateID(t, "coordinate1"),
+ ClientSession: TOSession,
+ RequestHeaders: http.Header{rfc.IfUnmodifiedSince: {currentTimeRFC}},
+ RequestBody: map[string]interface{}{
+ "latitude": 1.1,
+ "longitude": 2.2,
+ "name": "coordinate1",
+ },
+ Expectations: utils.CkRequest(utils.HasError(), utils.HasStatus(http.StatusPreconditionFailed)),
+ },
+ "PRECONDITION FAILED when updating with IFMATCH ETAG Header": {
+ EndpointId: GetCoordinateID(t, "coordinate1"),
+ ClientSession: TOSession,
+ RequestBody: map[string]interface{}{
+ "latitude": 1.1,
+ "longitude": 2.2,
+ "name": "coordinate1",
+ },
+ RequestHeaders: http.Header{rfc.IfMatch: {rfc.ETag(currentTime)}},
+ Expectations: utils.CkRequest(utils.HasError(), utils.HasStatus(http.StatusPreconditionFailed)),
+ },
+ },
+ "GET AFTER CHANGES": {
+ "OK when CHANGES made": {
+ ClientSession: TOSession,
+ RequestHeaders: http.Header{rfc.IfModifiedSince: {currentTimeRFC}},
+ Expectations: utils.CkRequest(utils.NoError(), utils.HasStatus(http.StatusOK)),
+ },
+ },
}
- }
-}
-func GetTestCoordinatesIMSAfterChange(t *testing.T, header http.Header) {
- for _, coord := range testData.Coordinates {
- _, reqInf, err := TOSession.GetCoordinateByNameWithHdr(coord.Name, header)
- if err != nil {
- t.Fatalf("could not GET coordinates: %v", err)
- }
- if reqInf.StatusCode != http.StatusOK {
- t.Fatalf("Expected 304 status code, got %v", reqInf.StatusCode)
- }
- }
- currentTime := time.Now().UTC()
- currentTime = currentTime.Add(1 * time.Second)
- timeStr := currentTime.Format(time.RFC1123)
- header.Set(rfc.IfModifiedSince, timeStr)
- for _, coord := range testData.Coordinates {
- _, reqInf, err := TOSession.GetCoordinateByNameWithHdr(coord.Name, header)
- if err != nil {
- t.Fatalf("could not GET coordinates: %v", err)
- }
- if reqInf.StatusCode != http.StatusNotModified {
- t.Fatalf("Expected 304 status code, got %v", reqInf.StatusCode)
+ for method, testCases := range methodTests {
+ t.Run(method, func(t *testing.T) {
+ for name, testCase := range testCases {
+ coordinate := tc.Coordinate{}
+
+ if testCase.RequestBody != nil {
+ dat, err := json.Marshal(testCase.RequestBody)
+ assert.NoError(t, err, "Error occurred when marshalling request body: %v", err)
+ err = json.Unmarshal(dat, &coordinate)
+ assert.NoError(t, err, "Error occurred when unmarshalling request body: %v", err)
+ }
+
+ switch method {
+ case "GET", "GET AFTER CHANGES":
+ t.Run(name, func(t *testing.T) {
+ if name == "OK when VALID NAME parameter" {
+ resp, reqInf, err := testCase.ClientSession.GetCoordinateByNameWithHdr(testCase.RequestParams["name"][0], testCase.RequestHeaders)
+ for _, check := range testCase.Expectations {
+ check(t, reqInf, resp, tc.Alerts{}, err)
+ }
+ } else {
+ resp, reqInf, err := testCase.ClientSession.GetCoordinatesWithHdr(testCase.RequestHeaders)
+ for _, check := range testCase.Expectations {
+ check(t, reqInf, resp, tc.Alerts{}, err)
+ }
+ }
+ })
+ case "POST":
+ t.Run(name, func(t *testing.T) {
+ alerts, reqInf, err := testCase.ClientSession.CreateCoordinate(coordinate)
+ for _, check := range testCase.Expectations {
+ check(t, reqInf, nil, alerts, err)
+ }
+ })
+ case "PUT":
+ t.Run(name, func(t *testing.T) {
+ alerts, reqInf, err := testCase.ClientSession.UpdateCoordinateByIDWithHdr(testCase.EndpointId(), coordinate, testCase.RequestHeaders)
+ for _, check := range testCase.Expectations {
+ check(t, reqInf, nil, alerts, err)
+ }
+ })
+ case "DELETE":
+ t.Run(name, func(t *testing.T) {
+ alerts, reqInf, err := testCase.ClientSession.DeleteCoordinateByID(testCase.EndpointId())
+ for _, check := range testCase.Expectations {
+ check(t, reqInf, nil, alerts, err)
+ }
+ })
+ }
+ }
+ })
}
- }
+ })
}
-func GetTestCoordinatesIMS(t *testing.T) {
- var header http.Header
- header = make(map[string][]string)
- futureTime := time.Now().AddDate(0, 0, 1)
- time := futureTime.Format(time.RFC1123)
- header.Set(rfc.IfModifiedSince, time)
- for _, coord := range testData.Coordinates {
- _, reqInf, err := TOSession.GetCoordinateByNameWithHdr(coord.Name, header)
- if err != nil {
- t.Fatalf("No error expected, but got: %v", err)
- }
- if reqInf.StatusCode != http.StatusNotModified {
- t.Fatalf("Expected 304 status code, got %v", reqInf.StatusCode)
+func validateCoordinateFields(expectedResp map[string]interface{}) utils.CkReqFunc {
+ return func(t *testing.T, _ toclientlib.ReqInf, resp interface{}, _ tc.Alerts, _ error) {
+ assert.RequireNotNil(t, resp, "Expected Coordinate response to not be nil.")
+ coordinateResp := resp.([]tc.Coordinate)
+ for field, expected := range expectedResp {
+ for _, coordinate := range coordinateResp {
+ switch field {
+ case "Name":
+ assert.Equal(t, expected, coordinate.Name, "Expected Name to be %v, but got %s", expected, coordinate.Name)
+ case "Latitude":
+ assert.Equal(t, expected, coordinate.Latitude, "Expected Latitude to be %v, but got %s", expected, coordinate.Latitude)
+ case "Longitude":
+ assert.Equal(t, expected, coordinate.Longitude, "Expected Longitude to be %v, but got %s", expected, coordinate.Longitude)
Review Comment:
Latitude and Longitude are float64 so this should be %f instead of %s
##########
traffic_ops/testing/api/v3/coordinates_test.go:
##########
@@ -16,185 +16,211 @@ package v3
*/
import (
+ "encoding/json"
"net/http"
+ "net/url"
"sort"
"testing"
"time"
"github.com/apache/trafficcontrol/lib/go-rfc"
- tc "github.com/apache/trafficcontrol/lib/go-tc"
+ "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"
)
func TestCoordinates(t *testing.T) {
WithObjs(t, []TCObj{Parameters, Coordinates}, func() {
- GetTestCoordinatesIMS(t)
- GetTestCoordinates(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)
- SortTestCoordinates(t)
- UpdateTestCoordinates(t)
- UpdateTestCoordinatesWithHeaders(t, header)
- header = make(map[string][]string)
- etag := rfc.ETag(currentTime)
- header.Set(rfc.IfMatch, etag)
- UpdateTestCoordinatesWithHeaders(t, header)
- GetTestCoordinatesIMSAfterChange(t, header)
- })
-}
-func UpdateTestCoordinatesWithHeaders(t *testing.T, header http.Header) {
- firstCoord := testData.Coordinates[0]
- resp, _, err := TOSession.GetCoordinateByNameWithHdr(firstCoord.Name, header)
- if err != nil {
- t.Errorf("cannot GET Coordinate by name: %v - %v", firstCoord.Name, err)
- }
- if len(resp) > 0 {
- coord := resp[0]
- expectedLat := 12.34
- coord.Latitude = expectedLat
-
- _, reqInf, err := TOSession.UpdateCoordinateByIDWithHdr(coord.ID, coord, header)
- 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)
+ currentTime := time.Now().UTC().Add(-15 * time.Second)
+ currentTimeRFC := currentTime.Format(time.RFC1123)
+ tomorrow := currentTime.AddDate(0, 0, 1).Format(time.RFC1123)
+
+ 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), validateCoordinateSort()),
+ },
+ "OK when VALID NAME parameter": {
+ ClientSession: TOSession,
+ RequestParams: url.Values{"name": {"coordinate1"}},
+ Expectations: utils.CkRequest(utils.NoError(), utils.HasStatus(http.StatusOK), utils.ResponseHasLength(1),
+ validateCoordinateFields(map[string]interface{}{"Name": "coordinate1"})),
+ },
+ },
+ "PUT": {
+ "OK when VALID request": {
+ EndpointId: GetCoordinateID(t, "coordinate2"),
+ ClientSession: TOSession,
+ RequestBody: map[string]interface{}{
+ "latitude": 7.7,
+ "longitude": 8.8,
+ "name": "coordinate2",
+ },
+ Expectations: utils.CkRequest(utils.NoError(), utils.HasStatus(http.StatusOK),
+ validateCoordinateUpdateCreateFields("coordinate2", map[string]interface{}{"Latitude": 7.7, "Longitude": 8.8})),
+ },
+ "PRECONDITION FAILED when updating with IMS & IUS Headers": {
+ EndpointId: GetCoordinateID(t, "coordinate1"),
+ ClientSession: TOSession,
+ RequestHeaders: http.Header{rfc.IfUnmodifiedSince: {currentTimeRFC}},
+ RequestBody: map[string]interface{}{
+ "latitude": 1.1,
+ "longitude": 2.2,
+ "name": "coordinate1",
+ },
+ Expectations: utils.CkRequest(utils.HasError(), utils.HasStatus(http.StatusPreconditionFailed)),
+ },
+ "PRECONDITION FAILED when updating with IFMATCH ETAG Header": {
+ EndpointId: GetCoordinateID(t, "coordinate1"),
+ ClientSession: TOSession,
+ RequestBody: map[string]interface{}{
+ "latitude": 1.1,
+ "longitude": 2.2,
+ "name": "coordinate1",
+ },
+ RequestHeaders: http.Header{rfc.IfMatch: {rfc.ETag(currentTime)}},
+ Expectations: utils.CkRequest(utils.HasError(), utils.HasStatus(http.StatusPreconditionFailed)),
+ },
+ },
+ "GET AFTER CHANGES": {
+ "OK when CHANGES made": {
+ ClientSession: TOSession,
+ RequestHeaders: http.Header{rfc.IfModifiedSince: {currentTimeRFC}},
+ Expectations: utils.CkRequest(utils.NoError(), utils.HasStatus(http.StatusOK)),
+ },
+ },
}
- }
-}
-func GetTestCoordinatesIMSAfterChange(t *testing.T, header http.Header) {
- for _, coord := range testData.Coordinates {
- _, reqInf, err := TOSession.GetCoordinateByNameWithHdr(coord.Name, header)
- if err != nil {
- t.Fatalf("could not GET coordinates: %v", err)
- }
- if reqInf.StatusCode != http.StatusOK {
- t.Fatalf("Expected 304 status code, got %v", reqInf.StatusCode)
- }
- }
- currentTime := time.Now().UTC()
- currentTime = currentTime.Add(1 * time.Second)
- timeStr := currentTime.Format(time.RFC1123)
- header.Set(rfc.IfModifiedSince, timeStr)
- for _, coord := range testData.Coordinates {
- _, reqInf, err := TOSession.GetCoordinateByNameWithHdr(coord.Name, header)
- if err != nil {
- t.Fatalf("could not GET coordinates: %v", err)
- }
- if reqInf.StatusCode != http.StatusNotModified {
- t.Fatalf("Expected 304 status code, got %v", reqInf.StatusCode)
+ for method, testCases := range methodTests {
+ t.Run(method, func(t *testing.T) {
+ for name, testCase := range testCases {
+ coordinate := tc.Coordinate{}
+
+ if testCase.RequestBody != nil {
+ dat, err := json.Marshal(testCase.RequestBody)
+ assert.NoError(t, err, "Error occurred when marshalling request body: %v", err)
+ err = json.Unmarshal(dat, &coordinate)
+ assert.NoError(t, err, "Error occurred when unmarshalling request body: %v", err)
+ }
+
+ switch method {
+ case "GET", "GET AFTER CHANGES":
+ t.Run(name, func(t *testing.T) {
+ if name == "OK when VALID NAME parameter" {
+ resp, reqInf, err := testCase.ClientSession.GetCoordinateByNameWithHdr(testCase.RequestParams["name"][0], testCase.RequestHeaders)
+ for _, check := range testCase.Expectations {
+ check(t, reqInf, resp, tc.Alerts{}, err)
+ }
+ } else {
+ resp, reqInf, err := testCase.ClientSession.GetCoordinatesWithHdr(testCase.RequestHeaders)
+ for _, check := range testCase.Expectations {
+ check(t, reqInf, resp, tc.Alerts{}, err)
+ }
+ }
+ })
+ case "POST":
+ t.Run(name, func(t *testing.T) {
+ alerts, reqInf, err := testCase.ClientSession.CreateCoordinate(coordinate)
+ for _, check := range testCase.Expectations {
+ check(t, reqInf, nil, alerts, err)
+ }
+ })
+ case "PUT":
+ t.Run(name, func(t *testing.T) {
+ alerts, reqInf, err := testCase.ClientSession.UpdateCoordinateByIDWithHdr(testCase.EndpointId(), coordinate, testCase.RequestHeaders)
+ for _, check := range testCase.Expectations {
+ check(t, reqInf, nil, alerts, err)
+ }
+ })
+ case "DELETE":
+ t.Run(name, func(t *testing.T) {
+ alerts, reqInf, err := testCase.ClientSession.DeleteCoordinateByID(testCase.EndpointId())
+ for _, check := range testCase.Expectations {
+ check(t, reqInf, nil, alerts, err)
+ }
+ })
+ }
+ }
+ })
}
- }
+ })
}
-func GetTestCoordinatesIMS(t *testing.T) {
- var header http.Header
- header = make(map[string][]string)
- futureTime := time.Now().AddDate(0, 0, 1)
- time := futureTime.Format(time.RFC1123)
- header.Set(rfc.IfModifiedSince, time)
- for _, coord := range testData.Coordinates {
- _, reqInf, err := TOSession.GetCoordinateByNameWithHdr(coord.Name, header)
- if err != nil {
- t.Fatalf("No error expected, but got: %v", err)
- }
- if reqInf.StatusCode != http.StatusNotModified {
- t.Fatalf("Expected 304 status code, got %v", reqInf.StatusCode)
+func validateCoordinateFields(expectedResp map[string]interface{}) utils.CkReqFunc {
+ return func(t *testing.T, _ toclientlib.ReqInf, resp interface{}, _ tc.Alerts, _ error) {
+ assert.RequireNotNil(t, resp, "Expected Coordinate response to not be nil.")
+ coordinateResp := resp.([]tc.Coordinate)
+ for field, expected := range expectedResp {
+ for _, coordinate := range coordinateResp {
+ switch field {
+ case "Name":
+ assert.Equal(t, expected, coordinate.Name, "Expected Name to be %v, but got %s", expected, coordinate.Name)
+ case "Latitude":
+ assert.Equal(t, expected, coordinate.Latitude, "Expected Latitude to be %v, but got %s", expected, coordinate.Latitude)
+ case "Longitude":
+ assert.Equal(t, expected, coordinate.Longitude, "Expected Longitude to be %v, but got %s", expected, coordinate.Longitude)
+ default:
+ t.Errorf("Expected field: %v, does not exist in response", field)
+ }
+ }
}
}
}
-func CreateTestCoordinates(t *testing.T) {
- for _, coord := range testData.Coordinates {
-
- _, _, err := TOSession.CreateCoordinate(coord)
- if err != nil {
- t.Errorf("could not CREATE coordinates: %v", err)
- }
+func validateCoordinateUpdateCreateFields(name string, expectedResp map[string]interface{}) utils.CkReqFunc {
+ return func(t *testing.T, _ toclientlib.ReqInf, resp interface{}, _ tc.Alerts, _ error) {
+ coordinates, _, err := TOSession.GetCoordinateByNameWithHdr(name, nil)
+ assert.RequireNoError(t, err, "Error getting Coordinate: %v", err, coordinates)
Review Comment:
this has 2 arguments but only 1 formatting parameter
##########
traffic_ops/testing/api/v4/coordinates_test.go:
##########
@@ -16,462 +16,348 @@ package v4
*/
import (
+ "encoding/json"
"net/http"
"net/url"
- "reflect"
"sort"
"strconv"
"testing"
"time"
"github.com/apache/trafficcontrol/lib/go-rfc"
- tc "github.com/apache/trafficcontrol/lib/go-tc"
+ "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"
)
func TestCoordinates(t *testing.T) {
WithObjs(t, []TCObj{Parameters, Coordinates}, func() {
- GetTestCoordinatesIMS(t)
- GetTestCoordinates(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)
- SortTestCoordinates(t)
- SortTestCoordinatesDesc(t)
- UpdateTestCoordinates(t)
- UpdateTestCoordinatesWithHeaders(t, header)
- header = make(map[string][]string)
- etag := rfc.ETag(currentTime)
- header.Set(rfc.IfMatch, etag)
- UpdateTestCoordinatesWithHeaders(t, header)
- GetTestCoordinatesIMSAfterChange(t, header)
- GetTestCoordinatesByInvalidId(t)
- GetTestCoordiantesByInvalidName(t)
- GetTestPaginationSupportCoordinates(t)
- CreateTestCoordinatesWithInvalidName(t)
- CreateTestCoordinatesWithInvalidLatitude(t)
- CreateTestCoordinatesWithInvalidLogitude(t)
- UpdateTestCoordinatesByInvalidId(t)
- DeleteTestCoordinatesByInvalidId(t)
- })
-}
-
-func UpdateTestCoordinatesWithHeaders(t *testing.T, header http.Header) {
- if len(testData.Coordinates) < 1 {
- t.Error("Need at least one Coordinate to test updating a Coordinate with an HTTP header")
- return
- }
- firstCoord := testData.Coordinates[0]
-
- opts := client.NewRequestOptions()
- opts.Header = header
- opts.QueryParameters.Set("name", firstCoord.Name)
-
- resp, _, err := TOSession.GetCoordinates(opts)
- if err != nil {
- t.Errorf("cannot get Coordinate '%s' from Traffic Ops: %v - alerts: %+v", firstCoord.Name, err, resp.Alerts)
- }
- if len(resp.Response) > 0 {
- coord := resp.Response[0]
- expectedLat := 12.34
- coord.Latitude = expectedLat
- opts.QueryParameters.Del("name")
- _, reqInf, err := TOSession.UpdateCoordinate(coord.ID, coord, opts)
- if err == nil {
- t.Errorf("Expected error about precondition failed, but got none")
+ 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 request": {
+ ClientSession: TOSession,
+ Expectations: utils.CkRequest(utils.NoError(), utils.HasStatus(http.StatusOK), validateCoordinateSort()),
+ },
+ "OK when VALID NAME parameter": {
+ ClientSession: TOSession,
+ RequestOpts: client.RequestOptions{QueryParameters: url.Values{"name": {"coordinate1"}}},
+ Expectations: utils.CkRequest(utils.NoError(), utils.HasStatus(http.StatusOK), utils.ResponseHasLength(1),
+ validateCoordinateFields(map[string]interface{}{"Name": "coordinate1"})),
+ },
+ "EMPTY RESPONSE when INVALID ID parameter": {
+ ClientSession: TOSession,
+ RequestOpts: client.RequestOptions{QueryParameters: url.Values{"id": {"10000"}}},
+ Expectations: utils.CkRequest(utils.NoError(), utils.HasStatus(http.StatusOK), utils.ResponseHasLength(0)),
+ },
+ "EMPTY RESPONSE when INVALID NAME parameter": {
+ ClientSession: TOSession,
+ RequestOpts: client.RequestOptions{QueryParameters: url.Values{"name": {"abcd"}}},
+ Expectations: utils.CkRequest(utils.NoError(), utils.HasStatus(http.StatusOK), utils.ResponseHasLength(0)),
+ },
+ "VALID when SORTORDER param is DESC": {
+ ClientSession: TOSession,
+ RequestOpts: client.RequestOptions{QueryParameters: url.Values{"sortOrder": {"desc"}}},
+ Expectations: utils.CkRequest(utils.NoError(), utils.HasStatus(http.StatusOK), validateCoordinateDescSort()),
+ },
+ "FIRST RESULT when LIMIT=1": {
+ ClientSession: TOSession,
+ RequestOpts: client.RequestOptions{QueryParameters: url.Values{"orderby": {"id"}, "limit": {"1"}}},
+ Expectations: utils.CkRequest(utils.NoError(), utils.HasStatus(http.StatusOK), validateCoordinatePagination("limit")),
+ },
+ "SECOND RESULT when LIMIT=1 OFFSET=1": {
+ ClientSession: TOSession,
+ RequestOpts: client.RequestOptions{QueryParameters: url.Values{"orderby": {"id"}, "limit": {"1"}, "offset": {"1"}}},
+ Expectations: utils.CkRequest(utils.NoError(), utils.HasStatus(http.StatusOK), validateCoordinatePagination("offset")),
+ },
+ "SECOND RESULT when LIMIT=1 PAGE=2": {
+ ClientSession: TOSession,
+ RequestOpts: client.RequestOptions{QueryParameters: url.Values{"orderby": {"id"}, "limit": {"1"}, "page": {"2"}}},
+ Expectations: utils.CkRequest(utils.NoError(), utils.HasStatus(http.StatusOK), validateCoordinatePagination("page")),
+ },
+ "BAD REQUEST when INVALID LIMIT parameter": {
+ ClientSession: TOSession,
+ RequestOpts: client.RequestOptions{QueryParameters: url.Values{"limit": {"-2"}}},
+ Expectations: utils.CkRequest(utils.HasError(), utils.HasStatus(http.StatusBadRequest)),
+ },
+ "BAD REQUEST when INVALID OFFSET parameter": {
+ ClientSession: TOSession,
+ RequestOpts: client.RequestOptions{QueryParameters: url.Values{"limit": {"1"}, "offset": {"0"}}},
+ Expectations: utils.CkRequest(utils.HasError(), utils.HasStatus(http.StatusBadRequest)),
+ },
+ "BAD REQUEST when INVALID PAGE parameter": {
+ ClientSession: TOSession,
+ RequestOpts: client.RequestOptions{QueryParameters: url.Values{"limit": {"1"}, "page": {"0"}}},
+ Expectations: utils.CkRequest(utils.HasError(), utils.HasStatus(http.StatusBadRequest)),
+ },
+ },
+ "POST": {
+ "BAD REQUEST when INVALID NAME": {
+ ClientSession: TOSession,
+ RequestBody: map[string]interface{}{
+ "latitude": 1.1,
+ "longitude": 2.2,
+ "name": "",
+ },
+ Expectations: utils.CkRequest(utils.HasError(), utils.HasStatus(http.StatusBadRequest)),
+ },
+ "BAD REQUEST when INVALID LATITUDE": {
+ ClientSession: TOSession,
+ RequestBody: map[string]interface{}{
+ "latitude": 20000,
+ "longitude": 2.2,
+ "name": "testlatitude",
+ },
+ Expectations: utils.CkRequest(utils.HasError(), utils.HasStatus(http.StatusBadRequest)),
+ },
+ "BAD REQUEST when INVALID LONGITUDE": {
+ ClientSession: TOSession,
+ RequestBody: map[string]interface{}{
+ "latitude": 1.1,
+ "longitude": 20000,
+ "name": "testlongitude",
+ },
+ Expectations: utils.CkRequest(utils.HasError(), utils.HasStatus(http.StatusBadRequest)),
+ },
+ },
+ "PUT": {
+ "OK when VALID request": {
+ EndpointId: GetCoordinateID(t, "coordinate2"),
+ ClientSession: TOSession,
+ RequestBody: map[string]interface{}{
+ "latitude": 7.7,
+ "longitude": 8.8,
+ "name": "coordinate2",
+ },
+ Expectations: utils.CkRequest(utils.NoError(), utils.HasStatus(http.StatusOK),
+ validateCoordinateUpdateCreateFields("coordinate2", map[string]interface{}{"Latitude": 7.7, "Longitude": 8.8})),
+ },
+ "NOT FOUND when INVALID ID parameter": {
+ EndpointId: func() int { return 111111 },
+ RequestBody: map[string]interface{}{
+ "latitude": 1.1,
+ "longitude": 2.2,
+ "name": "coordinate1",
+ },
+ ClientSession: TOSession,
+ Expectations: utils.CkRequest(utils.HasError(), utils.HasStatus(http.StatusNotFound)),
+ },
+ "PRECONDITION FAILED when updating with IMS & IUS Headers": {
+ EndpointId: GetCoordinateID(t, "coordinate1"),
+ ClientSession: TOSession,
+ RequestOpts: client.RequestOptions{Header: http.Header{rfc.IfUnmodifiedSince: {currentTimeRFC}}},
+ RequestBody: map[string]interface{}{
+ "latitude": 1.1,
+ "longitude": 2.2,
+ "name": "coordinate1",
+ },
+ Expectations: utils.CkRequest(utils.HasError(), utils.HasStatus(http.StatusPreconditionFailed)),
+ },
+ "PRECONDITION FAILED when updating with IFMATCH ETAG Header": {
+ EndpointId: GetCoordinateID(t, "coordinate1"),
+ ClientSession: TOSession,
+ RequestBody: map[string]interface{}{
+ "latitude": 1.1,
+ "longitude": 2.2,
+ "name": "coordinate1",
+ },
+ RequestOpts: client.RequestOptions{Header: http.Header{rfc.IfMatch: {rfc.ETag(currentTime)}}},
+ Expectations: utils.CkRequest(utils.HasError(), utils.HasStatus(http.StatusPreconditionFailed)),
+ },
+ },
+ "DELETE": {
+ "NOT FOUND when INVALID ID parameter": {
+ EndpointId: func() int { return 12345 },
+ ClientSession: TOSession,
+ Expectations: utils.CkRequest(utils.HasError(), utils.HasStatus(http.StatusNotFound)),
+ },
+ },
+ "GET AFTER CHANGES": {
+ "OK when CHANGES made": {
+ ClientSession: TOSession,
+ RequestOpts: client.RequestOptions{Header: http.Header{rfc.IfModifiedSince: {currentTimeRFC}}},
+ Expectations: utils.CkRequest(utils.NoError(), utils.HasStatus(http.StatusOK)),
+ },
+ },
}
- if reqInf.StatusCode != http.StatusPreconditionFailed {
- t.Errorf("Expected status code 412, got %v", reqInf.StatusCode)
- }
- } else {
- t.Errorf("No Coordinates available to update")
- }
-}
-func GetTestCoordinatesIMSAfterChange(t *testing.T, header http.Header) {
- opts := client.NewRequestOptions()
- opts.Header = header
- for _, coord := range testData.Coordinates {
- opts.QueryParameters.Set("name", coord.Name)
- resp, reqInf, err := TOSession.GetCoordinates(opts)
- if err != nil {
- t.Errorf("could not get Coordinate '%s' from Traffic Ops: %v - alerts: %+v", coord.Name, err, resp.Alerts)
- return
- }
- if reqInf.StatusCode != http.StatusOK {
- t.Errorf("Expected 304 status code, got %v", reqInf.StatusCode)
- return
+ for method, testCases := range methodTests {
+ t.Run(method, func(t *testing.T) {
+ for name, testCase := range testCases {
+ coordinate := tc.Coordinate{}
+
+ if testCase.RequestBody != nil {
+ dat, err := json.Marshal(testCase.RequestBody)
+ assert.NoError(t, err, "Error occurred when marshalling request body: %v", err)
+ err = json.Unmarshal(dat, &coordinate)
+ assert.NoError(t, err, "Error occurred when unmarshalling request body: %v", err)
+ }
+
+ switch method {
+ case "GET", "GET AFTER CHANGES":
+ t.Run(name, func(t *testing.T) {
+ resp, reqInf, err := testCase.ClientSession.GetCoordinates(testCase.RequestOpts)
+ for _, check := range testCase.Expectations {
+ check(t, reqInf, resp.Response, resp.Alerts, err)
+ }
+ })
+ case "POST":
+ t.Run(name, func(t *testing.T) {
+ alerts, reqInf, err := testCase.ClientSession.CreateCoordinate(coordinate, testCase.RequestOpts)
+ for _, check := range testCase.Expectations {
+ check(t, reqInf, nil, alerts, err)
+ }
+ })
+ case "PUT":
+ t.Run(name, func(t *testing.T) {
+ alerts, reqInf, err := testCase.ClientSession.UpdateCoordinate(testCase.EndpointId(), coordinate, testCase.RequestOpts)
+ for _, check := range testCase.Expectations {
+ check(t, reqInf, nil, alerts, err)
+ }
+ })
+ case "DELETE":
+ t.Run(name, func(t *testing.T) {
+ alerts, reqInf, err := testCase.ClientSession.DeleteCoordinate(testCase.EndpointId(), testCase.RequestOpts)
+ for _, check := range testCase.Expectations {
+ check(t, reqInf, nil, alerts, err)
+ }
+ })
+ }
+ }
+ })
}
- }
-
- currentTime := time.Now().UTC()
- currentTime = currentTime.Add(1 * time.Second)
- timeStr := currentTime.Format(time.RFC1123)
-
- opts.Header.Set(rfc.IfModifiedSince, timeStr)
-
- for _, coord := range testData.Coordinates {
- opts.QueryParameters.Set("name", coord.Name)
- resp, reqInf, err := TOSession.GetCoordinates(opts)
- if err != nil {
- t.Fatalf("could not get Coordinate '%s' from Traffic Ops: %v - alerts: %+v", coord.Name, err, resp.Alerts)
- }
- if reqInf.StatusCode != http.StatusNotModified {
- t.Fatalf("Expected 304 status code, got %v", reqInf.StatusCode)
- }
- }
-}
-
-func GetTestCoordinatesIMS(t *testing.T) {
- futureTime := time.Now().AddDate(0, 0, 1)
- time := futureTime.Format(time.RFC1123)
-
- opts := client.NewRequestOptions()
- opts.Header.Set(rfc.IfModifiedSince, time)
- for _, coord := range testData.Coordinates {
- opts.QueryParameters.Set("name", coord.Name)
- resp, reqInf, err := TOSession.GetCoordinates(opts)
- if err != nil {
- t.Errorf("Unexpected error getting Coordinate '%s': %v - alerts: %+v", coord.Name, err, resp.Alerts)
- }
- if reqInf.StatusCode != http.StatusNotModified {
- t.Errorf("Expected 304 status code, got %v", reqInf.StatusCode)
- }
- }
-}
-
-func CreateTestCoordinates(t *testing.T) {
- for _, coord := range testData.Coordinates {
- resp, _, err := TOSession.CreateCoordinate(coord, client.RequestOptions{})
- if err != nil {
- t.Errorf("could not create coordinate: %v - alerts: %+v", err, resp.Alerts)
- }
- }
-}
-
-func GetTestCoordinates(t *testing.T) {
- opts := client.NewRequestOptions()
- for _, coord := range testData.Coordinates {
- opts.QueryParameters.Set("name", coord.Name)
- resp, _, err := TOSession.GetCoordinates(opts)
- if err != nil {
- t.Errorf("cannot get Coordinate '%s' from Traffic Ops: %v - alerts: %v", coord.Name, err, resp.Alerts)
- }
- }
-}
-
-func SortTestCoordinates(t *testing.T) {
- var sortedList []string
- resp, _, err := TOSession.GetCoordinates(client.RequestOptions{})
- if err != nil {
- t.Fatalf("Unexpected error getting Coordinates from Traffic Ops: %v - alerts: %+v", err, resp.Alerts)
- }
- for _, coord := range resp.Response {
- sortedList = append(sortedList, coord.Name)
- }
-
- res := sort.SliceIsSorted(sortedList, func(p, q int) bool {
- return sortedList[p] < sortedList[q]
})
- if res != true {
- t.Errorf("list is not sorted by their names: %v", sortedList)
- }
-}
-
-func SortTestCoordinatesDesc(t *testing.T) {
-
- resp, _, err := TOSession.GetCoordinates(client.RequestOptions{})
- if err != nil {
- t.Errorf("Unexpected error getting Coordinates with default sort order: %v - alerts: %+v", err, resp.Alerts)
- }
- respAsc := resp.Response
- if len(respAsc) < 1 {
- t.Fatal("Need at least one Coordinate in Traffic Ops to test sort ordering")
- }
-
- opts := client.NewRequestOptions()
- opts.QueryParameters.Set("sortOrder", "desc")
- resp, _, err = TOSession.GetCoordinates(opts)
- if err != nil {
- t.Errorf("Unexpected error getting Coordinates with explicit descending sort order: %v - alerts: %+v", err, resp.Alerts)
- }
- respDesc := resp.Response
- if len(respDesc) < 1 {
- t.Fatal("Need at least one Coordinate in Traffic Ops to test sort ordering")
- }
-
- if len(respAsc) != len(respDesc) {
- t.Fatalf("Traffic Ops returned %d Coordinates using default sort order, but returned %d Coordinates using explicit descending sort order", len(respAsc), len(respDesc))
- }
-
- // reverse the descending-sorted response and compare it to the ascending-sorted one
- for start, end := 0, len(respDesc)-1; start < end; start, end = start+1, end-1 {
- respDesc[start], respDesc[end] = respDesc[end], respDesc[start]
- }
- if respDesc[0].Name != respAsc[0].Name {
- t.Errorf("Coordinates responses are not equal after reversal: %s - %s", respDesc[0].Name, respAsc[0].Name)
- }
}
-func UpdateTestCoordinates(t *testing.T) {
- if len(testData.Coordinates) < 1 {
- t.Fatal("Need at least one Coordinate to test updating Coordinates")
- }
- firstCoord := testData.Coordinates[0]
-
- opts := client.NewRequestOptions()
- opts.QueryParameters.Set("name", firstCoord.Name)
- resp, _, err := TOSession.GetCoordinates(opts)
- if err != nil {
- t.Errorf("cannot get Coordinate '%s' by name: %v - alerts: %+v", firstCoord.Name, err, resp.Alerts)
- }
- if len(resp.Response) != 1 {
- t.Fatalf("Expected exactly one Coordinate to exist with name '%s', found: %d", firstCoord.Name, len(resp.Response))
- }
- coord := resp.Response[0]
- expectedLat := 12.34
- coord.Latitude = expectedLat
-
- alert, _, err := TOSession.UpdateCoordinate(coord.ID, coord, client.RequestOptions{})
- if err != nil {
- t.Errorf("cannot update Coordinate: %v - alerts: %+v", err, alert.Alerts)
- }
-
- // Retrieve the Coordinate to check Coordinate name got updated
- opts.QueryParameters.Del("name")
- opts.QueryParameters.Set("id", strconv.Itoa(coord.ID))
- resp, _, err = TOSession.GetCoordinates(opts)
- if err != nil {
- t.Errorf("cannot get Coordinate '%s' by id: %v - alerts: %+v", firstCoord.Name, err, resp.Alerts)
- }
- if len(resp.Response) > 0 {
- coord = resp.Response[0]
- if coord.Latitude != expectedLat {
- t.Errorf("results do not match actual: %s, expected: %f", coord.Name, expectedLat)
- }
- } else {
- t.Errorf("Can't retrieve coordinates to check the updated value")
- }
-}
-
-func DeleteTestCoordinates(t *testing.T) {
- opts := client.NewRequestOptions()
- for _, coord := range testData.Coordinates {
- // Retrieve the Coordinate by name so we can get the id for the Update
- opts.QueryParameters.Set("name", coord.Name)
- resp, _, err := TOSession.GetCoordinates(opts)
- if err != nil {
- t.Errorf("cannot get Coordinate '%s' from Traffic Ops: %v - alerts: %+v", coord.Name, err, resp.Alerts)
- }
- if len(resp.Response) > 0 {
- respCoord := resp.Response[0]
- delResp, _, err := TOSession.DeleteCoordinate(respCoord.ID, client.RequestOptions{})
- if err != nil {
- t.Errorf("cannot delete Coordinate '%s' (#%d): %v - alerts: %+v", respCoord.Name, respCoord.ID, err, delResp.Alerts)
+func validateCoordinateFields(expectedResp map[string]interface{}) utils.CkReqFunc {
+ return func(t *testing.T, _ toclientlib.ReqInf, resp interface{}, _ tc.Alerts, _ error) {
+ assert.RequireNotNil(t, resp, "Expected Coordinate response to not be nil.")
+ coordinateResp := resp.([]tc.Coordinate)
+ for field, expected := range expectedResp {
+ for _, coordinate := range coordinateResp {
+ switch field {
+ case "Name":
+ assert.Equal(t, expected, coordinate.Name, "Expected Name to be %v, but got %s", expected, coordinate.Name)
+ case "Latitude":
+ assert.Equal(t, expected, coordinate.Latitude, "Expected Latitude to be %v, but got %s", expected, coordinate.Latitude)
+ case "Longitude":
+ assert.Equal(t, expected, coordinate.Longitude, "Expected Longitude to be %v, but got %s", expected, coordinate.Longitude)
Review Comment:
same as above with %s as %f
--
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