diff --git a/github/actions_artifacts_test.go b/github/actions_artifacts_test.go index e526e05971..e35a873bc0 100644 --- a/github/actions_artifacts_test.go +++ b/github/actions_artifacts_test.go @@ -25,7 +25,7 @@ func TestActionsService_ListArtifacts(t *testing.T) { testFormValues(t, r, values{"page": "2"}) fmt.Fprint(w, `{ - "total_count":1, + "total_count":1, "artifacts":[{"id":1}] }`, ) @@ -107,7 +107,7 @@ func TestActionsService_ListWorkflowRunArtifacts(t *testing.T) { testFormValues(t, r, values{"page": "2"}) fmt.Fprint(w, `{ - "total_count":1, + "total_count":1, "artifacts":[{"id":1}] }`, ) diff --git a/github/activity_notifications_test.go b/github/activity_notifications_test.go index f46a12f474..fcd1cc3adc 100644 --- a/github/activity_notifications_test.go +++ b/github/activity_notifications_test.go @@ -59,7 +59,7 @@ func TestActivityService_ListNotification(t *testing.T) { }) } -func TestActivityService_ListRepositoryNotification(t *testing.T) { +func TestActivityService_ListRepositoryNotifications(t *testing.T) { client, mux, _, teardown := setup() defer teardown() @@ -81,7 +81,7 @@ func TestActivityService_ListRepositoryNotification(t *testing.T) { const methodName = "ListRepositoryNotifications" testBadOptions(t, methodName, func() (err error) { - _, _, err = client.Activity.ListRepositoryNotifications(ctx, "\n", "\n", nil) + _, _, err = client.Activity.ListRepositoryNotifications(ctx, "\n", "\n", &NotificationListOptions{}) return err }) diff --git a/github/activity_star.go b/github/activity_star.go index ad07aac752..d17ea9c3f2 100644 --- a/github/activity_star.go +++ b/github/activity_star.go @@ -108,6 +108,7 @@ func (s *ActivityService) IsStarred(ctx context.Context, owner, repo string) (bo if err != nil { return false, nil, err } + resp, err := s.client.Do(ctx, req, nil) starred, err := parseBoolResponse(err) return starred, resp, err @@ -122,6 +123,7 @@ func (s *ActivityService) Star(ctx context.Context, owner, repo string) (*Respon if err != nil { return nil, err } + return s.client.Do(ctx, req, nil) } @@ -134,5 +136,6 @@ func (s *ActivityService) Unstar(ctx context.Context, owner, repo string) (*Resp if err != nil { return nil, err } + return s.client.Do(ctx, req, nil) } diff --git a/github/billing.go b/github/billing.go index 8921833bdb..dea4677beb 100644 --- a/github/billing.go +++ b/github/billing.go @@ -79,7 +79,7 @@ func (s *BillingService) GetActionsBillingOrg(ctx context.Context, org string) ( return nil, resp, err } - return actionsOrgBilling, resp, err + return actionsOrgBilling, resp, nil } // GetPackagesBillingOrg returns the free and paid storage used for GitHub Packages in gigabytes for an Org. @@ -98,7 +98,7 @@ func (s *BillingService) GetPackagesBillingOrg(ctx context.Context, org string) return nil, resp, err } - return packagesOrgBilling, resp, err + return packagesOrgBilling, resp, nil } // GetStorageBillingOrg returns the estimated paid and estimated total storage used for GitHub Actions @@ -118,7 +118,7 @@ func (s *BillingService) GetStorageBillingOrg(ctx context.Context, org string) ( return nil, resp, err } - return storageOrgBilling, resp, err + return storageOrgBilling, resp, nil } // GetAdvancedSecurityActiveCommittersOrg returns the GitHub Advanced Security active committers for an organization per repository. @@ -137,7 +137,7 @@ func (s *BillingService) GetAdvancedSecurityActiveCommittersOrg(ctx context.Cont return nil, resp, err } - return activeOrgCommitters, resp, err + return activeOrgCommitters, resp, nil } // GetActionsBillingUser returns the summary of the free and paid GitHub Actions minutes used for a user. @@ -156,7 +156,7 @@ func (s *BillingService) GetActionsBillingUser(ctx context.Context, user string) return nil, resp, err } - return actionsUserBilling, resp, err + return actionsUserBilling, resp, nil } // GetPackagesBillingUser returns the free and paid storage used for GitHub Packages in gigabytes for a user. @@ -175,7 +175,7 @@ func (s *BillingService) GetPackagesBillingUser(ctx context.Context, user string return nil, resp, err } - return packagesUserBilling, resp, err + return packagesUserBilling, resp, nil } // GetStorageBillingUser returns the estimated paid and estimated total storage used for GitHub Actions @@ -195,5 +195,5 @@ func (s *BillingService) GetStorageBillingUser(ctx context.Context, user string) return nil, resp, err } - return storageUserBilling, resp, err + return storageUserBilling, resp, nil } diff --git a/github/gists.go b/github/gists.go index 4971c6bf54..40a4aaf581 100644 --- a/github/gists.go +++ b/github/gists.go @@ -279,6 +279,7 @@ func (s *GistsService) Delete(ctx context.Context, id string) (*Response, error) if err != nil { return nil, err } + return s.client.Do(ctx, req, nil) } @@ -291,6 +292,7 @@ func (s *GistsService) Star(ctx context.Context, id string) (*Response, error) { if err != nil { return nil, err } + return s.client.Do(ctx, req, nil) } @@ -303,6 +305,7 @@ func (s *GistsService) Unstar(ctx context.Context, id string) (*Response, error) if err != nil { return nil, err } + return s.client.Do(ctx, req, nil) } @@ -315,6 +318,7 @@ func (s *GistsService) IsStarred(ctx context.Context, id string) (bool, *Respons if err != nil { return false, nil, err } + resp, err := s.client.Do(ctx, req, nil) starred, err := parseBoolResponse(err) return starred, resp, err diff --git a/github/git_blobs.go b/github/git_blobs.go index 6bc59c6f88..7cbd411ca7 100644 --- a/github/git_blobs.go +++ b/github/git_blobs.go @@ -33,7 +33,11 @@ func (s *GitService) GetBlob(ctx context.Context, owner string, repo string, sha blob := new(Blob) resp, err := s.client.Do(ctx, req, blob) - return blob, resp, err + if err != nil { + return nil, resp, err + } + + return blob, resp, nil } // GetBlobRaw fetches a blob's contents from a repo. @@ -46,11 +50,16 @@ func (s *GitService) GetBlobRaw(ctx context.Context, owner, repo, sha string) ([ if err != nil { return nil, nil, err } + req.Header.Set("Accept", "application/vnd.github.v3.raw") var buf bytes.Buffer resp, err := s.client.Do(ctx, req, &buf) - return buf.Bytes(), resp, err + if err != nil { + return nil, resp, err + } + + return buf.Bytes(), resp, nil } // CreateBlob creates a blob object. @@ -65,5 +74,9 @@ func (s *GitService) CreateBlob(ctx context.Context, owner string, repo string, t := new(Blob) resp, err := s.client.Do(ctx, req, t) - return t, resp, err + if err != nil { + return nil, resp, err + } + + return t, resp, nil } diff --git a/github/git_blobs_test.go b/github/git_blobs_test.go index f2e2457b7c..c2cc455ab8 100644 --- a/github/git_blobs_test.go +++ b/github/git_blobs_test.go @@ -49,6 +49,14 @@ func TestGitService_GetBlob(t *testing.T) { _, _, err = client.Git.GetBlob(ctx, "\n", "\n", "\n") return err }) + + testNewRequestAndDoFailure(t, methodName, client, func() (*Response, error) { + got, resp, err := client.Git.GetBlob(ctx, "o", "r", "s") + if got != nil { + t.Errorf("testNewRequestAndDoFailure %v = %#v, want nil", methodName, got) + } + return resp, err + }) } func TestGitService_GetBlob_invalidOwner(t *testing.T) { @@ -144,6 +152,14 @@ func TestGitService_CreateBlob(t *testing.T) { _, _, err = client.Git.CreateBlob(ctx, "\n", "\n", input) return err }) + + testNewRequestAndDoFailure(t, methodName, client, func() (*Response, error) { + got, resp, err := client.Git.CreateBlob(ctx, "o", "r", input) + if got != nil { + t.Errorf("testNewRequestAndDoFailure %v = %#v, want nil", methodName, got) + } + return resp, err + }) } func TestGitService_CreateBlob_invalidOwner(t *testing.T) { diff --git a/github/git_commits_test.go b/github/git_commits_test.go index c73ff09a01..cc9cc3fcbd 100644 --- a/github/git_commits_test.go +++ b/github/git_commits_test.go @@ -285,7 +285,18 @@ func TestGitService_CreateSignedCommitWithInvalidParams(t *testing.T) { ctx := context.Background() _, _, err := client.Git.CreateCommit(ctx, "o", "r", input) if err == nil { - t.Errorf("Expected error to be returned because invalid params was passed") + t.Errorf("Expected error to be returned because invalid params were passed") + } +} + +func TestGitService_CreateSignedCommitWithNilCommit(t *testing.T) { + client, _, _, teardown := setup() + defer teardown() + + ctx := context.Background() + _, _, err := client.Git.CreateCommit(ctx, "o", "r", nil) + if err == nil { + t.Errorf("Expected error to be returned because commit=nil") } } diff --git a/github/git_tags.go b/github/git_tags.go index 10029c4549..12cfc1b3ef 100644 --- a/github/git_tags.go +++ b/github/git_tags.go @@ -45,7 +45,11 @@ func (s *GitService) GetTag(ctx context.Context, owner string, repo string, sha tag := new(Tag) resp, err := s.client.Do(ctx, req, tag) - return tag, resp, err + if err != nil { + return nil, resp, err + } + + return tag, resp, nil } // CreateTag creates a tag object. @@ -72,5 +76,9 @@ func (s *GitService) CreateTag(ctx context.Context, owner string, repo string, t t := new(Tag) resp, err := s.client.Do(ctx, req, t) - return t, resp, err + if err != nil { + return nil, resp, err + } + + return t, resp, nil } diff --git a/github/git_tags_test.go b/github/git_tags_test.go index 3962f49118..2d13722aba 100644 --- a/github/git_tags_test.go +++ b/github/git_tags_test.go @@ -40,6 +40,14 @@ func TestGitService_GetTag(t *testing.T) { _, _, err = client.Git.GetTag(ctx, "\n", "\n", "\n") return err }) + + testNewRequestAndDoFailure(t, methodName, client, func() (*Response, error) { + got, resp, err := client.Git.GetTag(ctx, "o", "r", "s") + if got != nil { + t.Errorf("testNewRequestAndDoFailure %v = %#v, want nil", methodName, got) + } + return resp, err + }) } func TestGitService_CreateTag(t *testing.T) { @@ -61,10 +69,11 @@ func TestGitService_CreateTag(t *testing.T) { }) ctx := context.Background() - tag, _, err := client.Git.CreateTag(ctx, "o", "r", &Tag{ + inputTag := &Tag{ Tag: input.Tag, Object: &GitObject{SHA: input.Object}, - }) + } + tag, _, err := client.Git.CreateTag(ctx, "o", "r", inputTag) if err != nil { t.Errorf("Git.CreateTag returned error: %v", err) } @@ -76,12 +85,17 @@ func TestGitService_CreateTag(t *testing.T) { const methodName = "CreateTag" testBadOptions(t, methodName, func() (err error) { - _, _, err = client.Git.CreateTag(ctx, "\n", "\n", &Tag{ - Tag: input.Tag, - Object: &GitObject{SHA: input.Object}, - }) + _, _, err = client.Git.CreateTag(ctx, "\n", "\n", inputTag) return err }) + + testNewRequestAndDoFailure(t, methodName, client, func() (*Response, error) { + got, resp, err := client.Git.CreateTag(ctx, "o", "r", inputTag) + if got != nil { + t.Errorf("testNewRequestAndDoFailure %v = %#v, want nil", methodName, got) + } + return resp, err + }) } func TestTag_Marshal(t *testing.T) { diff --git a/github/github.go b/github/github.go index be46c31028..080a8794dd 100644 --- a/github/github.go +++ b/github/github.go @@ -353,6 +353,7 @@ func NewEnterpriseClient(baseURL, uploadURL string, httpClient *http.Client) (*C if err != nil { return nil, err } + if !strings.HasSuffix(baseEndpoint.Path, "/") { baseEndpoint.Path += "/" } @@ -366,6 +367,7 @@ func NewEnterpriseClient(baseURL, uploadURL string, httpClient *http.Client) (*C if err != nil { return nil, err } + if !strings.HasSuffix(uploadEndpoint.Path, "/") { uploadEndpoint.Path += "/" } @@ -390,6 +392,7 @@ func (c *Client) NewRequest(method, urlStr string, body interface{}) (*http.Requ if !strings.HasSuffix(c.BaseURL.Path, "/") { return nil, fmt.Errorf("BaseURL must have a trailing slash, but %q does not", c.BaseURL) } + u, err := c.BaseURL.Parse(urlStr) if err != nil { return nil, err @@ -437,6 +440,7 @@ func (c *Client) NewUploadRequest(urlStr string, reader io.Reader, size int64, m if err != nil { return nil, err } + req.ContentLength = size if mediaType == "" { @@ -621,6 +625,7 @@ func (c *Client) BareDo(ctx context.Context, req *http.Request) (*Response, erro if ctx == nil { return nil, errNonNilContext } + req = withContext(ctx, req) rateLimitCategory := category(req.URL.Path) @@ -996,6 +1001,7 @@ func CheckResponse(r *http.Response) error { if c := r.StatusCode; 200 <= c && c <= 299 { return nil } + errorResponse := &ErrorResponse{Response: r} data, err := ioutil.ReadAll(r.Body) if err == nil && data != nil { diff --git a/github/github_test.go b/github/github_test.go index aa030d4f44..475eb9d820 100644 --- a/github/github_test.go +++ b/github/github_test.go @@ -594,6 +594,12 @@ func TestNewUploadRequest_badURL(t *testing.T) { c := NewClient(nil) _, err := c.NewUploadRequest(":", nil, 0, "") testURLParseError(t, err) + + const methodName = "NewUploadRequest" + testBadOptions(t, methodName, func() (err error) { + _, err = c.NewUploadRequest("\n", nil, -1, "\n") + return err + }) } func TestNewUploadRequest_errorForNoTrailingSlash(t *testing.T) { diff --git a/github/issue_import_test.go b/github/issue_import_test.go index 82b9f9c2d7..3533b3ab4b 100644 --- a/github/issue_import_test.go +++ b/github/issue_import_test.go @@ -167,6 +167,23 @@ func TestIssueImportService_CheckStatusSince(t *testing.T) { }) } +func TestIssueImportService_CheckStatusSince_badResponse(t *testing.T) { + client, mux, _, teardown := setup() + defer teardown() + + mux.HandleFunc("/repos/o/r/import/issues", func(w http.ResponseWriter, r *http.Request) { + testMethod(t, r, "GET") + testHeader(t, r, "Accept", mediaTypeIssueImportAPI) + w.WriteHeader(http.StatusOK) + w.Write([]byte("{badly-formed JSON")) + }) + + ctx := context.Background() + if _, _, err := client.IssueImport.CheckStatusSince(ctx, "o", "r", time.Now()); err == nil { + t.Errorf("CheckStatusSince returned no error, want JSON err") + } +} + func TestIssueImportService_CheckStatusSince_invalidOwner(t *testing.T) { client, _, _, teardown := setup() defer teardown() diff --git a/github/issues_assignees.go b/github/issues_assignees.go index 9f15aea43f..34e0bb506f 100644 --- a/github/issues_assignees.go +++ b/github/issues_assignees.go @@ -25,6 +25,7 @@ func (s *IssuesService) ListAssignees(ctx context.Context, owner, repo string, o if err != nil { return nil, nil, err } + var assignees []*User resp, err := s.client.Do(ctx, req, &assignees) if err != nil { @@ -43,6 +44,7 @@ func (s *IssuesService) IsAssignee(ctx context.Context, owner, repo, user string if err != nil { return false, nil, err } + resp, err := s.client.Do(ctx, req, nil) assignee, err := parseBoolResponse(err) return assignee, resp, err @@ -63,7 +65,11 @@ func (s *IssuesService) AddAssignees(ctx context.Context, owner, repo string, nu issue := &Issue{} resp, err := s.client.Do(ctx, req, issue) - return issue, resp, err + if err != nil { + return nil, resp, err + } + + return issue, resp, nil } // RemoveAssignees removes the provided GitHub users as assignees from the issue. @@ -81,5 +87,9 @@ func (s *IssuesService) RemoveAssignees(ctx context.Context, owner, repo string, issue := &Issue{} resp, err := s.client.Do(ctx, req, issue) - return issue, resp, err + if err != nil { + return nil, resp, err + } + + return issue, resp, nil } diff --git a/github/issues_assignees_test.go b/github/issues_assignees_test.go index ced7bfcd2c..26773b1154 100644 --- a/github/issues_assignees_test.go +++ b/github/issues_assignees_test.go @@ -202,6 +202,14 @@ func TestIssuesService_AddAssignees(t *testing.T) { _, _, err = client.Issues.AddAssignees(ctx, "\n", "\n", -1, []string{"\n", "\n"}) return err }) + + testNewRequestAndDoFailure(t, methodName, client, func() (*Response, error) { + got, resp, err := client.Issues.AddAssignees(ctx, "o", "r", 1, []string{"user1", "user2"}) + if got != nil { + t.Errorf("testNewRequestAndDoFailure %v = %#v, want nil", methodName, got) + } + return resp, err + }) } func TestIssuesService_RemoveAssignees(t *testing.T) { @@ -238,4 +246,12 @@ func TestIssuesService_RemoveAssignees(t *testing.T) { _, _, err = client.Issues.RemoveAssignees(ctx, "\n", "\n", -1, []string{"\n", "\n"}) return err }) + + testNewRequestAndDoFailure(t, methodName, client, func() (*Response, error) { + got, resp, err := client.Issues.RemoveAssignees(ctx, "o", "r", 1, []string{"user1", "user2"}) + if got != nil { + t.Errorf("testNewRequestAndDoFailure %v = %#v, want nil", methodName, got) + } + return resp, err + }) } diff --git a/github/issues_test.go b/github/issues_test.go index 3ed7390f5f..e2cf063914 100644 --- a/github/issues_test.go +++ b/github/issues_test.go @@ -129,6 +129,15 @@ func TestIssuesService_ListByOrg_invalidOrg(t *testing.T) { testURLParseError(t, err) } +func TestIssuesService_ListByOrg_badOrg(t *testing.T) { + client, _, _, teardown := setup() + defer teardown() + + ctx := context.Background() + _, _, err := client.Issues.ListByOrg(ctx, "\n", nil) + testURLParseError(t, err) +} + func TestIssuesService_ListByRepo(t *testing.T) { client, mux, _, teardown := setup() defer teardown() diff --git a/github/issues_timeline.go b/github/issues_timeline.go index 845e3f767c..d0ee0a78f2 100644 --- a/github/issues_timeline.go +++ b/github/issues_timeline.go @@ -180,5 +180,9 @@ func (s *IssuesService) ListIssueTimeline(ctx context.Context, owner, repo strin var events []*Timeline resp, err := s.client.Do(ctx, req, &events) - return events, resp, err + if err != nil { + return nil, resp, err + } + + return events, resp, nil } diff --git a/github/orgs_actions_allowed.go b/github/orgs_actions_allowed.go index 687ab2f667..49bcb00f94 100644 --- a/github/orgs_actions_allowed.go +++ b/github/orgs_actions_allowed.go @@ -52,7 +52,12 @@ func (s *OrganizationsService) EditActionsAllowed(ctx context.Context, org strin if err != nil { return nil, nil, err } + p := new(ActionsAllowed) resp, err := s.client.Do(ctx, req, p) - return p, resp, err + if err != nil { + return nil, resp, err + } + + return p, resp, nil } diff --git a/github/orgs_actions_allowed_test.go b/github/orgs_actions_allowed_test.go index 0525b1defb..6af0521352 100644 --- a/github/orgs_actions_allowed_test.go +++ b/github/orgs_actions_allowed_test.go @@ -82,6 +82,14 @@ func TestOrganizationsService_EditActionsAllowed(t *testing.T) { _, _, err = client.Organizations.EditActionsAllowed(ctx, "\n", *input) return err }) + + testNewRequestAndDoFailure(t, methodName, client, func() (*Response, error) { + got, resp, err := client.Organizations.EditActionsAllowed(ctx, "o", *input) + if got != nil { + t.Errorf("testNewRequestAndDoFailure %v = %#v, want nil", methodName, got) + } + return resp, err + }) } func TestActionsAllowed_Marshal(t *testing.T) { diff --git a/github/orgs_actions_permissions.go b/github/orgs_actions_permissions.go index b8a10b2520..fc1fe31361 100644 --- a/github/orgs_actions_permissions.go +++ b/github/orgs_actions_permissions.go @@ -52,7 +52,12 @@ func (s *OrganizationsService) EditActionsPermissions(ctx context.Context, org s if err != nil { return nil, nil, err } + p := new(ActionsPermissions) resp, err := s.client.Do(ctx, req, p) - return p, resp, err + if err != nil { + return nil, resp, err + } + + return p, resp, nil } diff --git a/github/orgs_actions_permissions_test.go b/github/orgs_actions_permissions_test.go index eff0d4c4ff..9664c79eda 100644 --- a/github/orgs_actions_permissions_test.go +++ b/github/orgs_actions_permissions_test.go @@ -83,4 +83,12 @@ func TestOrganizationsService_EditActionsPermissions(t *testing.T) { _, _, err = client.Organizations.EditActionsPermissions(ctx, "\n", *input) return err }) + + testNewRequestAndDoFailure(t, methodName, client, func() (*Response, error) { + got, resp, err := client.Organizations.EditActionsPermissions(ctx, "o", *input) + if got != nil { + t.Errorf("testNewRequestAndDoFailure %v = %#v, want nil", methodName, got) + } + return resp, err + }) } diff --git a/github/orgs_hooks.go b/github/orgs_hooks.go index dc90656878..26687e565b 100644 --- a/github/orgs_hooks.go +++ b/github/orgs_hooks.go @@ -43,9 +43,14 @@ func (s *OrganizationsService) GetHook(ctx context.Context, org string, id int64 if err != nil { return nil, nil, err } + hook := new(Hook) resp, err := s.client.Do(ctx, req, hook) - return hook, resp, err + if err != nil { + return nil, resp, err + } + + return hook, resp, nil } // CreateHook creates a Hook for the specified org. @@ -88,9 +93,14 @@ func (s *OrganizationsService) EditHook(ctx context.Context, org string, id int6 if err != nil { return nil, nil, err } + h := new(Hook) resp, err := s.client.Do(ctx, req, h) - return h, resp, err + if err != nil { + return nil, resp, err + } + + return h, resp, nil } // PingHook triggers a 'ping' event to be sent to the Hook. @@ -102,6 +112,7 @@ func (s *OrganizationsService) PingHook(ctx context.Context, org string, id int6 if err != nil { return nil, err } + return s.client.Do(ctx, req, nil) } @@ -114,5 +125,6 @@ func (s *OrganizationsService) DeleteHook(ctx context.Context, org string, id in if err != nil { return nil, err } + return s.client.Do(ctx, req, nil) } diff --git a/github/orgs_hooks_test.go b/github/orgs_hooks_test.go index be6c7bb8ad..d4d4c54ebb 100644 --- a/github/orgs_hooks_test.go +++ b/github/orgs_hooks_test.go @@ -132,6 +132,14 @@ func TestOrganizationsService_GetHook(t *testing.T) { _, _, err = client.Organizations.GetHook(ctx, "\n", -1) return err }) + + testNewRequestAndDoFailure(t, methodName, client, func() (*Response, error) { + got, resp, err := client.Organizations.GetHook(ctx, "o", 1) + if got != nil { + t.Errorf("testNewRequestAndDoFailure %v = %#v, want nil", methodName, got) + } + return resp, err + }) } func TestOrganizationsService_GetHook_invalidOrg(t *testing.T) { @@ -177,6 +185,14 @@ func TestOrganizationsService_EditHook(t *testing.T) { _, _, err = client.Organizations.EditHook(ctx, "\n", -1, input) return err }) + + testNewRequestAndDoFailure(t, methodName, client, func() (*Response, error) { + got, resp, err := client.Organizations.EditHook(ctx, "o", 1, input) + if got != nil { + t.Errorf("testNewRequestAndDoFailure %v = %#v, want nil", methodName, got) + } + return resp, err + }) } func TestOrganizationsService_EditHook_invalidOrg(t *testing.T) { diff --git a/github/orgs_members.go b/github/orgs_members.go index f3a2f17c08..dd4845441f 100644 --- a/github/orgs_members.go +++ b/github/orgs_members.go @@ -295,6 +295,7 @@ func (s *OrganizationsService) ListPendingOrgInvitations(ctx context.Context, or if err != nil { return nil, resp, err } + return pendingInvitations, resp, nil } @@ -336,6 +337,7 @@ func (s *OrganizationsService) CreateOrgInvitation(ctx context.Context, org stri if err != nil { return nil, resp, err } + return invitation, resp, nil } @@ -360,6 +362,7 @@ func (s *OrganizationsService) ListOrgInvitationTeams(ctx context.Context, org, if err != nil { return nil, resp, err } + return orgInvitationTeams, resp, nil } diff --git a/github/orgs_packages_test.go b/github/orgs_packages_test.go index bbe223b8de..f8c8701d30 100644 --- a/github/orgs_packages_test.go +++ b/github/orgs_packages_test.go @@ -181,7 +181,7 @@ func TestOrganizationsService_DeletePackage(t *testing.T) { const methodName = "DeletePackage" testBadOptions(t, methodName, func() (err error) { - _, _, err = client.Organizations.GetPackage(ctx, "\n", "", "") + _, _, err = client.Organizations.GetPackage(ctx, "\n", "\n", "\n") return err }) diff --git a/github/repos.go b/github/repos.go index 3e8b421787..49a64ee730 100644 --- a/github/repos.go +++ b/github/repos.go @@ -617,7 +617,6 @@ func (s *RepositoriesService) GetVulnerabilityAlerts(ctx context.Context, owner, resp, err := s.client.Do(ctx, req, nil) vulnerabilityAlertsEnabled, err := parseBoolResponse(err) - return vulnerabilityAlertsEnabled, resp, err } @@ -1249,7 +1248,7 @@ func (s *RepositoriesService) RequireSignaturesOnProtectedBranch(ctx context.Con return nil, resp, err } - return r, resp, err + return r, resp, nil } // OptionalSignaturesOnProtectedBranch removes required signed commits on a given branch. @@ -1361,7 +1360,7 @@ func (s *RepositoriesService) UpdatePullRequestReviewEnforcement(ctx context.Con return nil, resp, err } - return r, resp, err + return r, resp, nil } // DisableDismissalRestrictions disables dismissal restrictions of a protected branch. @@ -1389,7 +1388,7 @@ func (s *RepositoriesService) DisableDismissalRestrictions(ctx context.Context, return nil, resp, err } - return r, resp, err + return r, resp, nil } // RemovePullRequestReviewEnforcement removes pull request enforcement of a protected branch. @@ -1441,7 +1440,7 @@ func (s *RepositoriesService) AddAdminEnforcement(ctx context.Context, owner, re return nil, resp, err } - return r, resp, err + return r, resp, nil } // RemoveAdminEnforcement removes admin enforcement from a protected branch. diff --git a/github/repos_collaborators.go b/github/repos_collaborators.go index ccb97a192a..dcb97b5ed2 100644 --- a/github/repos_collaborators.go +++ b/github/repos_collaborators.go @@ -104,6 +104,7 @@ func (s *RepositoriesService) GetPermissionLevel(ctx context.Context, owner, rep if err != nil { return nil, resp, err } + return rpl, resp, nil } @@ -132,11 +133,13 @@ func (s *RepositoriesService) AddCollaborator(ctx context.Context, owner, repo, if err != nil { return nil, nil, err } + acr := new(CollaboratorInvitation) resp, err := s.client.Do(ctx, req, acr) if err != nil { return nil, resp, err } + return acr, resp, nil } @@ -150,5 +153,6 @@ func (s *RepositoriesService) RemoveCollaborator(ctx context.Context, owner, rep if err != nil { return nil, err } + return s.client.Do(ctx, req, nil) } diff --git a/github/repos_contents.go b/github/repos_contents.go index 3fe2710132..3175137250 100644 --- a/github/repos_contents.go +++ b/github/repos_contents.go @@ -102,15 +102,18 @@ func (s *RepositoriesService) GetReadme(ctx context.Context, owner, repo string, if err != nil { return nil, nil, err } + req, err := s.client.NewRequest("GET", u, nil) if err != nil { return nil, nil, err } + readme := new(RepositoryContent) resp, err := s.client.Do(ctx, req, readme) if err != nil { return nil, resp, err } + return readme, resp, nil } @@ -129,18 +132,22 @@ func (s *RepositoriesService) DownloadContents(ctx context.Context, owner, repo, if err != nil { return nil, resp, err } + for _, contents := range dirContents { if *contents.Name == filename { if contents.DownloadURL == nil || *contents.DownloadURL == "" { return nil, resp, fmt.Errorf("no download link found for %s", filepath) } + dlResp, err := s.client.client.Get(*contents.DownloadURL) if err != nil { return nil, &Response{Response: dlResp}, err } + return dlResp.Body, &Response{Response: dlResp}, nil } } + return nil, resp, fmt.Errorf("no file named %s found in %s", filename, dir) } @@ -159,18 +166,22 @@ func (s *RepositoriesService) DownloadContentsWithMeta(ctx context.Context, owne if err != nil { return nil, nil, resp, err } + for _, contents := range dirContents { if *contents.Name == filename { if contents.DownloadURL == nil || *contents.DownloadURL == "" { return nil, contents, resp, fmt.Errorf("no download link found for %s", filepath) } + dlResp, err := s.client.client.Get(*contents.DownloadURL) if err != nil { return nil, contents, &Response{Response: dlResp}, err } + return dlResp.Body, contents, &Response{Response: dlResp}, nil } } + return nil, nil, resp, fmt.Errorf("no file named %s found in %s", filename, dir) } @@ -189,23 +200,28 @@ func (s *RepositoriesService) GetContents(ctx context.Context, owner, repo, path if err != nil { return nil, nil, nil, err } + req, err := s.client.NewRequest("GET", u, nil) if err != nil { return nil, nil, nil, err } + var rawJSON json.RawMessage resp, err = s.client.Do(ctx, req, &rawJSON) if err != nil { return nil, nil, resp, err } + fileUnmarshalError := json.Unmarshal(rawJSON, &fileContent) if fileUnmarshalError == nil { return fileContent, nil, resp, nil } + directoryUnmarshalError := json.Unmarshal(rawJSON, &directoryContent) if directoryUnmarshalError == nil { return nil, directoryContent, resp, nil } + return nil, nil, resp, fmt.Errorf("unmarshalling failed for both file and directory content: %s and %s", fileUnmarshalError, directoryUnmarshalError) } @@ -219,11 +235,13 @@ func (s *RepositoriesService) CreateFile(ctx context.Context, owner, repo, path if err != nil { return nil, nil, err } + createResponse := new(RepositoryContentResponse) resp, err := s.client.Do(ctx, req, createResponse) if err != nil { return nil, resp, err } + return createResponse, resp, nil } @@ -237,11 +255,13 @@ func (s *RepositoriesService) UpdateFile(ctx context.Context, owner, repo, path if err != nil { return nil, nil, err } + updateResponse := new(RepositoryContentResponse) resp, err := s.client.Do(ctx, req, updateResponse) if err != nil { return nil, resp, err } + return updateResponse, resp, nil } @@ -255,11 +275,13 @@ func (s *RepositoriesService) DeleteFile(ctx context.Context, owner, repo, path if err != nil { return nil, nil, err } + deleteResponse := new(RepositoryContentResponse) resp, err := s.client.Do(ctx, req, deleteResponse) if err != nil { return nil, resp, err } + return deleteResponse, resp, nil } diff --git a/github/repos_stats.go b/github/repos_stats.go index 73f0a6768a..756dbc21b6 100644 --- a/github/repos_stats.go +++ b/github/repos_stats.go @@ -121,6 +121,9 @@ func (s *RepositoriesService) ListCodeFrequency(ctx context.Context, owner, repo var weeks [][]int resp, err := s.client.Do(ctx, req, &weeks) + if err != nil { + return nil, resp, err + } // convert int slices into WeeklyStats var stats []*WeeklyStats @@ -136,7 +139,7 @@ func (s *RepositoriesService) ListCodeFrequency(ctx context.Context, owner, repo stats = append(stats, stat) } - return stats, resp, err + return stats, resp, nil } // RepositoryParticipation is the number of commits by everyone @@ -207,6 +210,9 @@ func (s *RepositoriesService) ListPunchCard(ctx context.Context, owner, repo str var results [][]int resp, err := s.client.Do(ctx, req, &results) + if err != nil { + return nil, resp, err + } // convert int slices into Punchcards var cards []*PunchCard @@ -222,5 +228,5 @@ func (s *RepositoriesService) ListPunchCard(ctx context.Context, owner, repo str cards = append(cards, card) } - return cards, resp, err + return cards, resp, nil } diff --git a/github/repos_test.go b/github/repos_test.go index 5739c10b6f..cbf56a2f7c 100644 --- a/github/repos_test.go +++ b/github/repos_test.go @@ -915,6 +915,21 @@ func TestRepositoriesService_GetBranch(t *testing.T) { }) } +func TestRepositoriesService_GetBranch_BadJSONResponse(t *testing.T) { + client, mux, _, teardown := setup() + defer teardown() + + mux.HandleFunc("/repos/o/r/branches/b", func(w http.ResponseWriter, r *http.Request) { + testMethod(t, r, "GET") + fmt.Fprint(w, `{"name":"n", "commit":{"sha":...truncated`) + }) + + ctx := context.Background() + if _, _, err := client.Repositories.GetBranch(ctx, "o", "r", "b", false); err == nil { + t.Error("Repositories.GetBranch returned no error; wanted JSON error") + } +} + func TestRepositoriesService_GetBranch_StatusMovedPermanently_followRedirects(t *testing.T) { client, mux, serverURL, teardown := setup() defer teardown() @@ -976,7 +991,7 @@ func TestRepositoriesService_GetBranch_notFound(t *testing.T) { const methodName = "GetBranch" testBadOptions(t, methodName, func() (err error) { - _, _, err = client.Repositories.GetBranch(ctx, "o", "r", "b", true) + _, _, err = client.Repositories.GetBranch(ctx, "\n", "\n", "\n", true) return err }) } diff --git a/github/search.go b/github/search.go index 19aa892798..a3da304c51 100644 --- a/github/search.go +++ b/github/search.go @@ -73,7 +73,11 @@ type RepositoriesSearchResult struct { func (s *SearchService) Repositories(ctx context.Context, query string, opts *SearchOptions) (*RepositoriesSearchResult, *Response, error) { result := new(RepositoriesSearchResult) resp, err := s.search(ctx, "repositories", &searchParameters{Query: query}, opts, result) - return result, resp, err + if err != nil { + return nil, resp, err + } + + return result, resp, nil } // TopicsSearchResult represents the result of a topics search. @@ -104,7 +108,11 @@ type TopicResult struct { func (s *SearchService) Topics(ctx context.Context, query string, opts *SearchOptions) (*TopicsSearchResult, *Response, error) { result := new(TopicsSearchResult) resp, err := s.search(ctx, "topics", &searchParameters{Query: query}, opts, result) - return result, resp, err + if err != nil { + return nil, resp, err + } + + return result, resp, nil } // CommitsSearchResult represents the result of a commits search. @@ -135,7 +143,11 @@ type CommitResult struct { func (s *SearchService) Commits(ctx context.Context, query string, opts *SearchOptions) (*CommitsSearchResult, *Response, error) { result := new(CommitsSearchResult) resp, err := s.search(ctx, "commits", &searchParameters{Query: query}, opts, result) - return result, resp, err + if err != nil { + return nil, resp, err + } + + return result, resp, nil } // IssuesSearchResult represents the result of an issues search. @@ -151,7 +163,11 @@ type IssuesSearchResult struct { func (s *SearchService) Issues(ctx context.Context, query string, opts *SearchOptions) (*IssuesSearchResult, *Response, error) { result := new(IssuesSearchResult) resp, err := s.search(ctx, "issues", &searchParameters{Query: query}, opts, result) - return result, resp, err + if err != nil { + return nil, resp, err + } + + return result, resp, nil } // UsersSearchResult represents the result of a users search. @@ -167,7 +183,11 @@ type UsersSearchResult struct { func (s *SearchService) Users(ctx context.Context, query string, opts *SearchOptions) (*UsersSearchResult, *Response, error) { result := new(UsersSearchResult) resp, err := s.search(ctx, "users", &searchParameters{Query: query}, opts, result) - return result, resp, err + if err != nil { + return nil, resp, err + } + + return result, resp, nil } // Match represents a single text match. @@ -216,7 +236,11 @@ func (c CodeResult) String() string { func (s *SearchService) Code(ctx context.Context, query string, opts *SearchOptions) (*CodeSearchResult, *Response, error) { result := new(CodeSearchResult) resp, err := s.search(ctx, "code", &searchParameters{Query: query}, opts, result) - return result, resp, err + if err != nil { + return nil, resp, err + } + + return result, resp, nil } // LabelsSearchResult represents the result of a code search. @@ -247,7 +271,11 @@ func (l LabelResult) String() string { func (s *SearchService) Labels(ctx context.Context, repoID int64, query string, opts *SearchOptions) (*LabelsSearchResult, *Response, error) { result := new(LabelsSearchResult) resp, err := s.search(ctx, "labels", &searchParameters{RepositoryID: &repoID, Query: query}, opts, result) - return result, resp, err + if err != nil { + return nil, resp, err + } + + return result, resp, nil } // Helper function that executes search queries against different @@ -260,6 +288,7 @@ func (s *SearchService) search(ctx context.Context, searchType string, parameter if err != nil { return nil, err } + if parameters.RepositoryID != nil { params.Set("repository_id", strconv.FormatInt(*parameters.RepositoryID, 10)) } diff --git a/github/teams.go b/github/teams.go index 82d4093b47..5b1096ea29 100644 --- a/github/teams.go +++ b/github/teams.go @@ -771,6 +771,7 @@ func (s *TeamsService) ListIDPGroupsInOrganization(ctx context.Context, org stri if err != nil { return nil, resp, err } + return groups, resp, nil } @@ -791,7 +792,8 @@ func (s *TeamsService) ListIDPGroupsForTeamByID(ctx context.Context, orgID, team if err != nil { return nil, resp, err } - return groups, resp, err + + return groups, resp, nil } // ListIDPGroupsForTeamBySlug lists IDP groups connected to a team on GitHub @@ -811,7 +813,8 @@ func (s *TeamsService) ListIDPGroupsForTeamBySlug(ctx context.Context, org, slug if err != nil { return nil, resp, err } - return groups, resp, err + + return groups, resp, nil } // CreateOrUpdateIDPGroupConnectionsByID creates, updates, or removes a connection