X Tutup
Skip to content

Commit e4c8aa3

Browse files
mislavmislav
committed
Add tests for pr diff --patch 
1 parent e0897fd commit e4c8aa3

File tree

3 files changed

+82
-39
lines changed

3 files changed

+82
-39
lines changed

‎api/queries_pr.go‎

Lines changed: 0 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -2,9 +2,7 @@ package api
22

33
import (
44
"context"
5-
"errors"
65
"fmt"
7-
"io"
86
"net/http"
97
"strings"
108
"time"
@@ -267,34 +265,6 @@ func (pr *PullRequest) DisplayableReviews() PullRequestReviews {
267265
return PullRequestReviews{Nodes: published, TotalCount: len(published)}
268266
}
269267

270-
func (c Client) PullRequestDiff(baseRepo ghrepo.Interface, prNumber int, patch bool) (io.ReadCloser, error) {
271-
url := fmt.Sprintf("%srepos/%s/pulls/%d",
272-
ghinstance.RESTPrefix(baseRepo.RepoHost()), ghrepo.FullName(baseRepo), prNumber)
273-
req, err := http.NewRequest("GET", url, nil)
274-
if err != nil {
275-
return nil, err
276-
}
277-
278-
if patch {
279-
req.Header.Set("Accept", "application/vnd.github.v3.patch")
280-
} else {
281-
req.Header.Set("Accept", "application/vnd.github.v3.diff; charset=utf-8")
282-
}
283-
284-
resp, err := c.http.Do(req)
285-
if err != nil {
286-
return nil, err
287-
}
288-
289-
if resp.StatusCode == 404 {
290-
return nil, errors.New("pull request not found")
291-
} else if resp.StatusCode != 200 {
292-
return nil, HandleHTTPError(resp)
293-
}
294-
295-
return resp.Body, nil
296-
}
297-
298268
type pullRequestFeature struct {
299269
HasReviewDecision bool
300270
HasStatusCheckRollup bool

‎pkg/cmd/pr/diff/diff.go‎

Lines changed: 33 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,8 @@ import (
1111

1212
"github.com/MakeNowJust/heredoc"
1313
"github.com/cli/cli/v2/api"
14+
"github.com/cli/cli/v2/internal/ghinstance"
15+
"github.com/cli/cli/v2/internal/ghrepo"
1416
"github.com/cli/cli/v2/pkg/cmd/pr/shared"
1517
"github.com/cli/cli/v2/pkg/cmdutil"
1618
"github.com/cli/cli/v2/pkg/iostreams"
@@ -90,9 +92,8 @@ func diffRun(opts *DiffOptions) error {
9092
if err != nil {
9193
return err
9294
}
93-
apiClient := api.NewClientFromHTTP(httpClient)
9495

95-
diff, err := apiClient.PullRequestDiff(baseRepo, pr.Number, opts.Patch)
96+
diff, err := fetchDiff(httpClient, baseRepo, pr.Number, opts.Patch)
9697
if err != nil {
9798
return fmt.Errorf("could not find pull request diff: %w", err)
9899
}
@@ -134,6 +135,36 @@ func diffRun(opts *DiffOptions) error {
134135
return nil
135136
}
136137

138+
func fetchDiff(httpClient *http.Client, baseRepo ghrepo.Interface, prNumber int, asPatch bool) (io.ReadCloser, error) {
139+
url := fmt.Sprintf(
140+
"%srepos/%s/pulls/%d",
141+
ghinstance.RESTPrefix(baseRepo.RepoHost()),
142+
ghrepo.FullName(baseRepo),
143+
prNumber,
144+
)
145+
acceptType := "application/vnd.github.v3.diff"
146+
if asPatch {
147+
acceptType = "application/vnd.github.v3.patch"
148+
}
149+
150+
req, err := http.NewRequest("GET", url, nil)
151+
if err != nil {
152+
return nil, err
153+
}
154+
155+
req.Header.Set("Accept", acceptType)
156+
157+
resp, err := httpClient.Do(req)
158+
if err != nil {
159+
return nil, err
160+
}
161+
if resp.StatusCode != 200 {
162+
return nil, api.HandleHTTPError(resp)
163+
}
164+
165+
return resp.Body, nil
166+
}
167+
137168
var diffHeaderPrefixes = []string{"+++", "---", "diff", "index"}
138169

139170
func isHeaderLine(dl string) bool {

‎pkg/cmd/pr/diff/diff_test.go‎

Lines changed: 49 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import (
44
"bytes"
55
"io/ioutil"
66
"net/http"
7+
"strings"
78
"testing"
89

910
"github.com/cli/cli/v2/api"
@@ -140,26 +141,67 @@ func runCommand(rt http.RoundTripper, remotes context.Remotes, isTTY bool, cli s
140141
}, err
141142
}
142143

143-
func TestPRDiff_notty(t *testing.T) {
144-
http := &httpmock.Registry{}
145-
defer http.Verify(t)
144+
func TestPRDiff_notty_diff(t *testing.T) {
145+
httpReg := &httpmock.Registry{}
146+
defer httpReg.Verify(t)
146147

147148
shared.RunCommandFinder("", &api.PullRequest{Number: 123}, ghrepo.New("OWNER", "REPO"))
148149

149-
http.Register(
150+
var gotAccept string
151+
httpReg.Register(
152+
httpmock.REST("GET", "repos/OWNER/REPO/pulls/123"),
153+
func(req *http.Request) (*http.Response, error) {
154+
gotAccept = req.Header.Get("Accept")
155+
return &http.Response{
156+
StatusCode: 200,
157+
Request: req,
158+
Body: ioutil.NopCloser(strings.NewReader(testDiff)),
159+
}, nil
160+
})
161+
162+
output, err := runCommand(httpReg, nil, false, "")
163+
if err != nil {
164+
t.Fatalf("unexpected error: %s", err)
165+
}
166+
if diff := cmp.Diff(testDiff, output.String()); diff != "" {
167+
t.Errorf("command output did not match:\n%s", diff)
168+
}
169+
if gotAccept != "application/vnd.github.v3.diff" {
170+
t.Errorf("unexpected Accept header: %s", gotAccept)
171+
}
172+
}
173+
174+
func TestPRDiff_notty_patch(t *testing.T) {
175+
httpReg := &httpmock.Registry{}
176+
defer httpReg.Verify(t)
177+
178+
shared.RunCommandFinder("", &api.PullRequest{Number: 123}, ghrepo.New("OWNER", "REPO"))
179+
180+
var gotAccept string
181+
httpReg.Register(
150182
httpmock.REST("GET", "repos/OWNER/REPO/pulls/123"),
151-
httpmock.StringResponse(testDiff))
183+
func(req *http.Request) (*http.Response, error) {
184+
gotAccept = req.Header.Get("Accept")
185+
return &http.Response{
186+
StatusCode: 200,
187+
Request: req,
188+
Body: ioutil.NopCloser(strings.NewReader(testDiff)),
189+
}, nil
190+
})
152191

153-
output, err := runCommand(http, nil, false, "")
192+
output, err := runCommand(httpReg, nil, false, "--patch")
154193
if err != nil {
155194
t.Fatalf("unexpected error: %s", err)
156195
}
157196
if diff := cmp.Diff(testDiff, output.String()); diff != "" {
158197
t.Errorf("command output did not match:\n%s", diff)
159198
}
199+
if gotAccept != "application/vnd.github.v3.patch" {
200+
t.Errorf("unexpected Accept header: %s", gotAccept)
201+
}
160202
}
161203

162-
func TestPRDiff_tty(t *testing.T) {
204+
func TestPRDiff_tty_diff(t *testing.T) {
163205
http := &httpmock.Registry{}
164206
defer http.Verify(t)
165207

0 commit comments

Comments
 (0)
X Tutup