Skip to content

Commit 8a221bb

Browse files
mislavmislav
committed
Add tests for our default HTTP client
1 parent 75abeb1 commit 8a221bb

File tree

2 files changed

+172
-4
lines changed

2 files changed

+172
-4
lines changed

‎pkg/cmd/factory/http.go‎

Lines changed: 14 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,6 @@ import (
88
"time"
99

1010
"github.com/cli/cli/api"
11-
"github.com/cli/cli/internal/config"
1211
"github.com/cli/cli/internal/ghinstance"
1312
"github.com/cli/cli/pkg/iostreams"
1413
)
@@ -53,8 +52,12 @@ var timezoneNames = map[int]string{
5352
50400: "Pacific/Kiritimati",
5453
}
5554

55+
type configGetter interface {
56+
Get(string, string) (string, error)
57+
}
58+
5659
// generic authenticated HTTP client for commands
57-
func NewHTTPClient(io *iostreams.IOStreams, cfg config.Config, appVersion string, setAccept bool) *http.Client {
60+
func NewHTTPClient(io *iostreams.IOStreams, cfg configGetter, appVersion string, setAccept bool) *http.Client {
5861
var opts []api.ClientOption
5962
if verbose := os.Getenv("DEBUG"); verbose != "" {
6063
logTraffic := strings.Contains(verbose, "api")
@@ -64,7 +67,7 @@ func NewHTTPClient(io *iostreams.IOStreams, cfg config.Config, appVersion string
6467
opts = append(opts,
6568
api.AddHeader("User-Agent", fmt.Sprintf("GitHub CLI %s", appVersion)),
6669
api.AddHeaderFunc("Authorization", func(req *http.Request) (string, error) {
67-
hostname := ghinstance.NormalizeHostname(req.URL.Hostname())
70+
hostname := ghinstance.NormalizeHostname(getHost(req))
6871
if token, err := cfg.Get(hostname, "oauth_token"); err == nil && token != "" {
6972
return fmt.Sprintf("token %s", token), nil
7073
}
@@ -89,7 +92,7 @@ func NewHTTPClient(io *iostreams.IOStreams, cfg config.Config, appVersion string
8992
accept := "application/vnd.github.antiope-preview+json"
9093
// introduced for #2952: pr branch up to date status
9194
accept += ", application/vnd.github.merge-info-preview+json"
92-
if ghinstance.IsEnterprise(req.URL.Hostname()) {
95+
if ghinstance.IsEnterprise(getHost(req)) {
9396
// shadow-cat-preview: Draft pull requests
9497
accept += ", application/vnd.github.shadow-cat-preview"
9598
}
@@ -100,3 +103,10 @@ func NewHTTPClient(io *iostreams.IOStreams, cfg config.Config, appVersion string
100103

101104
return api.NewHTTPClient(opts...)
102105
}
106+
107+
func getHost(r *http.Request) string {
108+
if r.Host != "" {
109+
return r.Host
110+
}
111+
return r.URL.Hostname()
112+
}

‎pkg/cmd/factory/http_test.go‎

Lines changed: 158 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,158 @@
1+
package factory
2+
3+
import (
4+
"fmt"
5+
"net/http"
6+
"net/http/httptest"
7+
"os"
8+
"regexp"
9+
"testing"
10+
11+
"github.com/MakeNowJust/heredoc"
12+
"github.com/cli/cli/pkg/iostreams"
13+
"github.com/stretchr/testify/assert"
14+
"github.com/stretchr/testify/require"
15+
)
16+
17+
func TestNewHTTPClient(t *testing.T) {
18+
type args struct {
19+
config configGetter
20+
appVersion string
21+
setAccept bool
22+
}
23+
tests := []struct {
24+
name string
25+
args args
26+
envDebug string
27+
host string
28+
wantHeader map[string]string
29+
wantStderr string
30+
}{
31+
{
32+
name: "github.com with Accept header",
33+
args: args{
34+
config: tinyConfig{"github.com:oauth_token": "MYTOKEN"},
35+
appVersion: "v1.2.3",
36+
setAccept: true,
37+
},
38+
host: "github.com",
39+
wantHeader: map[string]string{
40+
"authorization": "token MYTOKEN",
41+
"user-agent": "GitHub CLI v1.2.3",
42+
"accept": "application/vnd.github.antiope-preview+json, application/vnd.github.merge-info-preview+json",
43+
},
44+
wantStderr: "",
45+
},
46+
{
47+
name: "github.com no Accept header",
48+
args: args{
49+
config: tinyConfig{"example.com:oauth_token": "MYTOKEN"},
50+
appVersion: "v1.2.3",
51+
setAccept: false,
52+
},
53+
host: "github.com",
54+
wantHeader: map[string]string{
55+
"authorization": "",
56+
"user-agent": "GitHub CLI v1.2.3",
57+
"accept": "",
58+
},
59+
wantStderr: "",
60+
},
61+
{
62+
name: "github.com in verbose mode",
63+
args: args{
64+
config: tinyConfig{"github.com:oauth_token": "MYTOKEN"},
65+
appVersion: "v1.2.3",
66+
setAccept: false,
67+
},
68+
host: "github.com",
69+
envDebug: "api",
70+
wantHeader: map[string]string{
71+
"authorization": "token MYTOKEN",
72+
"user-agent": "GitHub CLI v1.2.3",
73+
"accept": "",
74+
},
75+
wantStderr: heredoc.Doc(`
76+
* Request at <time>
77+
* Request to http://<host>:<port>
78+
> GET / HTTP/1.1
79+
> Host: github.com
80+
> Authorization: token ████████████████████
81+
> User-Agent: GitHub CLI v1.2.3
82+
83+
< HTTP/1.1 204 No Content
84+
< Date: <time>
85+
86+
* Request took <duration>
87+
`),
88+
},
89+
{
90+
name: "GHES Accept header",
91+
args: args{
92+
config: tinyConfig{"example.com:oauth_token": "GHETOKEN"},
93+
appVersion: "v1.2.3",
94+
setAccept: true,
95+
},
96+
host: "example.com",
97+
wantHeader: map[string]string{
98+
"authorization": "token GHETOKEN",
99+
"user-agent": "GitHub CLI v1.2.3",
100+
"accept": "application/vnd.github.antiope-preview+json, application/vnd.github.merge-info-preview+json, application/vnd.github.shadow-cat-preview",
101+
},
102+
wantStderr: "",
103+
},
104+
}
105+
106+
var gotReq *http.Request
107+
ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
108+
gotReq = r
109+
w.WriteHeader(http.StatusNoContent)
110+
}))
111+
defer ts.Close()
112+
113+
for _, tt := range tests {
114+
t.Run(tt.name, func(t *testing.T) {
115+
oldDebug := os.Getenv("DEBUG")
116+
os.Setenv("DEBUG", tt.envDebug)
117+
t.Cleanup(func() {
118+
os.Setenv("DEBUG", oldDebug)
119+
})
120+
121+
io, _, _, stderr := iostreams.Test()
122+
client := NewHTTPClient(io, tt.args.config, tt.args.appVersion, tt.args.setAccept)
123+
124+
req, err := http.NewRequest("GET", ts.URL, nil)
125+
req.Host = tt.host
126+
require.NoError(t, err)
127+
128+
res, err := client.Do(req)
129+
require.NoError(t, err)
130+
131+
for name, value := range tt.wantHeader {
132+
assert.Equal(t, value, gotReq.Header.Get(name), name)
133+
}
134+
135+
assert.Equal(t, 204, res.StatusCode)
136+
assert.Equal(t, tt.wantStderr, normalizeVerboseLog(stderr.String()))
137+
})
138+
}
139+
}
140+
141+
type tinyConfig map[string]string
142+
143+
func (c tinyConfig) Get(host, key string) (string, error) {
144+
return c[fmt.Sprintf("%s:%s", host, key)], nil
145+
}
146+
147+
var requestAtRE = regexp.MustCompile(`(?m)^\* Request at .+`)
148+
var dateRE = regexp.MustCompile(`(?m)^< Date: .+`)
149+
var hostWithPortRE = regexp.MustCompile(`127\.0\.0\.1:\d+`)
150+
var durationRE = regexp.MustCompile(`(?m)^\* Request took .+`)
151+
152+
func normalizeVerboseLog(t string) string {
153+
t = requestAtRE.ReplaceAllString(t, "* Request at <time>")
154+
t = hostWithPortRE.ReplaceAllString(t, "<host>:<port>")
155+
t = dateRE.ReplaceAllString(t, "< Date: <time>")
156+
t = durationRE.ReplaceAllString(t, "* Request took <duration>")
157+
return t
158+
}

0 commit comments

Comments
 (0)