Merge pull request #23 from kitagry/fix-last-one

Fix comment to associated MR when push pipeline

pkg/notifier/gitlab/comment_test.go

@@ -1,6 +1,7 @@
 package gitlab
 
 import (
+	"errors"
 	"reflect"
 	"testing"
 
@@ -81,6 +82,23 @@ func TestCommentPost(t *testing.T) {
 			},
 			ok: false,
 		},
+		{
+			name:   "should postForRevision when listMergeRequestIIDs is failed",
+			config: newFakeConfig(),
+			createMockGitLabAPI: func(ctrl *gomock.Controller) *gitlabmock.MockAPI {
+				api := gitlabmock.NewMockAPI(ctrl)
+				api.EXPECT().ListMergeRequestsByCommit("revision").Return(nil, nil, errors.New("error"))
+				// PostCommitComment should be called
+				api.EXPECT().PostCommitComment("revision", &gitlab.PostCommitCommentOptions{Note: gitlab.String(body)}).Return(&gitlab.CommitComment{}, nil, nil)
+				return api
+			},
+			body: body,
+			opt: PostOptions{
+				Number:   0,
+				Revision: "revision",
+			},
+			ok: true,
+		},
 	}
 
 	for _, testCase := range testCases {

pkg/notifier/gitlab/commits.go

@@ -2,32 +2,12 @@ package gitlab
 
 import (
 	"errors"
-
-	gitlab "github.com/xanzy/go-gitlab"
 )
 
 // CommitsService handles communication with the commits related
 // methods of GitLab API
 type CommitsService service
 
-// List lists commits on a repository
-func (g *CommitsService) List(revision string) ([]string, error) {
-	if revision == "" {
-		return []string{}, errors.New("no revision specified")
-	}
-	commits, _, err := g.client.API.ListCommits(
-		&gitlab.ListCommitsOptions{},
-	)
-	if err != nil {
-		return nil, err
-	}
-	s := make([]string, len(commits))
-	for i, commit := range commits {
-		s[i] = commit.ID
-	}
-	return s, nil
-}
-
 func (g *CommitsService) ListMergeRequestIIDsByRevision(revision string) ([]int, error) {
 	if revision == "" {
 		return nil, errors.New("no revision specified")
@@ -43,15 +23,3 @@ func (g *CommitsService) ListMergeRequestIIDsByRevision(revision string) ([]int,
 	}
 	return result, nil
 }
-
-// lastOne returns the hash of the previous commit of the given commit
-func (g *CommitsService) lastOne(commits []string, revision string) (string, error) {
-	if revision == "" {
-		return "", errors.New("no revision specified")
-	}
-	if len(commits) == 0 {
-		return "", errors.New("no commits")
-	}
-
-	return commits[1], nil
-}

pkg/notifier/gitlab/gitlab.go

@@ -14,7 +14,6 @@ type API interface {
 	GetMergeRequest(mergeRequest int, opt *gitlab.GetMergeRequestsOptions, options ...gitlab.RequestOptionFunc) (*gitlab.MergeRequest, *gitlab.Response, error)
 	UpdateMergeRequest(mergeRequest int, opt *gitlab.UpdateMergeRequestOptions, options ...gitlab.RequestOptionFunc) (*gitlab.MergeRequest, *gitlab.Response, error)
 	PostCommitComment(sha string, opt *gitlab.PostCommitCommentOptions, options ...gitlab.RequestOptionFunc) (*gitlab.CommitComment, *gitlab.Response, error)
-	ListCommits(opt *gitlab.ListCommitsOptions, options ...gitlab.RequestOptionFunc) ([]*gitlab.Commit, *gitlab.Response, error)
 	AddMergeRequestLabels(labels *[]string, mergeRequest int) (gitlab.Labels, error)
 	RemoveMergeRequestLabels(labels *[]string, mergeRequest int) (gitlab.Labels, error)
 	ListMergeRequestLabels(mergeRequest int, opt *gitlab.GetMergeRequestsOptions, options ...gitlab.RequestOptionFunc) (gitlab.Labels, error)
@@ -60,11 +59,6 @@ func (g *GitLab) PostCommitComment(sha string, opt *gitlab.PostCommitCommentOpti
 	return g.Client.Commits.PostCommitComment(fmt.Sprintf("%s/%s", g.namespace, g.project), sha, opt, options...)
 }
 
-// ListCommits is a wrapper of https://godoc.org/github.com/xanzy/go-gitlab#CommitsService.ListCommits
-func (g *GitLab) ListCommits(opt *gitlab.ListCommitsOptions, options ...gitlab.RequestOptionFunc) ([]*gitlab.Commit, *gitlab.Response, error) {
-	return g.Client.Commits.ListCommits(fmt.Sprintf("%s/%s", g.namespace, g.project), opt, options...)
-}
-
 // AddMergeRequestLabels adds labels on the merge request.
 func (g *GitLab) AddMergeRequestLabels(labels *[]string, mergeRequest int) (gitlab.Labels, error) {
 	var addLabels gitlab.Labels

pkg/notifier/gitlab/notify.go

@@ -64,17 +64,6 @@ func (g *NotifyService) Notify(param notifier.ParamExec) (int, error) { //nolint
 	}
 
 	_, isApply := parser.(*terraform.ApplyParser)
-	if isApply {
-		if !cfg.MR.IsNumber() {
-			commits, err := g.client.Commits.List(cfg.MR.Revision)
-			if err != nil {
-				return result.ExitCode, err
-			}
-			lastRevision, _ := g.client.Commits.lastOne(commits, cfg.MR.Revision)
-			cfg.MR.Revision = lastRevision
-		}
-	}
-
 	logE := logrus.WithFields(logrus.Fields{
 		"program": "tfcmt",
 	})

pkg/notifier/gitlab/notify_test.go

@@ -234,11 +234,10 @@ func TestNotifyNotify(t *testing.T) { //nolint:maintidx
 			exitCode: 2,
 		},
 		{
-			name: "apply case",
+			name: "get MR IID when MR number is 0",
 			createMockGitLabAPI: func(ctrl *gomock.Controller) *gitlabmock.MockAPI {
 				api := gitlabmock.NewMockAPI(ctrl)
-				api.EXPECT().ListCommits(gomock.Any()).Return([]*gitlab.Commit{{ID: "1"}, {ID: "2"}}, nil, nil)
-				api.EXPECT().ListMergeRequestsByCommit("2").Return([]*gitlab.MergeRequest{{IID: 1}}, nil, nil)
+				api.EXPECT().ListMergeRequestsByCommit("revision").Return([]*gitlab.MergeRequest{{IID: 1}}, nil, nil)
 				api.EXPECT().CreateMergeRequestNote(1, gomock.Any()).Return(nil, nil, nil)
 				return api
 			},