From 3528ce60ca466ff0c66eec9f8f20c510c3d90a7c Mon Sep 17 00:00:00 2001 From: Luke Kysow Date: Wed, 1 Nov 2017 07:43:55 -0700 Subject: [PATCH] Fix gometalint errors --- server/events_controller_test.go | 66 ++++++++++++++++---------------- server/server.go | 4 +- server/server_test.go | 48 +++++++++++------------ 3 files changed, 59 insertions(+), 59 deletions(-) diff --git a/server/events_controller_test.go b/server/events_controller_test.go index a02177eb0..40a25418a 100644 --- a/server/events_controller_test.go +++ b/server/events_controller_test.go @@ -22,14 +22,14 @@ import ( const secret = "secret" -var req *http.Request +var eventsReq *http.Request func TestPost_InvalidSecret(t *testing.T) { t.Log("when the payload can't be validated a 400 is returned") e, v, _, _, _ := setup(t) w := httptest.NewRecorder() - When(v.Validate(req, []byte(secret))).ThenReturn(nil, errors.New("err")) - e.Post(w, req) + When(v.Validate(eventsReq, []byte(secret))).ThenReturn(nil, errors.New("err")) + e.Post(w, eventsReq) responseContains(t, w, http.StatusBadRequest, "err") } @@ -37,54 +37,54 @@ func TestPost_UnsupportedEvent(t *testing.T) { t.Log("when the event type is unsupported we ignore it") e, v, _, _, _ := setup(t) w := httptest.NewRecorder() - When(v.Validate(req, nil)).ThenReturn([]byte(`{"not an event": ""}`), nil) - e.Post(w, req) + When(v.Validate(eventsReq, nil)).ThenReturn([]byte(`{"not an event": ""}`), nil) + e.Post(w, eventsReq) responseContains(t, w, http.StatusOK, "Ignoring unsupported event") } func TestPost_CommentNotCreated(t *testing.T) { t.Log("when the event is a comment but it's not a created event we ignore it") e, v, _, _, _ := setup(t) - req.Header.Set("X-Github-Event", "issue_comment") + eventsReq.Header.Set("X-Github-Event", "issue_comment") // comment action is deleted, not created event := `{"action": "deleted"}` - When(v.Validate(req, []byte(secret))).ThenReturn([]byte(event), nil) + When(v.Validate(eventsReq, []byte(secret))).ThenReturn([]byte(event), nil) w := httptest.NewRecorder() - e.Post(w, req) + e.Post(w, eventsReq) responseContains(t, w, http.StatusOK, "Ignoring comment event since action was not created") } func TestPost_CommentInvalidComment(t *testing.T) { t.Log("when the event is a comment without all expected data we return a 400") e, v, p, _, _ := setup(t) - req.Header.Set("X-Github-Event", "issue_comment") + eventsReq.Header.Set("X-Github-Event", "issue_comment") event := `{"action": "created"}` - When(v.Validate(req, []byte(secret))).ThenReturn([]byte(event), nil) + When(v.Validate(eventsReq, []byte(secret))).ThenReturn([]byte(event), nil) When(p.ExtractCommentData(AnyComment())).ThenReturn(models.Repo{}, models.User{}, models.PullRequest{}, errors.New("err")) w := httptest.NewRecorder() - e.Post(w, req) + e.Post(w, eventsReq) responseContains(t, w, http.StatusBadRequest, "Failed parsing event") } func TestPost_CommentInvalidCommand(t *testing.T) { t.Log("when the event is a comment with an invalid command we ignore it") e, v, p, _, _ := setup(t) - req.Header.Set("X-Github-Event", "issue_comment") + eventsReq.Header.Set("X-Github-Event", "issue_comment") event := `{"action": "created"}` - When(v.Validate(req, []byte(secret))).ThenReturn([]byte(event), nil) + When(v.Validate(eventsReq, []byte(secret))).ThenReturn([]byte(event), nil) When(p.ExtractCommentData(AnyComment())).ThenReturn(models.Repo{}, models.User{}, models.PullRequest{}, nil) When(p.DetermineCommand(AnyComment())).ThenReturn(nil, errors.New("err")) w := httptest.NewRecorder() - e.Post(w, req) + e.Post(w, eventsReq) responseContains(t, w, http.StatusOK, "Ignoring: err") } func TestPost_CommentSuccess(t *testing.T) { t.Log("when the event is comment with a valid command we call the command handler") e, v, p, cr, _ := setup(t) - req.Header.Set("X-Github-Event", "issue_comment") + eventsReq.Header.Set("X-Github-Event", "issue_comment") event := `{"action": "created"}` - When(v.Validate(req, []byte(secret))).ThenReturn([]byte(event), nil) + When(v.Validate(eventsReq, []byte(secret))).ThenReturn([]byte(event), nil) baseRepo := models.Repo{} user := models.User{} pull := models.PullRequest{} @@ -92,7 +92,7 @@ func TestPost_CommentSuccess(t *testing.T) { When(p.ExtractCommentData(AnyComment())).ThenReturn(baseRepo, user, pull, nil) When(p.DetermineCommand(AnyComment())).ThenReturn(&cmd, nil) w := httptest.NewRecorder() - e.Post(w, req) + e.Post(w, eventsReq) responseContains(t, w, http.StatusOK, "Processing...") // wait for 200ms so goroutine is called @@ -107,38 +107,38 @@ func TestPost_CommentSuccess(t *testing.T) { func TestPost_PullRequestNotClosed(t *testing.T) { t.Log("when the event is pull reuqest but it's not a closed event we ignore it") e, v, _, _, _ := setup(t) - req.Header.Set("X-Github-Event", "pull_request") + eventsReq.Header.Set("X-Github-Event", "pull_request") event := `{"action": "opened"}` - When(v.Validate(req, []byte(secret))).ThenReturn([]byte(event), nil) + When(v.Validate(eventsReq, []byte(secret))).ThenReturn([]byte(event), nil) w := httptest.NewRecorder() - e.Post(w, req) + e.Post(w, eventsReq) responseContains(t, w, http.StatusOK, "Ignoring pull request event since action was not closed") } func TestPost_PullRequestInvalid(t *testing.T) { t.Log("when the event is pull request with invalid data we return a 400") e, v, p, _, _ := setup(t) - req.Header.Set("X-Github-Event", "pull_request") + eventsReq.Header.Set("X-Github-Event", "pull_request") event := `{"action": "closed"}` - When(v.Validate(req, []byte(secret))).ThenReturn([]byte(event), nil) + When(v.Validate(eventsReq, []byte(secret))).ThenReturn([]byte(event), nil) When(p.ExtractPullData(AnyPull())).ThenReturn(models.PullRequest{}, models.Repo{}, errors.New("err")) w := httptest.NewRecorder() - e.Post(w, req) + e.Post(w, eventsReq) responseContains(t, w, http.StatusBadRequest, "Error parsing pull data: err") } func TestPost_PullRequestInvalidRepo(t *testing.T) { t.Log("when the event is pull reuqest with invalid repo data we return a 400") e, v, p, _, _ := setup(t) - req.Header.Set("X-Github-Event", "pull_request") + eventsReq.Header.Set("X-Github-Event", "pull_request") event := `{"action": "closed"}` - When(v.Validate(req, []byte(secret))).ThenReturn([]byte(event), nil) + When(v.Validate(eventsReq, []byte(secret))).ThenReturn([]byte(event), nil) When(p.ExtractPullData(AnyPull())).ThenReturn(models.PullRequest{}, models.Repo{}, nil) When(p.ExtractRepoData(AnyRepo())).ThenReturn(models.Repo{}, errors.New("err")) w := httptest.NewRecorder() - e.Post(w, req) + e.Post(w, eventsReq) responseContains(t, w, http.StatusBadRequest, "Error parsing repo data: err") } @@ -146,40 +146,40 @@ func TestPost_PullRequestErrCleaningPull(t *testing.T) { t.Log("when the event is a pull request and we have an error calling CleanUpPull we return a 503") RegisterMockTestingT(t) e, v, p, _, c := setup(t) - req.Header.Set("X-Github-Event", "pull_request") + eventsReq.Header.Set("X-Github-Event", "pull_request") event := `{"action": "closed"}` - When(v.Validate(req, []byte(secret))).ThenReturn([]byte(event), nil) + When(v.Validate(eventsReq, []byte(secret))).ThenReturn([]byte(event), nil) repo := models.Repo{} pull := models.PullRequest{} When(p.ExtractPullData(AnyPull())).ThenReturn(pull, repo, nil) When(p.ExtractRepoData(AnyRepo())).ThenReturn(repo, nil) When(c.CleanUpPull(repo, pull)).ThenReturn(errors.New("cleanup err")) w := httptest.NewRecorder() - e.Post(w, req) + e.Post(w, eventsReq) responseContains(t, w, http.StatusInternalServerError, "Error cleaning pull request: cleanup err") } func TestPost_PullRequestSuccess(t *testing.T) { t.Log("when the event is a pull request and everything works we return a 200") e, v, p, _, c := setup(t) - req.Header.Set("X-Github-Event", "pull_request") + eventsReq.Header.Set("X-Github-Event", "pull_request") event := `{"action": "closed"}` - When(v.Validate(req, []byte(secret))).ThenReturn([]byte(event), nil) + When(v.Validate(eventsReq, []byte(secret))).ThenReturn([]byte(event), nil) repo := models.Repo{} pull := models.PullRequest{} When(p.ExtractPullData(AnyPull())).ThenReturn(pull, repo, nil) When(p.ExtractRepoData(AnyRepo())).ThenReturn(repo, nil) When(c.CleanUpPull(repo, pull)).ThenReturn(nil) w := httptest.NewRecorder() - e.Post(w, req) + e.Post(w, eventsReq) responseContains(t, w, http.StatusOK, "Pull request cleaned successfully") } func setup(t *testing.T) (server.EventsController, *mocks.MockGHRequestValidator, *emocks.MockEventParsing, *emocks.MockCommandRunner, *emocks.MockPullCleaner) { RegisterMockTestingT(t) - req, _ = http.NewRequest("GET", "", bytes.NewBuffer(nil)) + eventsReq, _ = http.NewRequest("GET", "", bytes.NewBuffer(nil)) v := mocks.NewMockGHRequestValidator() p := emocks.NewMockEventParsing() cr := emocks.NewMockCommandRunner() diff --git a/server/server.go b/server/server.go index 6d6018722..7e8127fa7 100644 --- a/server/server.go +++ b/server/server.go @@ -207,7 +207,7 @@ func (s *Server) GetLockRoute(w http.ResponseWriter, r *http.Request) { // GetLock handles a lock detail page view. getLockRoute is expected to // be called before. This function was extracted to make it testable. -func (s *Server) GetLock(w http.ResponseWriter, r *http.Request, id string) { +func (s *Server) GetLock(w http.ResponseWriter, _ *http.Request, id string) { // get details for lock id idUnencoded, err := url.QueryUnescape(id) if err != nil { @@ -254,7 +254,7 @@ func (s *Server) DeleteLockRoute(w http.ResponseWriter, r *http.Request) { s.DeleteLock(w, r, id) } -func (s *Server) DeleteLock(w http.ResponseWriter, r *http.Request, id string) { +func (s *Server) DeleteLock(w http.ResponseWriter, _ *http.Request, id string) { idUnencoded, err := url.PathUnescape(id) if err != nil { s.respond(w, logging.Warn, http.StatusBadRequest, "Invalid lock id: %s", err) diff --git a/server/server_test.go b/server/server_test.go index 8be5499cb..057622620 100644 --- a/server/server_test.go +++ b/server/server_test.go @@ -28,9 +28,9 @@ func TestIndex_LockErr(t *testing.T) { s := server.Server{ Locker: l, } - req, _ = http.NewRequest("GET", "", bytes.NewBuffer(nil)) + eventsReq, _ = http.NewRequest("GET", "", bytes.NewBuffer(nil)) w := httptest.NewRecorder() - s.Index(w, req) + s.Index(w, eventsReq) responseContains(t, w, 503, "Could not retrieve locks: err") } @@ -61,9 +61,9 @@ func TestIndex_Success(t *testing.T) { IndexTemplate: it, Router: r, } - req, _ = http.NewRequest("GET", "", bytes.NewBuffer(nil)) + eventsReq, _ = http.NewRequest("GET", "", bytes.NewBuffer(nil)) w := httptest.NewRecorder() - s.Index(w, req) + s.Index(w, eventsReq) it.VerifyWasCalledOnce().Execute(w, []server.LockIndexData{ { LockURL: "", @@ -77,19 +77,19 @@ func TestIndex_Success(t *testing.T) { func TestGetLockRoute_NoLockID(t *testing.T) { t.Log("If there is no lock ID in the request then we should get a 400") - req, _ = http.NewRequest("GET", "", bytes.NewBuffer(nil)) + eventsReq, _ = http.NewRequest("GET", "", bytes.NewBuffer(nil)) w := httptest.NewRecorder() s := server.Server{} - s.GetLockRoute(w, req) + s.GetLockRoute(w, eventsReq) responseContains(t, w, http.StatusBadRequest, "No lock id in request") } func TestGetLock_InvalidLockID(t *testing.T) { t.Log("If the lock ID is invalid then we should get a 400") s := server.Server{} - req, _ = http.NewRequest("GET", "", bytes.NewBuffer(nil)) + eventsReq, _ = http.NewRequest("GET", "", bytes.NewBuffer(nil)) w := httptest.NewRecorder() - s.GetLock(w, req, "%A@") + s.GetLock(w, eventsReq, "%A@") responseContains(t, w, http.StatusBadRequest, "Invalid lock id") } @@ -101,9 +101,9 @@ func TestGetLock_LockerErr(t *testing.T) { s := server.Server{ Locker: l, } - req, _ = http.NewRequest("GET", "", bytes.NewBuffer(nil)) + eventsReq, _ = http.NewRequest("GET", "", bytes.NewBuffer(nil)) w := httptest.NewRecorder() - s.GetLock(w, req, "id") + s.GetLock(w, eventsReq, "id") responseContains(t, w, http.StatusInternalServerError, "err") } @@ -115,9 +115,9 @@ func TestGetLock_None(t *testing.T) { s := server.Server{ Locker: l, } - req, _ = http.NewRequest("GET", "", bytes.NewBuffer(nil)) + eventsReq, _ = http.NewRequest("GET", "", bytes.NewBuffer(nil)) w := httptest.NewRecorder() - s.GetLock(w, req, "id") + s.GetLock(w, eventsReq, "id") responseContains(t, w, http.StatusNotFound, "No lock found at that id") } @@ -135,9 +135,9 @@ func TestGetLock_Success(t *testing.T) { Locker: l, LockDetailTemplate: tmpl, } - req, _ = http.NewRequest("GET", "", bytes.NewBuffer(nil)) + eventsReq, _ = http.NewRequest("GET", "", bytes.NewBuffer(nil)) w := httptest.NewRecorder() - s.GetLock(w, req, "id") + s.GetLock(w, eventsReq, "id") tmpl.VerifyWasCalledOnce().Execute(w, server.LockDetailData{ LockKeyEncoded: "id", LockKey: "id", @@ -152,19 +152,19 @@ func TestGetLock_Success(t *testing.T) { func TestDeleteLockRoute_NoLockID(t *testing.T) { t.Log("If there is no lock ID in the request then we should get a 400") - req, _ = http.NewRequest("GET", "", bytes.NewBuffer(nil)) + eventsReq, _ = http.NewRequest("GET", "", bytes.NewBuffer(nil)) w := httptest.NewRecorder() s := server.Server{Logger: logging.NewNoopLogger()} - s.DeleteLockRoute(w, req) + s.DeleteLockRoute(w, eventsReq) responseContains(t, w, http.StatusBadRequest, "No lock id in request") } func TestDeleteLock_InvalidLockID(t *testing.T) { t.Log("If the lock ID is invalid then we should get a 400") s := server.Server{Logger: logging.NewNoopLogger()} - req, _ = http.NewRequest("GET", "", bytes.NewBuffer(nil)) + eventsReq, _ = http.NewRequest("GET", "", bytes.NewBuffer(nil)) w := httptest.NewRecorder() - s.DeleteLock(w, req, "%A@") + s.DeleteLock(w, eventsReq, "%A@") responseContains(t, w, http.StatusBadRequest, "Invalid lock id") } @@ -177,9 +177,9 @@ func TestDeleteLock_LockerErr(t *testing.T) { Locker: l, Logger: logging.NewNoopLogger(), } - req, _ = http.NewRequest("GET", "", bytes.NewBuffer(nil)) + eventsReq, _ = http.NewRequest("GET", "", bytes.NewBuffer(nil)) w := httptest.NewRecorder() - s.DeleteLock(w, req, "id") + s.DeleteLock(w, eventsReq, "id") responseContains(t, w, http.StatusInternalServerError, "err") } @@ -192,9 +192,9 @@ func TestDeleteLock_None(t *testing.T) { Locker: l, Logger: logging.NewNoopLogger(), } - req, _ = http.NewRequest("GET", "", bytes.NewBuffer(nil)) + eventsReq, _ = http.NewRequest("GET", "", bytes.NewBuffer(nil)) w := httptest.NewRecorder() - s.DeleteLock(w, req, "id") + s.DeleteLock(w, eventsReq, "id") responseContains(t, w, http.StatusNotFound, "No lock found at that id") } @@ -207,9 +207,9 @@ func TestDeleteLock_Success(t *testing.T) { Locker: l, Logger: logging.NewNoopLogger(), } - req, _ = http.NewRequest("GET", "", bytes.NewBuffer(nil)) + eventsReq, _ = http.NewRequest("GET", "", bytes.NewBuffer(nil)) w := httptest.NewRecorder() - s.DeleteLock(w, req, "id") + s.DeleteLock(w, eventsReq, "id") responseContains(t, w, http.StatusOK, "Deleted lock id id") }