rimashah25 commented on code in PR #7401:
URL: https://github.com/apache/trafficcontrol/pull/7401#discussion_r1134859452
##########
traffic_ops/traffic_ops_golang/crconfig/snapshot_test.go:
##########
@@ -37,56 +38,161 @@ func ExpectedGetSnapshot(crc *tc.CRConfig) ([]byte, error)
{
return json.Marshal(crc)
}
-func ExpectedGetMontioringSnapshot(crc *tc.CRConfig, tx *sql.Tx) ([]byte,
error) {
+func ExpectedGetMonitoringSnapshot(crc *tc.CRConfig, tx *sql.Tx) ([]byte,
error) {
tm, _ := monitoring.GetMonitoringJSON(tx, *crc.Stats.CDNName)
return json.Marshal(tm)
}
-func MockGetSnapshot(mock sqlmock.Sqlmock, expected []byte, cdn string) {
- rows := sqlmock.NewRows([]string{"snapshot"})
- rows = rows.AddRow(expected)
- rows = rows.AddRow(expected)
- mock.ExpectQuery("SELECT").WithArgs(cdn).WillReturnRows(rows)
+func MockGetSnapshotTestCases(mock sqlmock.Sqlmock, expected []byte, cdn
string) {
+ if expected != nil {
+ rows := sqlmock.NewRows([]string{"snapshot"})
+ rows = rows.AddRow(expected)
+ mock.ExpectQuery("SELECT").WithArgs(cdn).WillReturnRows(rows)
+ } else if expected == nil {
+ rows := sqlmock.NewRows([]string{"snapshot"})
+ mock.ExpectQuery("SELECT").WithArgs(cdn).WillReturnRows(rows)
+ }
}
func TestGetSnapshot(t *testing.T) {
- db, mock, err := sqlmock.New()
- if err != nil {
- t.Fatalf("an error '%s' was not expected when opening a stub
database connection", err)
- }
- defer db.Close()
+ testCases := []string{"success", "emptyRows", "badCdnName"}
- cdn := "mycdn"
+ for _, v := range testCases {
+ db, mock, err := sqlmock.New()
+ if err != nil {
+ t.Fatalf("an error '%s' was not expected when opening a
stub database connection", err)
Review Comment:
`%v` is preferred over `%s` to showcase error.
##########
traffic_ops/traffic_ops_golang/crconfig/snapshot_test.go:
##########
@@ -37,56 +38,161 @@ func ExpectedGetSnapshot(crc *tc.CRConfig) ([]byte, error)
{
return json.Marshal(crc)
}
-func ExpectedGetMontioringSnapshot(crc *tc.CRConfig, tx *sql.Tx) ([]byte,
error) {
+func ExpectedGetMonitoringSnapshot(crc *tc.CRConfig, tx *sql.Tx) ([]byte,
error) {
tm, _ := monitoring.GetMonitoringJSON(tx, *crc.Stats.CDNName)
return json.Marshal(tm)
}
-func MockGetSnapshot(mock sqlmock.Sqlmock, expected []byte, cdn string) {
- rows := sqlmock.NewRows([]string{"snapshot"})
- rows = rows.AddRow(expected)
- rows = rows.AddRow(expected)
- mock.ExpectQuery("SELECT").WithArgs(cdn).WillReturnRows(rows)
+func MockGetSnapshotTestCases(mock sqlmock.Sqlmock, expected []byte, cdn
string) {
+ if expected != nil {
+ rows := sqlmock.NewRows([]string{"snapshot"})
+ rows = rows.AddRow(expected)
+ mock.ExpectQuery("SELECT").WithArgs(cdn).WillReturnRows(rows)
+ } else if expected == nil {
+ rows := sqlmock.NewRows([]string{"snapshot"})
+ mock.ExpectQuery("SELECT").WithArgs(cdn).WillReturnRows(rows)
+ }
}
func TestGetSnapshot(t *testing.T) {
- db, mock, err := sqlmock.New()
- if err != nil {
- t.Fatalf("an error '%s' was not expected when opening a stub
database connection", err)
- }
- defer db.Close()
+ testCases := []string{"success", "emptyRows", "badCdnName"}
- cdn := "mycdn"
+ for _, v := range testCases {
+ db, mock, err := sqlmock.New()
+ if err != nil {
+ t.Fatalf("an error '%s' was not expected when opening a
stub database connection", err)
+ }
+ defer db.Close()
- crc := &tc.CRConfig{}
- crc.Stats.CDNName = &cdn
+ cdn := "mycdn"
- mock.ExpectBegin()
- expected, err := ExpectedGetSnapshot(crc)
- if err != nil {
- t.Fatalf("GetSnapshot creating expected err expected: nil,
actual: %v", err)
- }
- MockGetSnapshot(mock, expected, cdn)
- mock.ExpectCommit()
+ crc := &tc.CRConfig{}
+ crc.Stats.CDNName = &cdn
- dbCtx, cancelTx := context.WithTimeout(context.TODO(), 10*time.Second)
- defer cancelTx()
- tx, err := db.BeginTx(dbCtx, nil)
- if err != nil {
- t.Fatalf("creating transaction: %v", err)
- }
- defer tx.Commit()
+ mock.ExpectBegin()
- actual, exists, err := GetSnapshot(tx, cdn)
- if err != nil {
- t.Fatalf("GetSnapshot err expected: nil, actual: %v", err)
- }
- if !exists {
- t.Fatalf("GetSnapshot exists expected: true, actual: false")
+ expected, err := ExpectedGetSnapshot(crc)
+
+ if err != nil {
+ t.Fatalf("GetSnapshot creating expected err expected:
nil, actual: %v", err)
+ }
+ if v == "success" {
+ MockGetSnapshotTestCases(mock, expected, cdn)
+ } else if v == "emptyRows" {
+ MockGetSnapshotTestCases(mock, nil, cdn)
+ } else if v == "badCdnName" {
+ MockGetSnapshotTestCases(mock, expected, "bad")
+ } else {
+ t.Fatalf("GetSnapshot testCase %v not found", v)
+ }
+ mock.ExpectCommit()
+
+ dbCtx, cancelTx := context.WithTimeout(context.TODO(),
10*time.Second)
+ defer cancelTx()
+ tx, err := db.BeginTx(dbCtx, nil)
+
+ actual, exists, err := GetSnapshot(tx, cdn)
+
+ if v == "success" {
+ if err != nil {
+ t.Fatalf("GetSnapshot err expected: nil,
actual: %v", err)
+ }
+ if !exists {
+ t.Fatalf("GetSnapshot exists expected: true,
actual: false")
+ }
+ if !reflect.DeepEqual(string(expected), actual) {
+ t.Errorf("GetSnapshot expected: %+v, actual:
%+v", string(expected), actual)
+ }
+ } else if v == "emptyRows" {
+ if err != nil {
+ t.Fatalf("GetSnapshot err expected: nil,
actual: %v", err)
+ }
+ if !reflect.DeepEqual("", actual) {
+ t.Errorf("GetSnapshot expected an empty string,
actual: %+v", actual)
+ }
+ } else if v == "badCdnName" {
+ if err == nil && strings.Contains("does not match
actual [string - mycdn]", err.Error()) {
+ t.Errorf("Expected a mismatched error when
supplying a bad CDN name in GetSnapshot")
+ }
+ if !reflect.DeepEqual("", actual) {
+ t.Errorf("GetSnapshot expected an empty string,
actual: %+v", actual)
+ }
+ } else {
+ t.Fatalf("Test case %v not correctly accounted for", v)
+ }
+
+ defer tx.Commit()
}
+}
+
+func TestGetSnapshotMonitoring(t *testing.T) {
+ testCases := []string{"success", "emptyRows", "badCdnName"}
+
+ for _, v := range testCases {
+ db, mock, err := sqlmock.New()
+ if err != nil {
+ t.Fatalf("an error '%s' was not expected when opening a
stub database connection", err)
Review Comment:
`%v` is preferred over `%s` to showcase error.
##########
traffic_ops/traffic_ops_golang/crconfig/config_test.go:
##########
@@ -21,6 +21,8 @@ package crconfig
import (
"context"
+ "errors"
+ "github.com/apache/trafficcontrol/lib/go-util"
Review Comment:
Re-arrange imports.. github ones come in the end..
Eg:

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