Rename wontCheck to ignoreError and change most checks to return bools

internal/github/handlers.go

@@ -66,10 +66,11 @@ func (g *GitHub) WebHookHandler(w http.ResponseWriter, r *http.Request) {
 			break
 		}
 		if !installation.IsEnabled() {
-			err = &wontCheck{wontCheckNoInstallation, ""}
+			err = &ignoreEvent{reason: ignoreNoInstallation}
 			break
 		}
-		if err = g.checkPushAffectsGo(e); err != nil {
+		if !g.checkPushAffectsGo(e) {
+			err = &ignoreEvent{reason: ignoreNoGoFiles}
 			break
 		}
 		log.Printf("github: push event: installation id: %v", *e.Installation.ID)
@@ -86,15 +87,16 @@ func (g *GitHub) WebHookHandler(w http.ResponseWriter, r *http.Request) {
 			break
 		}
 		if !installation.IsEnabled() {
-			err = &wontCheck{wontCheckNoInstallation, ""}
+			err = &ignoreEvent{reason: ignoreNoInstallation}
 			break
 		}
 		commits, _, err = installation.client.PullRequests.ListCommits(r.Context(), *e.Repo.Owner.Name, *e.Repo.Name, *e.Number, nil)
 		if err != nil {
 			err = errors.Wrap(err, "could not list commits")
 			break
 		}
-		if err = g.checkCommitsAffectGo(commits); err != nil {
+		if !g.checkCommitsAffectGo(commits) {
+			err = &ignoreEvent{reason: ignoreNoGoFiles}
 			break
 		}
 		log.Printf("github: pull request event: %v, installation id: %v", *e.Action, *e.Installation.ID)
@@ -105,89 +107,90 @@ func (g *GitHub) WebHookHandler(w http.ResponseWriter, r *http.Request) {
 
 	switch err.(type) {
 	case nil:
-	case *wontCheck:
-		log.Println("github: won't check repository:", err)
+	case *ignoreEvent:
+		log.Println("github: will not check repository:", err)
 	default:
 		log.Println("github: event handler error:", err)
 		http.Error(w, err.Error(), http.StatusInternalServerError)
 	}
 }
 
-type wontCheckCode int
+type ignoreReason int
 
 const (
-	wontCheckInvalidAction wontCheckCode = iota
-	wontCheckNoAction
-	wontCheckNoInstallation
-	wontCheckNoGoFiles
+	ignoreInvalidAction ignoreReason = iota
+	ignoreNoAction
+	ignoreNoInstallation
+	ignoreNoGoFiles
 )
 
-// wontCheck indicates the repository won't be checked.
-type wontCheck struct {
-	Code wontCheckCode
-	Msg  string
+// ignoreEvent indicates the event should be accepted but ignored.
+type ignoreEvent struct {
+	reason ignoreReason
+	extra  string
 }
 
 // Error implements the error interface.
-func (e *wontCheck) Error() string {
-	switch e.Code {
-	case wontCheckInvalidAction:
-		return "invalid action: " + e.Msg
-	case wontCheckNoAction:
+func (e *ignoreEvent) Error() string {
+	switch e.reason {
+	case ignoreInvalidAction:
+		return "invalid action: " + e.extra
+	case ignoreNoAction:
 		return "no action"
-	case wontCheckNoInstallation:
+	case ignoreNoInstallation:
 		return "no enabled installation found"
-	case wontCheckNoGoFiles:
+	case ignoreNoGoFiles:
 		return "no go files affected"
 	}
-	return e.Msg
+	return e.extra
 }
 
-// checkPRAction returns true if pull request's action is acceptable.
+// checkPRAction checks a pull request's action to determine whether the event
+// should continue to be processed. Returns error type *ignoreEvent if the event
+// should be ignored, nil if it should be processed, or other error if check
+// could not be completed.
 func (g *GitHub) checkPRAction(e *github.PullRequestEvent) error {
 	if e.Action == nil {
-		return &wontCheck{wontCheckNoAction, ""}
+		return &ignoreEvent{reason: ignoreNoAction}
 	}
 	if *e.Action != "opened" && *e.Action != "synchronize" && *e.Action != "reopened" {
-		return &wontCheck{wontCheckInvalidAction, *e.Action}
+		return &ignoreEvent{reason: ignoreInvalidAction, extra: *e.Action}
 	}
 	return nil
 }
 
-// checkCommitsAffectGo checks a pull request to see if it modified, added or removed Go files.
-func (g *GitHub) checkCommitsAffectGo(commits []*github.RepositoryCommit) error {
+// checkCommitsAffectGo returns true if any of the commits modify, add or remove Go files.
+func (g *GitHub) checkCommitsAffectGo(commits []*github.RepositoryCommit) bool {
 	for _, commit := range commits {
 		for _, file := range commit.Files {
-			if isGoFile(*file.Filename) {
-				return nil
+			if hasGoExtension(*file.Filename) {
+				return true
 			}
 		}
 	}
-	return &wontCheck{wontCheckNoGoFiles, ""}
+	return false
 }
 
-// checkPushAffectsGo checks a GitHub Push Event to determine if it should be checked.
-// Returns nil if it should be checked, or error type wontCheck if it should
-// not be checked, or other error types in case of error.
-func (g *GitHub) checkPushAffectsGo(e *github.PushEvent) error {
+// checkPushAffectsGo returns true if the event modifies, adds or removes Go files.
+func (g *GitHub) checkPushAffectsGo(event *github.PushEvent) bool {
 	hasGoFile := func(files []string) bool {
 		for _, filename := range files {
-			if isGoFile(filename) {
+			if hasGoExtension(filename) {
 				return true
 			}
 		}
 		return false
 	}
-	for _, commit := range e.Commits {
+	for _, commit := range event.Commits {
 		if hasGoFile(commit.Modified) || hasGoFile(commit.Added) || hasGoFile(commit.Removed) {
-			return nil
+			return true
 		}
 	}
-	return &wontCheck{wontCheckNoGoFiles, ""}
+	return false
 }
 
-// isGoFile returns true if the filename has the
-func isGoFile(filename string) bool {
+// hasGoExtension returns true if the filename has the suffix ".go".
+func hasGoExtension(filename string) bool {
 	return strings.HasSuffix(filename, ".go")
 }
 

internal/github/handlers_test.go

@@ -333,8 +333,8 @@ func TestCheckPRAction(t *testing.T) {
 		action *string
 		want   error
 	}{
-		{nil, &wontCheck{}},
-		{github.String("invalid"), &wontCheck{}},
+		{nil, &ignoreEvent{}},
+		{github.String("invalid"), &ignoreEvent{}},
 		{github.String("opened"), nil},
 		{github.String("synchronize"), nil},
 		{github.String("reopened"), nil},
@@ -352,13 +352,13 @@ func TestCheckPRAction(t *testing.T) {
 func TestCheckPushAffectsGo(t *testing.T) {
 	tests := []struct {
 		commits github.PushEventCommit
-		want    error
+		want    bool
 	}{
-		{github.PushEventCommit{}, &wontCheck{}},
-		{github.PushEventCommit{Added: []string{"main.php"}}, &wontCheck{}},
-		{github.PushEventCommit{Added: []string{"main.go"}}, nil},
-		{github.PushEventCommit{Removed: []string{"main.go"}}, nil},
-		{github.PushEventCommit{Modified: []string{"main.go"}}, nil},
+		{github.PushEventCommit{}, false},
+		{github.PushEventCommit{Added: []string{"main.php"}}, false},
+		{github.PushEventCommit{Added: []string{"main.go"}}, true},
+		{github.PushEventCommit{Removed: []string{"main.go"}}, true},
+		{github.PushEventCommit{Modified: []string{"main.go"}}, true},
 	}
 
 	for _, test := range tests {
@@ -367,19 +367,19 @@ func TestCheckPushAffectsGo(t *testing.T) {
 		}
 		g, _, _ := setup(t)
 		have := g.checkPushAffectsGo(e)
-		if reflect.TypeOf(have) != reflect.TypeOf(test.want) {
-			t.Errorf("wrong error type, have: %T, want: %T", have, test.want)
+		if have != test.want {
+			t.Errorf("have: %v, want: %v", have, test.want)
 		}
 	}
 }
 
 func TestCheckCommitsAffectGo(t *testing.T) {
 	tests := []struct {
 		file github.CommitFile
-		want error
+		want bool
 	}{
-		{github.CommitFile{Filename: github.String("main.php")}, &wontCheck{}},
-		{github.CommitFile{Filename: github.String("main.go")}, nil},
+		{github.CommitFile{Filename: github.String("main.php")}, false},
+		{github.CommitFile{Filename: github.String("main.go")}, true},
 	}
 
 	for _, test := range tests {
@@ -388,8 +388,8 @@ func TestCheckCommitsAffectGo(t *testing.T) {
 		}
 		g, _, _ := setup(t)
 		have := g.checkCommitsAffectGo([]*github.RepositoryCommit{e})
-		if reflect.TypeOf(have) != reflect.TypeOf(test.want) {
-			t.Errorf("wrong error type, have: %T, want: %T", have, test.want)
+		if have != test.want {
+			t.Errorf("have: %v, want: %v", have, test.want)
 		}
 	}