Repository: google/go-github
Pull request #1229 (2019-07-11 08:48:46)

addOptions to list gists forks endpoint
Added test cases to increase coverage

codecov[bot]: # [Codecov](https://codecov.io/gh/google/go-github/pull/1229?src=pr&el=h1) Report
> Merging [#1229](https://codecov.io/gh/google/go-github/pull/1229?src=pr&el=desc) into [master](https://codecov.io/gh/google/go-github/commit/cad32f26bbd0216620995d028657c304c95a5eab?src=pr&el=desc) will **increase** coverage by `0.07%`.
> The diff coverage is `100%`.

[![Impacted file tree graph](https://codecov.io/gh/google/go-github/pull/1229/graphs/tree.svg?width=650&token=EgYjGjZlI0&height=150&src=pr)](https://codecov.io/gh/google/go-github/pull/1229?src=pr&el=tree)

```diff
@@            Coverage Diff             @@
##           master    #1229      +/-   ##
==========================================
+ Coverage   73.28%   73.36%   +0.07%     
==========================================
  Files          86       86              
  Lines        6023     6026       +3     
==========================================
+ Hits         4414     4421       +7     
+ Misses        838      836       -2     
+ Partials      771      769       -2
```


| [Impacted Files](https://codecov.io/gh/google/go-github/pull/1229?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [github/gists.go](https://codecov.io/gh/google/go-github/pull/1229/diff?src=pr&el=tree#diff-Z2l0aHViL2dpc3RzLmdv) | `77.48% <100%> (+3.15%)` | :arrow_up: |

------

[Continue to review full report at Codecov](https://codecov.io/gh/google/go-github/pull/1229?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/google/go-github/pull/1229?src=pr&el=footer). Last update [cad32f2...5c17575](https://codecov.io/gh/google/go-github/pull/1229?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).

gmlewis: Thank you, @srivignessh. I have a pro-tip for you. Instead of opening and closing multiple PRs until you get all the details perfect (thus artificially bumping up this repo's overall PR count), you can leave the one PR open and commit more changes to it and the tools will rerun tests after each additional commit push.

That has the added benefit that it will reduce all the emails being sent to the (currently) 177 people who are watching this repo.  Thanks!

Are you ready to have this PR reviewed?
srivignessh: Thank you, @gmlewis 

I will follow your advice and update the existing PRs with updated commits. 
I apologize for the opening and closing multiple PRs
.  
Yes, I am ready to get the PR reviewed.

gmlewis: Thank you, @gauntface!
Merging, then bumping major version.

Patch:
diff --git a/github/gists.go b/github/gists.go
index 15e0bc2c..36d93619 100644
--- a/github/gists.go
+++ b/github/gists.go
@@ -341,8 +341,13 @@ func (s *GistsService) Fork(ctx context.Context, id string) (*Gist, *Response, e
 // ListForks lists forks of a gist.
 //
 // GitHub API docs: https://developer.github.com/v3/gists/#list-gist-forks
-func (s *GistsService) ListForks(ctx context.Context, id string) ([]*GistFork, *Response, error) {
+func (s *GistsService) ListForks(ctx context.Context, id string, opt *ListOptions) ([]*GistFork, *Response, error) {
 	u := fmt.Sprintf("gists/%v/forks", id)
+	u, err := addOptions(u, opt)
+	if err != nil {
+		return nil, nil, err
+	}
+
 	req, err := s.client.NewRequest("GET", u, nil)
 	if err != nil {
 		return nil, nil, err


Test patch:
diff --git a/github/gists_test.go b/github/gists_test.go
index 2d79b8c5..d24e0e40 100644
--- a/github/gists_test.go
+++ b/github/gists_test.go
@@ -568,6 +568,14 @@ func TestGistsService_ListCommits_withOptions(t *testing.T) {
 	}
 }
 
+func TestGistsService_ListCommits_invalidID(t *testing.T) {
+	client, _, _, teardown := setup()
+	defer teardown()
+
+	_, _, err := client.Gists.ListCommits(context.Background(), "%", nil)
+	testURLParseError(t, err)
+}
+
 func TestGistsService_Delete(t *testing.T) {
 	client, mux, _, teardown := setup()
 	defer teardown()
@@ -717,7 +725,7 @@ func TestGistsService_ListForks(t *testing.T) {
 		`)
 	})
 
-	gistForks, _, err := client.Gists.ListForks(context.Background(), "1")
+	gistForks, _, err := client.Gists.ListForks(context.Background(), "1", nil)
 	if err != nil {
 		t.Errorf("Gists.ListForks returned error: %v", err)
 	}
@@ -732,12 +740,59 @@ func TestGistsService_ListForks(t *testing.T) {
 	if !reflect.DeepEqual(gistForks, want) {
 		t.Errorf("Gists.ListForks returned %+v, want %+v", gistForks, want)
 	}
+
 }
 
-func TestGistsService_Fork_invalidID(t *testing.T) {
-	client, _, _, teardown := setup()
+func TestGistsService_ListForks_withOptions(t *testing.T) {
+	client, mux, _, teardown := setup()
 	defer teardown()
 
-	_, _, err := client.Gists.Fork(context.Background(), "%")
-	testURLParseError(t, err)
+	mux.HandleFunc("/gists/1/forks", func(w http.ResponseWriter, r *http.Request) {
+		testMethod(t, r, "GET")
+		testFormValues(t, r, values{
+			"page": "2",
+		})
+		fmt.Fprint(w, `[]`)
+	})
+
+	gistForks, _, err := client.Gists.ListForks(context.Background(), "1", &ListOptions{Page: 2})
+	if err != nil {
+		t.Errorf("Gists.ListForks returned error: %v", err)
+	}
+
+	want := []*GistFork{}
+	if !reflect.DeepEqual(gistForks, want) {
+		t.Errorf("Gists.ListForks returned %+v, want %+v", gistForks, want)
+	}
+
+	// Test addOptions failure
+	_, _, err = client.Gists.ListForks(context.Background(), "%", &ListOptions{})
+	if err == nil {
+		t.Error("Gists.ListForks returned err = nil")
+	}
+
+	// Test client.NewRequest failure
+	got, resp, err := client.Gists.ListForks(context.Background(), "%", nil)
+	if got != nil {
+		t.Errorf("Gists.ListForks = %#v, want nil", got)
+	}
+	if resp != nil {
+		t.Errorf("Gists.ListForks resp = %#v, want nil", resp)
+	}
+	if err == nil {
+		t.Error("Gists.ListForks err = nil, want error")
+	}
+
+	// Test client.Do failure
+	client.rateLimits[0].Reset.Time = time.Now().Add(10 * time.Minute)
+	got, resp, err = client.Gists.ListForks(context.Background(), "1", &ListOptions{Page: 2})
+	if got != nil {
+		t.Errorf("Gists.ListForks returned = %#v, want nil", got)
+	}
+	if want := http.StatusForbidden; resp == nil || resp.Response.StatusCode != want {
+		t.Errorf("Gists.ListForks returned resp = %#v, want StatusCode=%v", resp.Response, want)
+	}
+	if err == nil {
+		t.Error("rGists.ListForks returned err = nil, want error")
+	}
 }