Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix regression caused by resolveOptions #18

Merged
merged 6 commits into from
Jan 27, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
39 changes: 25 additions & 14 deletions gh.go
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,14 @@ func run(path string, env []string, args ...string) (stdOut, stdErr bytes.Buffer
// As part of the configuration a hostname, auth token, and default set of headers are resolved
// from the gh environment configuration. These behaviors can be overridden using the opts argument.
func RESTClient(opts *api.ClientOptions) (api.RESTClient, error) {
err := resolveOptions(opts)
if opts == nil {
opts = &api.ClientOptions{}
}
cfg, err := config.Load()
if err != nil {
return nil, err
}
err = resolveOptions(opts, cfg)
if err != nil {
return nil, err
}
Expand All @@ -68,7 +75,14 @@ func RESTClient(opts *api.ClientOptions) (api.RESTClient, error) {
// As part of the configuration a hostname, auth token, and default set of headers are resolved
// from the gh environment configuration. These behaviors can be overridden using the opts argument.
func GQLClient(opts *api.ClientOptions) (api.GQLClient, error) {
err := resolveOptions(opts)
if opts == nil {
opts = &api.ClientOptions{}
}
cfg, err := config.Load()
if err != nil {
return nil, err
}
err = resolveOptions(opts, cfg)
if err != nil {
return nil, err
}
Expand All @@ -83,7 +97,14 @@ func GQLClient(opts *api.ClientOptions) (api.GQLClient, error) {
// host, the auth token will not be added to the headers. This is to protect against the case where tokens
// could be sent to an arbitrary host.
func HTTPClient(opts *api.ClientOptions) (*http.Client, error) {
err := resolveOptions(opts)
if opts == nil {
opts = &api.ClientOptions{}
}
cfg, err := config.Load()
if err != nil {
return nil, err
}
err = resolveOptions(opts, cfg)
if err != nil {
return nil, err
}
Expand Down Expand Up @@ -126,19 +147,9 @@ func CurrentRepository() (repo.Repository, error) {
return irepo.New(r.Host, r.Owner, r.Repo), nil
}

func resolveOptions(opts *api.ClientOptions) error {
var cfg config.Config
func resolveOptions(opts *api.ClientOptions, cfg config.Config) error {
var token string
var err error
if opts == nil {
opts = &api.ClientOptions{}
}
if opts.Host == "" || opts.AuthToken == "" {
cfg, err = config.Load()
if err != nil {
return err
}
}
if opts.Host == "" {
opts.Host = cfg.Host()
}
Expand Down
49 changes: 49 additions & 0 deletions gh_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import (
"os"
"testing"

"github.com/cli/go-gh/internal/config"
"github.com/cli/go-gh/internal/httpmock"
"github.com/cli/go-gh/pkg/api"
"github.com/stretchr/testify/assert"
Expand Down Expand Up @@ -128,3 +129,51 @@ func TestHTTPClient(t *testing.T) {
assert.Equal(t, "api.github.com", http.Requests[0].URL.Hostname())
assert.Equal(t, "token GH_TOKEN", http.Requests[0].Header.Get("Authorization"))
}

func TestResolveOptions(t *testing.T) {
cfg := testConfig()

tests := []struct {
name string
opts *api.ClientOptions
wantAuthToken string
wantHost string
}{
{
name: "honors consumer provided ClientOptions",
opts: &api.ClientOptions{
Host: "test.com",
AuthToken: "token_from_opts",
},
wantAuthToken: "token_from_opts",
wantHost: "test.com",
},
{
name: "uses config values if there are no consumer provided ClientOptions",
opts: &api.ClientOptions{},
wantAuthToken: "token",
wantHost: "github.com",
},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
err := resolveOptions(tt.opts, cfg)
assert.NoError(t, err)
assert.Equal(t, tt.wantHost, tt.opts.Host)
assert.Equal(t, tt.wantAuthToken, tt.opts.AuthToken)
})
}
}

func testConfig() config.Config {
var data = `
hosts:
github.com:
user: user1
oauth_token: token
git_protocol: ssh
`
cfg, _ := config.FromString(data)
return cfg
}
2 changes: 1 addition & 1 deletion internal/config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,7 @@ func normalizeHostname(host string) string {
return hostname
}

func fromString(str string) (Config, error) {
func FromString(str string) (Config, error) {
root, err := parseData([]byte(str))
if err != nil {
return nil, err
Expand Down
6 changes: 3 additions & 3 deletions internal/config/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -659,7 +659,7 @@ hosts:
oauth_token: yyyyyyyyyyyyyyyyyyyy
git_protocol: https
`
cfg, _ := fromString(data)
cfg, _ := FromString(data)
return cfg
}

Expand All @@ -675,11 +675,11 @@ hosts:
oauth_token: yyyyyyyyyyyyyyyyyyyy
git_protocol: https
`
cfg, _ := fromString(data)
cfg, _ := FromString(data)
return cfg
}

func testLoadedNoHostConfig() Config {
cfg, _ := fromString(testGlobalConfig())
cfg, _ := FromString(testGlobalConfig())
return cfg
}