Merge branch 'main' into fix/commit-quality-pr-comment-format

Signed-off-by: Paul Horton <[email protected]>

.github/workflows/ci.yml

@@ -3,9 +3,9 @@ name: Continuous Integration
 # This GitHub action runs your tests for each pull request and push.
 # Optionally, you can turn it on using a schedule for regular testing.
 on:
-    pull_request:
-        paths-ignore:
-            - 'README.md'
+    # pull_request:
+    #     paths-ignore:
+    #         - 'README.md'
     pull_request_target:
         branches:
             - main

github/github.go

@@ -353,11 +353,11 @@ The commits to review are:
 %s`
 		commitsMessage := ""
 		for _, c := range commitsMissingAuthor {
-			commitsMessage += commitsMessage + fmt.Sprintf(`- <a href="%s">%s</a> - missing author :cop:
+			commitsMessage += fmt.Sprintf(`- <a href="%s">%s</a> - missing author :cop:
 `, *c.HTMLURL, *c.SHA)
 		}
 		for _, c := range commitsMissingVerification {
-			commitsMessage += commitsMessage + fmt.Sprintf(`- <a href="%s">%s</a> - unsigned commit :key:
+			commitsMessage += fmt.Sprintf(`- <a href="%s">%s</a> - unsigned commit :key:
 `, *c.HTMLURL, *c.SHA)
 		}
 		logger.Debug("Adding Comment to Issue", zap.Int("Issue #", int(evalInfo.PRNumber)), zap.String("Comment", fmt.Sprintf(message, commitsMessage)))
@@ -368,6 +368,11 @@ The commits to review are:
 			return err
 		}
 
+		err = createRepoStatus(client.Repositories, evalInfo.RepoOwner, evalInfo.RepoName, evalInfo.Sha, "failure", "One or more commits haven't met our Quality requirements.", botName)
+		if err != nil {
+			return err
+		}
+
 		return nil
 	}
 

github/github_mocks.go

@@ -31,23 +31,29 @@ import (
 
 // RepositoriesMock mocks RepositoriesService
 type RepositoriesMock struct {
-	t                                                      *testing.T
-	assertParameters                                       bool
-	expectedCtx                                            context.Context
-	expectedOwner, expectedRepo, expectedRef, expectedUser string
+	t *testing.T
+	/* callCount is the number of times the CreateStatus function has been called by production code. */
+	callCount int
+	/* assertParameters is a slice of booleans that determine whether to assert the parameters passed to the function for
+	each call to the CreateStatus function. If the value at the index of the callCount is true, the parameters will be
+	asserted.
+	*/
+	assertParameters                                       []bool
+	expectedCtx                                            []context.Context
+	expectedOwner, expectedRepo, expectedRef, expectedUser []string
 	// expectedOpts                                           *github.ListOptions
-	expectedCreateStatusRepoStatus *github.RepoStatus
-	createStatusRepoStatus         *github.RepoStatus
-	createStatusResponse           *github.Response
-	createStatusError              error
+	expectedCreateStatusRepoStatus []*github.RepoStatus
+	createStatusRepoStatus         []*github.RepoStatus
+	createStatusResponse           []*github.Response
+	createStatusError              []error
 	isCollaboratorResult           bool
 	isCollaboratorResp             *github.Response
 	isCollaboratorErr              error
 }
 
 var _ RepositoriesService = (*RepositoriesMock)(nil)
 
-func setupMockRepositoriesService(t *testing.T, assertParameters bool) (mock *RepositoriesMock) {
+func setupMockRepositoriesService(t *testing.T, assertParameters []bool) (mock *RepositoriesMock) {
 	mock = &RepositoriesMock{
 		t:                t,
 		assertParameters: assertParameters,
@@ -61,15 +67,38 @@ func (r *RepositoriesMock) ListStatuses(ctx context.Context, owner, repo, ref st
 	panic("implement me")
 }
 
-func (r *RepositoriesMock) CreateStatus(ctx context.Context, owner, repo, ref string, status *github.RepoStatus) (*github.RepoStatus, *github.Response, error) {
-	if r.assertParameters {
-		assert.Equal(r.t, r.expectedCtx, ctx)
-		assert.Equal(r.t, r.expectedOwner, owner)
-		assert.Equal(r.t, r.expectedRepo, repo)
-		assert.Equal(r.t, r.expectedRef, ref)
-		assert.Equal(r.t, r.expectedCreateStatusRepoStatus, status)
+func (r *RepositoriesMock) CreateStatus(ctx context.Context, owner, repo, ref string, status *github.RepoStatus) (retRepoStatus *github.RepoStatus, createStatusResponse *github.Response, createStatusError error) {
+	defer func() { r.callCount++ }()
+	if r.assertParameters != nil && r.assertParameters[r.callCount] {
+		if r.expectedCtx != nil {
+			assert.Equal(r.t, r.expectedCtx[r.callCount], ctx)
+		}
+		if r.expectedOwner != nil {
+			assert.Equal(r.t, r.expectedOwner[r.callCount], owner)
+		}
+		if r.expectedRepo != nil {
+			assert.Equal(r.t, r.expectedRepo[r.callCount], repo)
+		}
+		if r.expectedRef != nil {
+			assert.Equal(r.t, r.expectedRef[r.callCount], ref)
+		}
+		if r.expectedCreateStatusRepoStatus != nil {
+			assert.Equal(r.t, r.expectedCreateStatusRepoStatus[r.callCount], status)
+		}
+	}
+
+	if r.createStatusRepoStatus != nil {
+		retRepoStatus = r.createStatusRepoStatus[r.callCount]
+	}
+
+	if r.createStatusRepoStatus != nil {
+		createStatusResponse = r.createStatusResponse[r.callCount]
 	}
-	return r.createStatusRepoStatus, r.createStatusResponse, r.createStatusError
+
+	if r.createStatusError != nil {
+		createStatusError = r.createStatusError[r.callCount]
+	}
+	return
 }
 
 // Get returns a repository.
@@ -86,11 +115,11 @@ func (r *RepositoriesMock) Get(context.Context, string, string) (*github.Reposit
 }
 
 func (r *RepositoriesMock) IsCollaborator(ctx context.Context, owner, repo, user string) (bool, *github.Response, error) {
-	if r.assertParameters {
-		assert.Equal(r.t, r.expectedCtx, ctx)
-		assert.Equal(r.t, r.expectedOwner, owner)
-		assert.Equal(r.t, r.expectedRepo, repo)
-		assert.Equal(r.t, r.expectedUser, user)
+	if r.assertParameters != nil && r.assertParameters[r.callCount] {
+		assert.Equal(r.t, r.expectedCtx[r.callCount], ctx)
+		assert.Equal(r.t, r.expectedOwner[r.callCount], owner)
+		assert.Equal(r.t, r.expectedRepo[r.callCount], repo)
+		assert.Equal(r.t, r.expectedUser[r.callCount], user)
 	}
 	return r.isCollaboratorResult, r.isCollaboratorResp, r.isCollaboratorErr
 }
@@ -204,7 +233,7 @@ func (a *AppsMock) GetInstallation(ctx context.Context, id int64) (*github.Insta
 	return a.mockInstallation, a.mockInstallationResp, a.mockInstallationError
 }
 
-var appSlug = "myAppSlug"
+var MockAppSlug = "myAppSlug"
 
 func SetupMockGHJWT() (resetImpl func()) {
 	origGHJWT := GHJWTImpl
@@ -214,7 +243,7 @@ func SetupMockGHJWT() (resetImpl func()) {
 	GHJWTImpl = &GHJWTMock{
 		AppsMock: AppsMock{
 			mockInstallation: &github.Installation{
-				AppSlug: &appSlug,
+				AppSlug: &MockAppSlug,
 			},
 		},
 	}

github/github_test.go

@@ -20,6 +20,7 @@
 package github
 
 import (
+	"context"
 	"fmt"
 	"net/http"
 	"os"
@@ -351,7 +352,7 @@ func TestWithFullEnvironment(t *testing.T) {
 	t.Run("TestHandlePullRequestListCommitsError", func(t *testing.T) {
 		forcedError := fmt.Errorf("forced ListCommits error")
 		GHImpl = &GHInterfaceMock{
-			RepositoriesMock: *setupMockRepositoriesService(t, false),
+			RepositoriesMock: *setupMockRepositoriesService(t, []bool{false}),
 			PullRequestsMock: PullRequestsMock{
 				mockListCommitsError: forcedError,
 			},
@@ -369,7 +370,7 @@ func TestWithFullEnvironment(t *testing.T) {
 		GHJWTImpl = &GHJWTMock{
 			AppsMock: AppsMock{
 				mockInstallation: &github.Installation{
-					AppSlug: &appSlug,
+					AppSlug: &MockAppSlug,
 				},
 				mockAppResp: &github.Response{Response: &http.Response{StatusCode: http.StatusOK}},
 				mockApp:     &github.App{ExternalURL: &mockExternalUrl},
@@ -389,7 +390,24 @@ func TestWithFullEnvironment(t *testing.T) {
 
 	t.Run("TestHandlePullRequestListCommitsUnsignedCommit", func(t *testing.T) {
 		authors := []string{"john", "doe"}
-		GHImpl = getGHMock(getMockRepositoryCommits(authors, false), nil, nil)
+
+		repositoriesMock := *setupMockRepositoriesService(t, []bool{true, true})
+		repositoriesMock.expectedCtx = []context.Context{context.Background(), context.Background()}
+
+		repositoriesMock.expectedCreateStatusRepoStatus = []*github.RepoStatus{
+			{
+				State:       github.String("pending"),
+				Description: github.String("Paul Botsco, the CLA verifier is running"),
+				Context:     &MockAppSlug,
+			},
+			{
+				State:       github.String("failure"),
+				Description: github.String("One or more commits haven't met our Quality requirements."),
+				Context:     &MockAppSlug,
+			},
+		}
+
+		GHImpl = getGHMock(getMockRepositoryCommits(authors, false), nil, &repositoriesMock)
 		mockDB, logger := setupMockDB(t, false)
 		err := HandlePullRequest(logger, mockDB, webhook.PullRequestPayload{}, 0, "")
 		assert.NoError(t, err)
@@ -412,7 +430,7 @@ func TestHandlePullRequestGetAppError(t *testing.T) {
 	GHJWTImpl = &GHJWTMock{
 		AppsMock: AppsMock{
 			mockInstallation: &github.Installation{
-				AppSlug: &appSlug,
+				AppSlug: &MockAppSlug,
 			},
 			//mockAppErr: forcedError,
 			mockAppResp: &github.Response{Response: &http.Response{StatusCode: http.StatusNotFound}},
@@ -496,7 +514,7 @@ func TestHandlePullRequestListCommitsNoAuthor(t *testing.T) {
 	GHJWTImpl = &GHJWTMock{
 		AppsMock: AppsMock{
 			mockInstallation: &github.Installation{
-				AppSlug: &appSlug,
+				AppSlug: &MockAppSlug,
 			},
 			mockAppResp: &github.Response{Response: &http.Response{StatusCode: http.StatusOK}},
 			mockApp:     &github.App{ExternalURL: &mockExternalUrl},
@@ -630,7 +648,7 @@ func TestReviewPriorPRsEvaluatePRError(t *testing.T) {
 	GHImpl = &GHInterfaceMock{
 		RepositoriesMock: RepositoriesMock{
 			t:                 t,
-			createStatusError: forcedError,
+			createStatusError: []error{forcedError},
 		},
 	}