From 0daa71a5971af7409e297c92f68a3be98b735216 Mon Sep 17 00:00:00 2001 From: Hemachandar Date: Mon, 13 Sep 2021 10:08:55 +0530 Subject: [PATCH 1/2] Handle protection status errors for unprotected branches --- github/repos.go | 23 +++++++++++++ github/repos_test.go | 78 ++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 101 insertions(+) diff --git a/github/repos.go b/github/repos.go index aab18d4d459..c1d2727fa45 100644 --- a/github/repos.go +++ b/github/repos.go @@ -8,11 +8,16 @@ package github import ( "context" "encoding/json" + "errors" "fmt" "net/http" "strings" ) +const githubBranchNotProtected string = "Branch not protected" + +var ErrBranchNotProtected = errors.New("branch is not protected") + // RepositoriesService handles communication with the repository related // methods of the GitHub API. // @@ -1009,6 +1014,9 @@ func (s *RepositoriesService) GetBranchProtection(ctx context.Context, owner, re p := new(Protection) resp, err := s.client.Do(ctx, req, p) if err != nil { + if isBranchNotProtected(err) { + err = ErrBranchNotProtected + } return nil, resp, err } @@ -1028,6 +1036,9 @@ func (s *RepositoriesService) GetRequiredStatusChecks(ctx context.Context, owner p := new(RequiredStatusChecks) resp, err := s.client.Do(ctx, req, p) if err != nil { + if isBranchNotProtected(err) { + err = ErrBranchNotProtected + } return nil, resp, err } @@ -1046,6 +1057,9 @@ func (s *RepositoriesService) ListRequiredStatusChecksContexts(ctx context.Conte resp, err = s.client.Do(ctx, req, &contexts) if err != nil { + if isBranchNotProtected(err) { + err = ErrBranchNotProtected + } return nil, resp, err } @@ -1539,3 +1553,12 @@ func (s *RepositoriesService) Dispatch(ctx context.Context, owner, repo string, return r, resp, nil } + +// isBranchNotProtected determines whether a branch is not protected +// based on the error message returned by GitHub API. +func isBranchNotProtected(err error) bool { + if errorResponse, ok := err.(*ErrorResponse); ok { + return errorResponse.Message == githubBranchNotProtected + } + return false +} diff --git a/github/repos_test.go b/github/repos_test.go index 4bb6be22a56..11a23e99c4d 100644 --- a/github/repos_test.go +++ b/github/repos_test.go @@ -1150,6 +1150,32 @@ func TestRepositoriesService_GetBranchProtection_noDismissalRestrictions(t *test } } +func TestRepositoriesService_GetBranchProtection_branchNotProtected(t *testing.T) { + client, mux, _, teardown := setup() + defer teardown() + + mux.HandleFunc("/repos/o/r/branches/b/protection", func(w http.ResponseWriter, r *http.Request) { + testMethod(t, r, "GET") + + w.WriteHeader(http.StatusBadRequest) + fmt.Fprintf(w, `{ + "message": "Branch not protected", + "documentation_url": "https://docs.github.com/rest/reference/repos#get-branch-protection" + }`) + }) + + ctx := context.Background() + protection, _, err := client.Repositories.GetBranchProtection(ctx, "o", "r", "b") + + if protection != nil { + t.Errorf("Repositories.GetBranchProtection returned non-nil protection data") + } + + if err != ErrBranchNotProtected { + t.Errorf("Repositories.GetBranchProtection returned an invalid error: %v", err) + } +} + func TestRepositoriesService_UpdateBranchProtection(t *testing.T) { client, mux, _, teardown := setup() defer teardown() @@ -1387,6 +1413,32 @@ func TestRepositoriesService_GetRequiredStatusChecks(t *testing.T) { }) } +func TestRepositoriesService_GetRequiredStatusChecks_branchNotProtected(t *testing.T) { + client, mux, _, teardown := setup() + defer teardown() + + mux.HandleFunc("/repos/o/r/branches/b/protection/required_status_checks", func(w http.ResponseWriter, r *http.Request) { + testMethod(t, r, "GET") + + w.WriteHeader(http.StatusBadRequest) + fmt.Fprintf(w, `{ + "message": "Branch not protected", + "documentation_url": "https://docs.github.com/rest/reference/repos#get-branch-protection" + }`) + }) + + ctx := context.Background() + checks, _, err := client.Repositories.GetRequiredStatusChecks(ctx, "o", "r", "b") + + if checks != nil { + t.Errorf("Repositories.GetRequiredStatusChecks returned non-nil status-checks data") + } + + if err != ErrBranchNotProtected { + t.Errorf("Repositories.GetRequiredStatusChecks returned an invalid error: %v", err) + } +} + func TestRepositoriesService_UpdateRequiredStatusChecks(t *testing.T) { client, mux, _, teardown := setup() defer teardown() @@ -1502,6 +1554,32 @@ func TestRepositoriesService_ListRequiredStatusChecksContexts(t *testing.T) { }) } +func TestRepositoriesService_ListRequiredStatusChecksContexts_branchNotProtected(t *testing.T) { + client, mux, _, teardown := setup() + defer teardown() + + mux.HandleFunc("/repos/o/r/branches/b/protection/required_status_checks/contexts", func(w http.ResponseWriter, r *http.Request) { + testMethod(t, r, "GET") + + w.WriteHeader(http.StatusBadRequest) + fmt.Fprintf(w, `{ + "message": "Branch not protected", + "documentation_url": "https://docs.github.com/rest/reference/repos#get-branch-protection" + }`) + }) + + ctx := context.Background() + contexts, _, err := client.Repositories.ListRequiredStatusChecksContexts(ctx, "o", "r", "b") + + if contexts != nil { + t.Errorf("Repositories.ListRequiredStatusChecksContexts returned non-nil contexts data") + } + + if err != ErrBranchNotProtected { + t.Errorf("Repositories.ListRequiredStatusChecksContexts returned an invalid error: %v", err) + } +} + func TestRepositoriesService_GetPullRequestReviewEnforcement(t *testing.T) { client, mux, _, teardown := setup() defer teardown() From 003563544444f3fe9ff0a5a379f4173c03605848 Mon Sep 17 00:00:00 2001 From: Hemachandar Date: Mon, 13 Sep 2021 18:55:02 +0530 Subject: [PATCH 2/2] Address review comments --- github/repos.go | 6 ++---- github/repos_test.go | 12 ++++++------ 2 files changed, 8 insertions(+), 10 deletions(-) diff --git a/github/repos.go b/github/repos.go index c1d2727fa45..8a20361210e 100644 --- a/github/repos.go +++ b/github/repos.go @@ -1557,8 +1557,6 @@ func (s *RepositoriesService) Dispatch(ctx context.Context, owner, repo string, // isBranchNotProtected determines whether a branch is not protected // based on the error message returned by GitHub API. func isBranchNotProtected(err error) bool { - if errorResponse, ok := err.(*ErrorResponse); ok { - return errorResponse.Message == githubBranchNotProtected - } - return false + errorResponse, ok := err.(*ErrorResponse) + return ok && errorResponse.Message == githubBranchNotProtected } diff --git a/github/repos_test.go b/github/repos_test.go index 11a23e99c4d..a452b6a0540 100644 --- a/github/repos_test.go +++ b/github/repos_test.go @@ -1159,9 +1159,9 @@ func TestRepositoriesService_GetBranchProtection_branchNotProtected(t *testing.T w.WriteHeader(http.StatusBadRequest) fmt.Fprintf(w, `{ - "message": "Branch not protected", + "message": %q, "documentation_url": "https://docs.github.com/rest/reference/repos#get-branch-protection" - }`) + }`, githubBranchNotProtected) }) ctx := context.Background() @@ -1422,9 +1422,9 @@ func TestRepositoriesService_GetRequiredStatusChecks_branchNotProtected(t *testi w.WriteHeader(http.StatusBadRequest) fmt.Fprintf(w, `{ - "message": "Branch not protected", + "message": %q, "documentation_url": "https://docs.github.com/rest/reference/repos#get-branch-protection" - }`) + }`, githubBranchNotProtected) }) ctx := context.Background() @@ -1563,9 +1563,9 @@ func TestRepositoriesService_ListRequiredStatusChecksContexts_branchNotProtected w.WriteHeader(http.StatusBadRequest) fmt.Fprintf(w, `{ - "message": "Branch not protected", + "message": %q, "documentation_url": "https://docs.github.com/rest/reference/repos#get-branch-protection" - }`) + }`, githubBranchNotProtected) }) ctx := context.Background()