From 021bb32a2287a183547f0b710f23252eb9ea3ae1 Mon Sep 17 00:00:00 2001 From: James Jeffrey Date: Thu, 6 Jul 2017 10:35:44 -0700 Subject: [PATCH] Refactor tests to use testRoute case struct. Use one method for tests. Fix logic for when to skipzero --- api/server/routes_create_update.go | 6 +- api/server/routes_test.go | 172 ++++++++++++++++------------- 2 files changed, 98 insertions(+), 80 deletions(-) diff --git a/api/server/routes_create_update.go b/api/server/routes_create_update.go index 9401c5652..08ea6641e 100644 --- a/api/server/routes_create_update.go +++ b/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 } diff --git a/api/server/routes_test.go b/api/server/routes_test.go index 817b1ce45..ce78178f1 100644 --- a/api/server/routes_test.go +++ b/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 { - mock models.Datastore - logDB models.FnLog - path string - body string - expectedCode int - expectedError error - }{ + for i, test := range []routeTestCase{ // errors - {datastore.NewMock(), logs.NewMock(), "/v1/apps/a/routes", ``, http.StatusBadRequest, models.ErrInvalidJSON}, - {datastore.NewMock(), logs.NewMock(), "/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(), "/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(), "/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(), "/v1/apps/$/routes", `{ "route": { "image": "funcy/hello", "path": "/myroute" } }`, http.StatusInternalServerError, models.ErrAppsValidationInvalidName}, + {datastore.NewMock(), logs.NewMock(), http.MethodPost, "/v1/apps/a/routes", ``, http.StatusBadRequest, models.ErrInvalidJSON}, + {datastore.NewMock(), logs.NewMock(), http.MethodPost, "/v1/apps/a/routes", `{ }`, http.StatusBadRequest, models.ErrRoutesMissingNew}, + {datastore.NewMock(), logs.NewMock(), http.MethodPost, "/v1/apps/a/routes", `{ "path": "/myroute" }`, http.StatusBadRequest, models.ErrRoutesMissingNew}, + {datastore.NewMock(), logs.NewMock(), http.MethodPost, "/v1/apps/a/routes", `{ "route": { } }`, http.StatusBadRequest, models.ErrRoutesValidationMissingPath}, + {datastore.NewMock(), logs.NewMock(), http.MethodPost, "/v1/apps/a/routes", `{ "route": { "path": "/myroute" } }`, http.StatusBadRequest, models.ErrRoutesValidationMissingImage}, + {datastore.NewMock(), logs.NewMock(), http.MethodPost, "/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", "path": "myroute" } }`, http.StatusBadRequest, models.ErrRoutesValidationInvalidPath}, + {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) - srv := testServer(test.mock, &mqs.Mock{}, test.logDB, rnr) + test.run(t, i, buf) + } +} - body := bytes.NewBuffer([]byte(test.body)) - _, rec := routerRequest(t, srv.Router, "POST", test.path, body) +func TestRoutePut(t *testing.T) { + buf := setLogBuffer() - 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) - } + for i, test := range []routeTestCase{ + // errors + {datastore.NewMock(), logs.NewMock(), http.MethodPut, "/v1/apps/a/routes/myroute", `{ }`, http.StatusBadRequest, models.ErrRoutesMissingNew}, + {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 { - 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() + // success + {datastore.NewMock(), logs.NewMock(), http.MethodPut, "/v1/apps/a/routes/myroute", `{ "route": { "image": "funcy/hello", "path": "/myroute" } }`, http.StatusOK, nil}, + {datastore.NewMock(), logs.NewMock(), http.MethodPut, "/v1/apps/a/routes/myroute", `{ "route": { "image": "funcy/hello" } }`, http.StatusOK, nil}, + } { + test.run(t, i, buf) + test.ds = datastore.NewMockInit(nil, + []*models.Route{ + { + AppName: "a", + Path: "/myroute", + }, + }, nil, nil, + ) + 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 { - ds models.Datastore - logDB models.FnLog - path string - body string - expectedCode int - expectedError error - }{ + for i, test := range []routeTestCase{ // errors - {datastore.NewMock(), logs.NewMock(), "/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(), "/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", ``, http.StatusBadRequest, models.ErrInvalidJSON}, + {datastore.NewMock(), logs.NewMock(), http.MethodPatch, "/v1/apps/a/routes/myroute/do", `{}`, http.StatusBadRequest, models.ErrRoutesMissingNew}, + {datastore.NewMock(), logs.NewMock(), http.MethodPatch, "/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": { "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) - - 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() + test.run(t, i, buf) } }