From 7d8c1af009705e867af334ea11ce3c8144f704dd Mon Sep 17 00:00:00 2001 From: William Martin Date: Thu, 19 Oct 2023 12:48:30 +0200 Subject: [PATCH] Fix GitProtocol signature and rework test descriptions --- internal/config/auth_config_test.go | 45 +++++++++++++++++------------ internal/config/config.go | 14 ++++----- pkg/cmd/auth/refresh/refresh.go | 2 +- pkg/cmd/auth/status/status.go | 2 +- 4 files changed, 36 insertions(+), 27 deletions(-) diff --git a/internal/config/auth_config_test.go b/internal/config/auth_config_test.go index 221c5cb2f..2b0d0ff98 100644 --- a/internal/config/auth_config_test.go +++ b/internal/config/auth_config_test.go @@ -107,7 +107,7 @@ func TestTokenFromKeyringNonExistent(t *testing.T) { } func TestHasEnvTokenWithoutAnyEnvToken(t *testing.T) { - // Given an empty hosts configuration + // Given we have no env set authCfg := newTestAuthConfig(t) // When we check if it has an env token @@ -118,7 +118,8 @@ func TestHasEnvTokenWithoutAnyEnvToken(t *testing.T) { } func TestHasEnvTokenWithEnvToken(t *testing.T) { - // Given an empty hosts configuration but a token set in the env var + // Given we have an env token set + // Note that any valid env var for tokens will do, not just GH_ENTERPRISE_TOKEN authCfg := newTestAuthConfig(t) t.Setenv("GH_ENTERPRISE_TOKEN", "test-token") @@ -158,17 +159,28 @@ func TestUserNotLoggedIn(t *testing.T) { } func TestGitProtocolNotLoggedInDefaults(t *testing.T) { - // Given a host configuration without a git protocol + // Given we have not logged in authCfg := newTestAuthConfig(t) // When we get the git protocol - gitProtocol, err := authCfg.GitProtocol("github.com") + gitProtocol := authCfg.GitProtocol("github.com") - // Then it returns success, using the default - require.NoError(t, err) + // Then it returns the default require.Equal(t, "https", gitProtocol) } +func TestHostsIncludesEnvVar(t *testing.T) { + // Given the GH_HOST env var is set + authCfg := newTestAuthConfig(t) + t.Setenv("GH_HOST", "ghe.io") + + // When we get the hosts + hosts := authCfg.Hosts() + + // Then the host in the env var is included + require.Contains(t, hosts, "ghe.io") +} + func TestDefaultHostFromEnvVar(t *testing.T) { // Given the GH_HOST env var is set authCfg := newTestAuthConfig(t) @@ -284,31 +296,28 @@ func TestLoginSetsUserForProvidedHost(t *testing.T) { } func TestLoginSetsGitProtocolForProvidedHost(t *testing.T) { - // Given we are not logged in + // Given we are loggedin authCfg := newTestAuthConfig(t) - - // When we login _, err := authCfg.Login("github.com", "test-user", "test-token", "ssh", false) - - // Then it returns success and the git protocol is set require.NoError(t, err) - gitProtocol, err := authCfg.GitProtocol("github.com") - require.NoError(t, err) + // When we get the git protocol + gitProtocol := authCfg.GitProtocol("github.com") + + // Then it returns the git protocol we provided on login require.Equal(t, "ssh", gitProtocol) } func TestLoginAddsHostIfNotAlreadyAdded(t *testing.T) { - // Given we are not logged in + // Given we are logged in authCfg := newTestAuthConfig(t) - - // When we login _, err := authCfg.Login("github.com", "test-user", "test-token", "ssh", false) - - // Then it returns success and a host is added require.NoError(t, err) + // When we get the hosts hosts := authCfg.Hosts() + + // Then it includes our logged in host require.Contains(t, hosts, "github.com") } diff --git a/internal/config/config.go b/internal/config/config.go index 96a7f0716..587cc996b 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -164,19 +164,19 @@ func (c *AuthConfig) User(hostname string) (string, error) { // GitProtocol will retrieve the git protocol for the logged in user at the given hostname. // If none is set it will return the default value. -// TODO: although this returns an error, it actually has no path to error. -func (c *AuthConfig) GitProtocol(hostname string) (string, error) { +func (c *AuthConfig) GitProtocol(hostname string) string { key := "git_protocol" - val, err := c.cfg.Get([]string{hosts, hostname, key}) - if err == nil { - return val, err + if val, err := c.cfg.Get([]string{hosts, hostname, key}); err == nil { + return val } if val, ok := defaultFor(key); ok { - return val, nil + return val } - return "", nil + // This should not happen, as we know there is a default value for this key. + // Perhaps it says something about our current default abstraction being not quite right? + return "" } func (c *AuthConfig) Hosts() []string { diff --git a/pkg/cmd/auth/refresh/refresh.go b/pkg/cmd/auth/refresh/refresh.go index 489746179..30b5a3ff0 100644 --- a/pkg/cmd/auth/refresh/refresh.go +++ b/pkg/cmd/auth/refresh/refresh.go @@ -178,7 +178,7 @@ func refreshRun(opts *RefreshOptions) error { Prompter: opts.Prompter, GitClient: opts.GitClient, } - gitProtocol, _ := authCfg.GitProtocol(hostname) + gitProtocol := authCfg.GitProtocol(hostname) if opts.Interactive && gitProtocol == "https" { if err := credentialFlow.Prompt(hostname); err != nil { return err diff --git a/pkg/cmd/auth/status/status.go b/pkg/cmd/auth/status/status.go index 1739cfe09..bde74306a 100644 --- a/pkg/cmd/auth/status/status.go +++ b/pkg/cmd/auth/status/status.go @@ -139,7 +139,7 @@ func statusRun(opts *StatusOptions) error { } addMsg("%s Logged in to %s as %s (%s)", cs.SuccessIcon(), hostname, cs.Bold(username), tokenSource) - proto, _ := authCfg.GitProtocol(hostname) + proto := authCfg.GitProtocol(hostname) if proto != "" { addMsg("%s Git operations for %s configured to use %s protocol.", cs.SuccessIcon(), hostname, cs.Bold(proto))