From ecd69d7a1e15e36afb959b04635dd5cc381bf266 Mon Sep 17 00:00:00 2001 From: Sam Kiessler Date: Tue, 20 Dec 2022 08:45:16 +0000 Subject: [PATCH 1/4] Adding GetIsDeleted --- server/controllers/events/events_controller.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/server/controllers/events/events_controller.go b/server/controllers/events/events_controller.go index 7be7fa49ea..ddf7177054 100644 --- a/server/controllers/events/events_controller.go +++ b/server/controllers/events/events_controller.go @@ -627,7 +627,7 @@ func (e *VCSEventsController) HandleAzureDevopsPullRequestCommentedEvent(w http. return } - if *resource.Comment.IsDeleted { + if resource.Comment.GetIsDeleted() { e.respond(w, logging.Debug, http.StatusOK, "Ignoring comment event since it is linked to deleting a pull request comment; %s", azuredevopsReqID) return } From 342bcd480452fb157eee920005a237ffae163b84 Mon Sep 17 00:00:00 2001 From: Sam Kiessler Date: Tue, 20 Dec 2022 10:20:55 +0000 Subject: [PATCH 2/4] Adding test to see if ADO sucessfully passes all ignore statements --- .../events/events_controller_test.go | 67 +++++++++++++++++++ 1 file changed, 67 insertions(+) diff --git a/server/controllers/events/events_controller_test.go b/server/controllers/events/events_controller_test.go index 8bba02213b..ca67f36624 100644 --- a/server/controllers/events/events_controller_test.go +++ b/server/controllers/events/events_controller_test.go @@ -618,6 +618,73 @@ func TestPost_AzureDevopsPullRequestDeletedCommentIgnoreEvent(t *testing.T) { }) } +func TestPost_AzureDevopsPullRequestCommentPassingIgnores(t *testing.T) { + u := "user" + user := []byte(u) + + t.Log("when the event should not be ignored it should succeed until parsing section") + RegisterMockTestingT(t) + v := mocks.NewMockAzureDevopsRequestValidator() + p := emocks.NewMockEventParsing() + cp := emocks.NewMockCommentParsing() + cr := emocks.NewMockCommandRunner() + c := emocks.NewMockPullCleaner() + vcsmock := vcsmocks.NewMockClient() + repoAllowlistChecker, err := events.NewRepoAllowlistChecker("*") + Ok(t, err) + logger := logging.NewNoopLogger(t) + scope, _, _ := metrics.NewLoggingScope(logger, "null") + e := events_controllers.VCSEventsController{ + TestingMode: true, + Logger: logger, + Scope: scope, + ApplyDisabled: false, + AzureDevopsWebhookBasicUser: user, + AzureDevopsWebhookBasicPassword: secret, + AzureDevopsRequestValidator: v, + Parser: p, + CommentParser: cp, + CommandRunner: cr, + PullCleaner: c, + SupportedVCSHosts: []models.VCSHostType{models.AzureDevops}, + RepoAllowlistChecker: repoAllowlistChecker, + VCSClient: vcsmock, + } + + payload := `{ + "subscriptionId": "11111111-1111-1111-1111-111111111111", + "notificationId": 1, + "id": "22222222-2222-2222-2222-222222222222", + "eventType": "ms.vss-code.git-pullrequest-comment-event", + "publisherId": "tfs", + "message": { + "text": "Testing to see if comment passes ignore conditions" + }, + "resource": { + "comment": { + "id": 1, + "commentType": "text", + "content": "test" + }, + "PullRequest": { + "pullRequestId": 1, + "Repository": "" + } + } + }` + + t.Run("Testing to see if comment passes ignore conditions", func(t *testing.T) { + req, _ := http.NewRequest("GET", "", strings.NewReader(payload)) + req.Header.Set(azuredevopsHeader, "reqID") + When(v.Validate(req, user, secret)).ThenReturn([]byte(payload), nil) + w := httptest.NewRecorder() + e.Parser = &events.EventParser{} + e.Post(w, req) + t.Log(w) + ResponseContains(t, w, http.StatusBadRequest, "Failed parsing webhook: json: cannot unmarshal string into Go struct field") + }) +} + func TestPost_GithubPullRequestClosedErrCleaningPull(t *testing.T) { t.Skip("relies too much on mocks, should use real event parser") t.Log("when the event is a closed pull request and we have an error calling CleanUpPull we return a 503") From 544c9468cf6cf85ea5a57b520a3aaa03556ccb63 Mon Sep 17 00:00:00 2001 From: Sam Kiessler Date: Thu, 22 Dec 2022 09:35:53 +0000 Subject: [PATCH 3/4] Changing Ignore Passing test to use matcher --- server/controllers/events/events_controller_test.go | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/server/controllers/events/events_controller_test.go b/server/controllers/events/events_controller_test.go index ca67f36624..ad211ab3b1 100644 --- a/server/controllers/events/events_controller_test.go +++ b/server/controllers/events/events_controller_test.go @@ -622,7 +622,7 @@ func TestPost_AzureDevopsPullRequestCommentPassingIgnores(t *testing.T) { u := "user" user := []byte(u) - t.Log("when the event should not be ignored it should succeed until parsing section") + t.Log("when the event should not be ignored it should pass through all ignore statements without error") RegisterMockTestingT(t) v := mocks.NewMockAzureDevopsRequestValidator() p := emocks.NewMockEventParsing() @@ -651,6 +651,9 @@ func TestPost_AzureDevopsPullRequestCommentPassingIgnores(t *testing.T) { VCSClient: vcsmock, } + repo := models.Repo{} + When(e.Parser.ParseAzureDevopsRepo(matchers.AnyPtrToAzuredevopsGitRepository())).ThenReturn(repo, nil) + payload := `{ "subscriptionId": "11111111-1111-1111-1111-111111111111", "notificationId": 1, @@ -666,9 +669,9 @@ func TestPost_AzureDevopsPullRequestCommentPassingIgnores(t *testing.T) { "commentType": "text", "content": "test" }, - "PullRequest": { + "pullRequest": { "pullRequestId": 1, - "Repository": "" + "repository": {} } } }` @@ -678,10 +681,8 @@ func TestPost_AzureDevopsPullRequestCommentPassingIgnores(t *testing.T) { req.Header.Set(azuredevopsHeader, "reqID") When(v.Validate(req, user, secret)).ThenReturn([]byte(payload), nil) w := httptest.NewRecorder() - e.Parser = &events.EventParser{} e.Post(w, req) - t.Log(w) - ResponseContains(t, w, http.StatusBadRequest, "Failed parsing webhook: json: cannot unmarshal string into Go struct field") + ResponseContains(t, w, http.StatusOK, "Processing...") }) } From 30287f627d37362983d9c816f341fd12fdb7766c Mon Sep 17 00:00:00 2001 From: Sam Kiessler Date: Thu, 22 Dec 2022 14:15:22 +0000 Subject: [PATCH 4/4] Changing to use setup method --- .../events/events_controller_test.go | 33 ++----------------- 1 file changed, 2 insertions(+), 31 deletions(-) diff --git a/server/controllers/events/events_controller_test.go b/server/controllers/events/events_controller_test.go index 56c5ea2df0..5722627d42 100644 --- a/server/controllers/events/events_controller_test.go +++ b/server/controllers/events/events_controller_test.go @@ -562,37 +562,8 @@ func TestPost_AzureDevopsPullRequestDeletedCommentIgnoreEvent(t *testing.T) { } func TestPost_AzureDevopsPullRequestCommentPassingIgnores(t *testing.T) { - u := "user" - user := []byte(u) - t.Log("when the event should not be ignored it should pass through all ignore statements without error") - RegisterMockTestingT(t) - v := mocks.NewMockAzureDevopsRequestValidator() - p := emocks.NewMockEventParsing() - cp := emocks.NewMockCommentParsing() - cr := emocks.NewMockCommandRunner() - c := emocks.NewMockPullCleaner() - vcsmock := vcsmocks.NewMockClient() - repoAllowlistChecker, err := events.NewRepoAllowlistChecker("*") - Ok(t, err) - logger := logging.NewNoopLogger(t) - scope, _, _ := metrics.NewLoggingScope(logger, "null") - e := events_controllers.VCSEventsController{ - TestingMode: true, - Logger: logger, - Scope: scope, - ApplyDisabled: false, - AzureDevopsWebhookBasicUser: user, - AzureDevopsWebhookBasicPassword: secret, - AzureDevopsRequestValidator: v, - Parser: p, - CommentParser: cp, - CommandRunner: cr, - PullCleaner: c, - SupportedVCSHosts: []models.VCSHostType{models.AzureDevops}, - RepoAllowlistChecker: repoAllowlistChecker, - VCSClient: vcsmock, - } + e, _, _, ado, _, _, _, _, _ := setup(t) repo := models.Repo{} When(e.Parser.ParseAzureDevopsRepo(matchers.AnyPtrToAzuredevopsGitRepository())).ThenReturn(repo, nil) @@ -622,7 +593,7 @@ func TestPost_AzureDevopsPullRequestCommentPassingIgnores(t *testing.T) { t.Run("Testing to see if comment passes ignore conditions", func(t *testing.T) { req, _ := http.NewRequest("GET", "", strings.NewReader(payload)) req.Header.Set(azuredevopsHeader, "reqID") - When(v.Validate(req, user, secret)).ThenReturn([]byte(payload), nil) + When(ado.Validate(req, user, secret)).ThenReturn([]byte(payload), nil) w := httptest.NewRecorder() e.Post(w, req) ResponseContains(t, w, http.StatusOK, "Processing...")