Refactor tests to use testRoute case struct. Use one method for tests. Fix logic for when to skipzero

api/server/routes_create_update.go

@@ -28,7 +28,7 @@ func (s *Server) handleRouteCreateOrUpdate(c *gin.Context) {
 
 	var wroute models.RouteWrapper
 
-	err := bindAndValidate(c, log, method, &wroute)
+	err := s.bindAndValidate(ctx, c, log, method, &wroute)
 	if err != nil {
 		c.JSON(http.StatusBadRequest, simpleError(err))
 		return
@@ -92,7 +92,7 @@ func (s *Server) createApp(ctx context.Context, c *gin.Context, log logrus.Field
 	return nil
 }
 
-func bindAndValidate(c *gin.Context, log logrus.FieldLogger, method string, wroute *models.RouteWrapper) error {
+func (s *Server) bindAndValidate(ctx context.Context, c *gin.Context, log logrus.FieldLogger, method string, wroute *models.RouteWrapper) error {
 	err := c.BindJSON(wroute)
 	if err != nil {
 		log.WithError(err).Debug(models.ErrInvalidJSON)
@@ -118,7 +118,7 @@ func bindAndValidate(c *gin.Context, log logrus.FieldLogger, method string, wrou
 
 	wroute.Route.SetDefaults()
 
-	if err = wroute.Validate(method != http.MethodPost); err != nil {
+	if err = wroute.Validate(method == http.MethodPatch); err != nil {
 		log.WithError(err).Debug(models.ErrRoutesCreate)
 		return err
 	}

api/server/routes_test.go

@@ -12,55 +12,103 @@ import (
 	"gitlab-odx.oracle.com/odx/functions/api/mqs"
 )
 
+type routeTestCase struct {
+	ds            models.Datastore
+	logDB         models.FnLog
+	method        string
+	path          string
+	body          string
+	expectedCode  int
+	expectedError error
+}
+
+func (test *routeTestCase) run(t *testing.T, i int, buf *bytes.Buffer) {
+	rnr, cancel := testRunner(t)
+	srv := testServer(test.ds, &mqs.Mock{}, test.logDB, rnr)
+
+	body := bytes.NewBuffer([]byte(test.body))
+	_, rec := routerRequest(t, srv.Router, test.method, test.path, body)
+
+	if rec.Code != test.expectedCode {
+		t.Log(buf.String())
+		t.Errorf("Test %d: Expected status code to be %d but was %d",
+			i, test.expectedCode, rec.Code)
+	}
+
+	if test.expectedError != nil {
+		resp := getErrorResponse(t, rec)
+		if resp.Error == nil {
+			t.Log(buf.String())
+			t.Errorf("Test %d: Expected error message to have `%s`, but it was nil",
+				i, test.expectedError)
+		} else if !strings.Contains(resp.Error.Message, test.expectedError.Error()) {
+			t.Log(buf.String())
+			t.Errorf("Test %d: Expected error message to have `%s`, but it was `%s`",
+				i, test.expectedError, resp.Error.Message)
+		}
+	}
+	cancel()
+	buf.Reset()
+}
+
 func TestRouteCreate(t *testing.T) {
 	buf := setLogBuffer()
 
-	for i, test := range []struct {
+	for i, test := range []routeTestCase{
-		mock          models.Datastore
-		logDB         models.FnLog
-		path          string
-		body          string
-		expectedCode  int
-		expectedError error
-	}{
 		// errors
-		{datastore.NewMock(), logs.NewMock(), "/v1/apps/a/routes", ``, http.StatusBadRequest, models.ErrInvalidJSON},
+		{datastore.NewMock(), logs.NewMock(), http.MethodPost, "/v1/apps/a/routes", ``, http.StatusBadRequest, models.ErrInvalidJSON},
-		{datastore.NewMock(), logs.NewMock(), "/v1/apps/a/routes", `{ }`, http.StatusBadRequest, models.ErrRoutesMissingNew},
+		{datastore.NewMock(), logs.NewMock(), http.MethodPost, "/v1/apps/a/routes", `{ }`, http.StatusBadRequest, models.ErrRoutesMissingNew},
-		{datastore.NewMock(), logs.NewMock(), "/v1/apps/a/routes", `{ "path": "/myroute" }`, http.StatusBadRequest, models.ErrRoutesMissingNew},
+		{datastore.NewMock(), logs.NewMock(), http.MethodPost, "/v1/apps/a/routes", `{ "path": "/myroute" }`, http.StatusBadRequest, models.ErrRoutesMissingNew},
-		{datastore.NewMock(), logs.NewMock(), "/v1/apps/a/routes", `{ "route": { } }`, http.StatusBadRequest, models.ErrRoutesValidationMissingPath},
+		{datastore.NewMock(), logs.NewMock(), http.MethodPost, "/v1/apps/a/routes", `{ "route": { } }`, http.StatusBadRequest, models.ErrRoutesValidationMissingPath},
-		{datastore.NewMock(), logs.NewMock(), "/v1/apps/a/routes", `{ "route": { "path": "/myroute" } }`, http.StatusBadRequest, models.ErrRoutesValidationMissingImage},
+		{datastore.NewMock(), logs.NewMock(), http.MethodPost, "/v1/apps/a/routes", `{ "route": { "path": "/myroute" } }`, http.StatusBadRequest, models.ErrRoutesValidationMissingImage},
-		{datastore.NewMock(), logs.NewMock(), "/v1/apps/a/routes", `{ "route": { "image": "funcy/hello" } }`, http.StatusBadRequest, models.ErrRoutesValidationMissingPath},
+		{datastore.NewMock(), logs.NewMock(), http.MethodPost, "/v1/apps/a/routes", `{ "route": { "image": "funcy/hello" } }`, http.StatusBadRequest, models.ErrRoutesValidationMissingPath},
-		{datastore.NewMock(), logs.NewMock(), "/v1/apps/a/routes", `{ "route": { "image": "funcy/hello", "path": "myroute" } }`, http.StatusBadRequest, models.ErrRoutesValidationInvalidPath},
+		{datastore.NewMock(), logs.NewMock(), http.MethodPost, "/v1/apps/a/routes", `{ "route": { "image": "funcy/hello", "path": "myroute" } }`, http.StatusBadRequest, models.ErrRoutesValidationInvalidPath},
-		{datastore.NewMock(), logs.NewMock(), "/v1/apps/$/routes", `{ "route": { "image": "funcy/hello", "path": "/myroute" } }`, http.StatusInternalServerError, models.ErrAppsValidationInvalidName},
+		{datastore.NewMock(), logs.NewMock(), http.MethodPost, "/v1/apps/$/routes", `{ "route": { "image": "funcy/hello", "path": "/myroute" } }`, http.StatusInternalServerError, models.ErrAppsValidationInvalidName},
+		{datastore.NewMockInit(nil,
+			[]*models.Route{
+				{
+					AppName: "a",
+					Path:    "/myroute",
+				},
+			}, nil, nil,
+		), logs.NewMock(), http.MethodPost, "/v1/apps/a/routes", `{ "route": { "image": "funcy/hello", "path": "/myroute" } }`, http.StatusConflict, models.ErrRoutesAlreadyExists},
 
 		// success
-		{datastore.NewMock(), logs.NewMock(), "/v1/apps/a/routes", `{ "route": { "image": "funcy/hello", "path": "/myroute" } }`, http.StatusOK, nil},
+		{datastore.NewMock(), logs.NewMock(), http.MethodPost, "/v1/apps/a/routes", `{ "route": { "image": "funcy/hello", "path": "/myroute" } }`, http.StatusOK, nil},
 	} {
-		rnr, cancel := testRunner(t)
+		test.run(t, i, buf)
-		srv := testServer(test.mock, &mqs.Mock{}, test.logDB, rnr)
+	}
+}
 
-		body := bytes.NewBuffer([]byte(test.body))
+func TestRoutePut(t *testing.T) {
-		_, rec := routerRequest(t, srv.Router, "POST", test.path, body)
+	buf := setLogBuffer()
 
-		if rec.Code != test.expectedCode {
+	for i, test := range []routeTestCase{
-			t.Log(buf.String())
+		// errors
-			t.Errorf("Test %d: Expected status code to be %d but was %d",
+		{datastore.NewMock(), logs.NewMock(), http.MethodPut, "/v1/apps/a/routes/myroute", `{ }`, http.StatusBadRequest, models.ErrRoutesMissingNew},
-				i, test.expectedCode, rec.Code)
+		{datastore.NewMock(), logs.NewMock(), http.MethodPut, "/v1/apps/a/routes/myroute", `{ "path": "/myroute" }`, http.StatusBadRequest, models.ErrRoutesMissingNew},
-		}
+		{datastore.NewMock(), logs.NewMock(), http.MethodPut, "/v1/apps/a/routes/myroute", `{ "route": { } }`, http.StatusBadRequest, models.ErrRoutesValidationMissingImage},
+		{datastore.NewMock(), logs.NewMock(), http.MethodPut, "/v1/apps/a/routes/myroute", `{ "route": { "path": "/myroute" } }`, http.StatusBadRequest, models.ErrRoutesValidationMissingImage},
+		{datastore.NewMock(), logs.NewMock(), http.MethodPut, "/v1/apps/a/routes/myroute", `{ "route": { "image": "funcy/hello", "path": "myroute" } }`, http.StatusBadRequest, models.ErrRoutesPathImmutable},
+		{datastore.NewMock(), logs.NewMock(), http.MethodPut, "/v1/apps/a/routes/myroute", `{ "route": { "image": "funcy/hello", "path": "diffRoute" } }`, http.StatusBadRequest, models.ErrRoutesPathImmutable},
+		{datastore.NewMock(), logs.NewMock(), http.MethodPut, "/v1/apps/$/routes/myroute", `{ "route": { "image": "funcy/hello", "path": "/myroute" } }`, http.StatusInternalServerError, models.ErrAppsValidationInvalidName},
+		{datastore.NewMock(), logs.NewMock(), http.MethodPut, "/v1/apps/a/routes/myroute", `{ "route": { "type": "invalid-type" } }`, http.StatusBadRequest, models.ErrRoutesValidationInvalidType},
+		{datastore.NewMock(), logs.NewMock(), http.MethodPut, "/v1/apps/a/routes/myroute", `{ "route": { "format": "invalid-format" } }`, http.StatusBadRequest, models.ErrRoutesValidationInvalidFormat},
 
-		if test.expectedError != nil {
+		// success
-			resp := getErrorResponse(t, rec)
+		{datastore.NewMock(), logs.NewMock(), http.MethodPut, "/v1/apps/a/routes/myroute", `{ "route": { "image": "funcy/hello", "path": "/myroute" } }`, http.StatusOK, nil},
-			if resp.Error == nil {
+		{datastore.NewMock(), logs.NewMock(), http.MethodPut, "/v1/apps/a/routes/myroute", `{ "route": { "image": "funcy/hello" } }`, http.StatusOK, nil},
-				t.Log(buf.String())
+	} {
-				t.Errorf("Test %d: Expected error message to have `%s`, but it was nil",
+		test.run(t, i, buf)
-					i, test.expectedError)
+		test.ds = datastore.NewMockInit(nil,
-			} else if !strings.Contains(resp.Error.Message, test.expectedError.Error()) {
+			[]*models.Route{
-				t.Log(buf.String())
+				{
-				t.Errorf("Test %d: Expected error message to have `%s`, but it was `%s`",
+					AppName: "a",
-					i, test.expectedError, resp.Error.Message)
+					Path:    "/myroute",
-			}
+				},
-		}
+			}, nil, nil,
-		cancel()
+		)
+		test.run(t, i, buf)
 	}
 }
 
@@ -186,19 +234,12 @@ func TestRouteGet(t *testing.T) {
 func TestRouteUpdate(t *testing.T) {
 	buf := setLogBuffer()
 
-	for i, test := range []struct {
+	for i, test := range []routeTestCase{
-		ds            models.Datastore
-		logDB         models.FnLog
-		path          string
-		body          string
-		expectedCode  int
-		expectedError error
-	}{
 		// errors
-		{datastore.NewMock(), logs.NewMock(), "/v1/apps/a/routes/myroute/do", ``, http.StatusBadRequest, models.ErrInvalidJSON},
+		{datastore.NewMock(), logs.NewMock(), http.MethodPatch, "/v1/apps/a/routes/myroute/do", ``, http.StatusBadRequest, models.ErrInvalidJSON},
-		{datastore.NewMock(), logs.NewMock(), "/v1/apps/a/routes/myroute/do", `{}`, http.StatusBadRequest, models.ErrRoutesMissingNew},
+		{datastore.NewMock(), logs.NewMock(), http.MethodPatch, "/v1/apps/a/routes/myroute/do", `{}`, http.StatusBadRequest, models.ErrRoutesMissingNew},
-		{datastore.NewMock(), logs.NewMock(), "/v1/apps/a/routes/myroute/do", `{ "route": { "type": "invalid-type" } }`, http.StatusBadRequest, models.ErrRoutesValidationInvalidType},
+		{datastore.NewMock(), logs.NewMock(), http.MethodPatch, "/v1/apps/a/routes/myroute/do", `{ "route": { "type": "invalid-type" } }`, http.StatusBadRequest, models.ErrRoutesValidationInvalidType},
-		{datastore.NewMock(), logs.NewMock(), "/v1/apps/a/routes/myroute/do", `{ "route": { "format": "invalid-format" } }`, http.StatusBadRequest, models.ErrRoutesValidationInvalidFormat},
+		{datastore.NewMock(), logs.NewMock(), http.MethodPatch, "/v1/apps/a/routes/myroute/do", `{ "route": { "format": "invalid-format" } }`, http.StatusBadRequest, models.ErrRoutesValidationInvalidFormat},
 
 		// success
 		{datastore.NewMockInit(nil,
@@ -208,7 +249,7 @@ func TestRouteUpdate(t *testing.T) {
 					Path:    "/myroute/do",
 				},
 			}, nil, nil,
-		), logs.NewMock(), "/v1/apps/a/routes/myroute/do", `{ "route": { "image": "funcy/hello" } }`, http.StatusOK, nil},
+		), logs.NewMock(), http.MethodPatch, "/v1/apps/a/routes/myroute/do", `{ "route": { "image": "funcy/hello" } }`, http.StatusOK, nil},
 
 		// Addresses #381
 		{datastore.NewMockInit(nil,
@@ -218,31 +259,8 @@ func TestRouteUpdate(t *testing.T) {
 					Path:    "/myroute/do",
 				},
 			}, nil, nil,
-		), logs.NewMock(), "/v1/apps/a/routes/myroute/do", `{ "route": { "path": "/otherpath" } }`, http.StatusBadRequest, models.ErrRoutesPathImmutable},
+		), logs.NewMock(), http.MethodPatch, "/v1/apps/a/routes/myroute/do", `{ "route": { "path": "/otherpath" } }`, http.StatusBadRequest, models.ErrRoutesPathImmutable},
 	} {
-		rnr, cancel := testRunner(t)
+		test.run(t, i, buf)
-
-		srv := testServer(test.ds, &mqs.Mock{}, test.logDB, rnr)
-
-		body := bytes.NewBuffer([]byte(test.body))
-
-		_, rec := routerRequest(t, srv.Router, "PATCH", test.path, body)
-
-		if rec.Code != test.expectedCode {
-			t.Log(buf.String())
-			t.Errorf("Test %d: Expected status code to be %d but was %d: %s",
-				i, test.expectedCode, rec.Code, rec.Body.String())
-		}
-
-		if test.expectedError != nil {
-			resp := getErrorResponse(t, rec)
-
-			if !strings.Contains(resp.Error.Message, test.expectedError.Error()) {
-				t.Log(buf.String())
-				t.Errorf("Test %d: Expected error message to have `%s`",
-					i, test.expectedError.Error())
-			}
-		}
-		cancel()
 	}
 }