Repository: cli/cli
Pull request #9095 (2024-05-17 12:47:11)

Rework first auth tests with new gitcredential abstractions
## Description

This is a follow up to #9089 as progress towards #8875.

In #9089 new abstractions were introduced to more clearly separate the configuration of git credentials configuration, and informing the helpers of updated credentials.

This PR now cleaves `GitCredentialFlow` and `gitcredentials.HelperConfig` by introducing a `HelperConfig` interface on the consumer side. We introduce a contract test that can be run by concrete implementations of this interface to ensure they are able to be test doubled with confidence. That is, a new `gitcredentials.FakeHelperConfig` can be used in place of the `gitcredentials.HelperConfig` without requiring `git` and an isolated config (instead, that is only needed for the `HelperConfig` at the leaf of the dependency graph).

This `FakeHelperConfig` can then be used in the `LoginFlow` tests, of which I have created exactly one to demonstrate this pattern. The purpose of this new test is to ensure that when no credential helper is found for the targeted host, the user is presented with a prompt, and when they accept that prompt, `gh` is configured as their credential helper.

Summed up, it makes this branch go from red to green:

[Login Flow](https://github.com/cli/cli/blob/3fe555b0bb7c83cb6f06e83de929c6c3ef1300f2/pkg/cmd/auth/shared/login_flow.go#L205):

<img width="615" alt="image showing test coverage of the previous links code" src="https://github.com/cli/cli/assets/1611510/58f2dd45-8fed-4637-ba22-08d3f605ac33">

And therefore all of this [GitCredentialFlow](https://github.com/cli/cli/blob/3fe555b0bb7c83cb6f06e83de929c6c3ef1300f2/pkg/cmd/auth/shared/login_flow.go#L205) code goes green too:

<img width="1043" alt="image showing test coverage of the previous links code" src="https://github.com/cli/cli/assets/1611510/530c6dda-1490-4d3e-a7b3-fd6c36095665">

### Why now?

In #8875 I want to lean on these abstractions to ensure that `auth switch` exhibits similar behaviour but it should be easier to develop and review in smaller incremental steps demonstrating the pattern with existing code.


Patch:
diff --git a/pkg/cmd/auth/login/login.go b/pkg/cmd/auth/login/login.go
index 9104dd175..9e27f28ac 100644
--- a/pkg/cmd/auth/login/login.go
+++ b/pkg/cmd/auth/login/login.go
@@ -12,6 +12,7 @@ import (
 	"github.com/cli/cli/v2/internal/gh"
 	"github.com/cli/cli/v2/internal/ghinstance"
 	"github.com/cli/cli/v2/pkg/cmd/auth/shared"
+	"github.com/cli/cli/v2/pkg/cmd/auth/shared/gitcredentials"
 	"github.com/cli/cli/v2/pkg/cmdutil"
 	"github.com/cli/cli/v2/pkg/iostreams"
 	ghAuth "github.com/cli/go-gh/v2/pkg/auth"
@@ -195,18 +196,26 @@ func loginRun(opts *LoginOptions) error {
 	}
 
 	return shared.Login(&shared.LoginOptions{
-		IO:               opts.IO,
-		Config:           authCfg,
-		HTTPClient:       httpClient,
-		Hostname:         hostname,
-		Interactive:      opts.Interactive,
-		Web:              opts.Web,
-		Scopes:           opts.Scopes,
-		Executable:       opts.MainExecutable,
-		GitProtocol:      opts.GitProtocol,
-		Prompter:         opts.Prompter,
-		GitClient:        opts.GitClient,
-		Browser:          opts.Browser,
+		IO:          opts.IO,
+		Config:      authCfg,
+		HTTPClient:  httpClient,
+		Hostname:    hostname,
+		Interactive: opts.Interactive,
+		Web:         opts.Web,
+		Scopes:      opts.Scopes,
+		GitProtocol: opts.GitProtocol,
+		Prompter:    opts.Prompter,
+		Browser:     opts.Browser,
+		CredentialFlow: &shared.GitCredentialFlow{
+			Prompter: opts.Prompter,
+			HelperConfig: &gitcredentials.HelperConfig{
+				SelfExecutablePath: opts.MainExecutable,
+				GitClient:          opts.GitClient,
+			},
+			Updater: &gitcredentials.Updater{
+				GitClient: opts.GitClient,
+			},
+		},
 		SecureStorage:    !opts.InsecureStorage,
 		SkipSSHKeyPrompt: opts.SkipSSHKeyPrompt,
 	})
diff --git a/pkg/cmd/auth/shared/contract/helper_config.go b/pkg/cmd/auth/shared/contract/helper_config.go
new file mode 100644
index 000000000..d507e1091
--- /dev/null
+++ b/pkg/cmd/auth/shared/contract/helper_config.go
@@ -0,0 +1,64 @@
+package contract
+
+import (
+	"testing"
+
+	"github.com/cli/cli/v2/pkg/cmd/auth/shared"
+	"github.com/stretchr/testify/require"
+)
+
+// This HelperConfig contract exist to ensure that any HelperConfig implementation conforms to this behaviour.
+// This is useful because we can swap in fake implementations for testing, rather than requiring our tests to be
+// isolated from git.
+//
+// See for example, TestAuthenticatingGitCredentials for LoginFlow.
+type HelperConfig struct {
+	NewHelperConfig func(t *testing.T) shared.HelperConfig
+	ConfigureHelper func(t *testing.T, hostname string)
+}
+
+func (contract HelperConfig) Test(t *testing.T) {
+	t.Run("when there are no credential helpers, configures gh for repo and gist host", func(t *testing.T) {
+		hc := contract.NewHelperConfig(t)
+		require.NoError(t, hc.ConfigureOurs("github.com"))
+
+		repoHelper, err := hc.ConfiguredHelper("github.com")
+		require.NoError(t, err)
+		require.True(t, repoHelper.IsConfigured(), "expected our helper to be configured")
+		require.True(t, repoHelper.IsOurs(), "expected the helper to be ours but was %q", repoHelper.Cmd)
+
+		gistHelper, err := hc.ConfiguredHelper("gist.github.com")
+		require.NoError(t, err)
+		require.True(t, gistHelper.IsConfigured(), "expected our helper to be configured")
+		require.True(t, gistHelper.IsOurs(), "expected the helper to be ours but was %q", gistHelper.Cmd)
+	})
+
+	t.Run("when there is a global credential helper, it should be configured but not ours", func(t *testing.T) {
+		hc := contract.NewHelperConfig(t)
+		contract.ConfigureHelper(t, "credential.helper")
+
+		helper, err := hc.ConfiguredHelper("github.com")
+		require.NoError(t, err)
+		require.True(t, helper.IsConfigured(), "expected helper to be configured")
+		require.False(t, helper.IsOurs(), "expected the helper not to be ours but was %q", helper.Cmd)
+	})
+
+	t.Run("when there is a host credential helper, it should be configured but not ours", func(t *testing.T) {
+		hc := contract.NewHelperConfig(t)
+		contract.ConfigureHelper(t, "credential.https://github.com.helper")
+
+		helper, err := hc.ConfiguredHelper("github.com")
+		require.NoError(t, err)
+		require.True(t, helper.IsConfigured(), "expected helper to be configured")
+		require.False(t, helper.IsOurs(), "expected the helper not to be ours but was %q", helper.Cmd)
+	})
+
+	t.Run("returns non configured helper when no helpers are configured", func(t *testing.T) {
+		hc := contract.NewHelperConfig(t)
+
+		helper, err := hc.ConfiguredHelper("github.com")
+		require.NoError(t, err)
+		require.False(t, helper.IsConfigured(), "expected no helper to be configured")
+		require.False(t, helper.IsOurs(), "expected the helper not to be ours but was %q", helper.Cmd)
+	})
+}
diff --git a/pkg/cmd/auth/shared/git_credential.go b/pkg/cmd/auth/shared/git_credential.go
index c98992a76..e3136de43 100644
--- a/pkg/cmd/auth/shared/git_credential.go
+++ b/pkg/cmd/auth/shared/git_credential.go
@@ -7,10 +7,15 @@ import (
 	"github.com/cli/cli/v2/pkg/cmd/auth/shared/gitcredentials"
 )
 
+type HelperConfig interface {
+	ConfigureOurs(hostname string) error
+	ConfiguredHelper(hostname string) (gitcredentials.Helper, error)
+}
+
 type GitCredentialFlow struct {
 	Prompter Prompt
 
-	HelperConfig *gitcredentials.HelperConfig
+	HelperConfig HelperConfig
 	Updater      *gitcredentials.Updater
 
 	shouldSetup bool
diff --git a/pkg/cmd/auth/shared/gitcredentials/fake_helper_config.go b/pkg/cmd/auth/shared/gitcredentials/fake_helper_config.go
new file mode 100644
index 000000000..f6ae2f2c0
--- /dev/null
+++ b/pkg/cmd/auth/shared/gitcredentials/fake_helper_config.go
@@ -0,0 +1,49 @@
+package gitcredentials
+
+import (
+	"fmt"
+	"strings"
+
+	"github.com/cli/cli/v2/internal/ghinstance"
+)
+
+type FakeHelperConfig struct {
+	SelfExecutablePath string
+	Helpers            map[string]Helper
+}
+
+// ConfigureOurs sets up the git credential helper chain to use the GitHub CLI credential helper for git repositories
+// including gists.
+func (hc *FakeHelperConfig) ConfigureOurs(hostname string) error {
+	credHelperKeys := []string{
+		keyFor(hostname),
+	}
+
+	gistHost := strings.TrimSuffix(ghinstance.GistHost(hostname), "/")
+	if strings.HasPrefix(gistHost, "gist.") {
+		credHelperKeys = append(credHelperKeys, keyFor(gistHost))
+	}
+
+	for _, credHelperKey := range credHelperKeys {
+		hc.Helpers[credHelperKey] = Helper{
+			Cmd: fmt.Sprintf("!%s auth git-credential", shellQuote(hc.SelfExecutablePath)),
+		}
+	}
+
+	return nil
+}
+
+// ConfiguredHelper returns the configured git credential helper for a given hostname.
+func (hc *FakeHelperConfig) ConfiguredHelper(hostname string) (Helper, error) {
+	helper, ok := hc.Helpers[keyFor(hostname)]
+	if ok {
+		return helper, nil
+	}
+
+	helper, ok = hc.Helpers["credential.helper"]
+	if ok {
+		return helper, nil
+	}
+
+	return Helper{}, nil
+}
diff --git a/pkg/cmd/auth/shared/login_flow.go b/pkg/cmd/auth/shared/login_flow.go
index b0feddd2f..93455e8ee 100644
--- a/pkg/cmd/auth/shared/login_flow.go
+++ b/pkg/cmd/auth/shared/login_flow.go
@@ -11,11 +11,9 @@ import (
 
 	"github.com/MakeNowJust/heredoc"
 	"github.com/cli/cli/v2/api"
-	"github.com/cli/cli/v2/git"
 	"github.com/cli/cli/v2/internal/authflow"
 	"github.com/cli/cli/v2/internal/browser"
 	"github.com/cli/cli/v2/internal/ghinstance"
-	"github.com/cli/cli/v2/pkg/cmd/auth/shared/gitcredentials"
 	"github.com/cli/cli/v2/pkg/cmd/ssh-key/add"
 	"github.com/cli/cli/v2/pkg/iostreams"
 	"github.com/cli/cli/v2/pkg/ssh"
@@ -32,15 +30,14 @@ type LoginOptions struct {
 	IO               *iostreams.IOStreams
 	Config           iconfig
 	HTTPClient       *http.Client
-	GitClient        *git.Client
 	Hostname         string
 	Interactive      bool
 	Web              bool
 	Scopes           []string
-	Executable       string
 	GitProtocol      string
 	Prompter         Prompt
 	Browser          browser.Browser
+	CredentialFlow   *GitCredentialFlow
 	SecureStorage    bool
 	SkipSSHKeyPrompt bool
 
@@ -72,21 +69,11 @@ func Login(opts *LoginOptions) error {
 
 	var additionalScopes []string
 
-	credentialFlow := &GitCredentialFlow{
-		Prompter: opts.Prompter,
-		HelperConfig: &gitcredentials.HelperConfig{
-			SelfExecutablePath: opts.Executable,
-			GitClient:          opts.GitClient,
-		},
-		Updater: &gitcredentials.Updater{
-			GitClient: opts.GitClient,
-		},
-	}
 	if opts.Interactive && gitProtocol == "https" {
-		if err := credentialFlow.Prompt(hostname); err != nil {
+		if err := opts.CredentialFlow.Prompt(hostname); err != nil {
 			return err
 		}
-		additionalScopes = append(additionalScopes, credentialFlow.Scopes()...)
+		additionalScopes = append(additionalScopes, opts.CredentialFlow.Scopes()...)
 	}
 
 	var keyToUpload string
@@ -214,8 +201,8 @@ func Login(opts *LoginOptions) error {
 		fmt.Fprintf(opts.IO.ErrOut, "%s Authentication credentials saved in plain text\n", cs.Yellow("!"))
 	}
 
-	if credentialFlow.ShouldSetup() {
-		err := credentialFlow.Setup(hostname, username, authToken)
+	if opts.CredentialFlow.ShouldSetup() {
+		err := opts.CredentialFlow.Setup(hostname, username, authToken)
 		if err != nil {
 			return err
 		}
diff --git a/pkg/cmd/pr/checks/aggregate.go b/pkg/cmd/pr/checks/aggregate.go
index 91cec4335..40e34e79a 100644
--- a/pkg/cmd/pr/checks/aggregate.go
+++ b/pkg/cmd/pr/checks/aggregate.go
@@ -6,7 +6,6 @@ import (
 	"time"
 
 	"github.com/cli/cli/v2/api"
-	"github.com/cli/cli/v2/pkg/cmdutil"
 )
 
 type check struct {
@@ -29,10 +28,6 @@ type checkCounts struct {
 	Canceled int
 }
 
-func (ch *check) ExportData(fields []string) map[string]interface{} {
-	return cmdutil.StructExportData(ch, fields)
-}
-
 func aggregateChecks(checkContexts []api.CheckContext, requiredChecks bool) (checks []check, counts checkCounts) {
 	for _, c := range eliminateDuplicates(checkContexts) {
 		if requiredChecks && !c.IsRequired {
diff --git a/pkg/cmd/pr/checks/checks.go b/pkg/cmd/pr/checks/checks.go
index bbf2f453b..335226a9c 100644
--- a/pkg/cmd/pr/checks/checks.go
+++ b/pkg/cmd/pr/checks/checks.go
@@ -20,23 +20,10 @@ import (
 
 const defaultInterval time.Duration = 10 * time.Second
 
-var prCheckFields = []string{
-	"name",
-	"state",
-	"startedAt",
-	"completedAt",
-	"link",
-	"bucket",
-	"event",
-	"workflow",
-	"description",
-}
-
 type ChecksOptions struct {
 	HttpClient func() (*http.Client, error)
 	IO         *iostreams.IOStreams
 	Browser    browser.Browser
-	Exporter   cmdutil.Exporter
 
 	Finder   shared.PRFinder
 	Detector fd.Detector
@@ -110,8 +97,6 @@ func NewCmdChecks(f *cmdutil.Factory, runF func(*ChecksOptions) error) *cobra.Co
 	cmd.Flags().IntVarP(&interval, "interval", "i", 10, "Refresh interval in seconds when using `--watch` flag")
 	cmd.Flags().BoolVar(&opts.Required, "required", false, "Only show checks that are required")
 
-	cmdutil.AddJSONFlags(cmd, &opts.Exporter, prCheckFields)
-
 	return cmd
 }
 
@@ -176,10 +161,6 @@ func checksRun(opts *ChecksOptions) error {
 		return err
 	}
 
-	if opts.Exporter != nil {
-		return opts.Exporter.Write(opts.IO, checks)
-	}
-
 	if opts.Watch {
 		opts.IO.StartAlternateScreenBuffer()
 	} else {


Test patch:
diff --git a/pkg/cmd/auth/shared/gitcredentials/fake_helper_config_test.go b/pkg/cmd/auth/shared/gitcredentials/fake_helper_config_test.go
new file mode 100644
index 000000000..441972aff
--- /dev/null
+++ b/pkg/cmd/auth/shared/gitcredentials/fake_helper_config_test.go
@@ -0,0 +1,32 @@
+package gitcredentials_test
+
+import (
+	"testing"
+
+	"github.com/cli/cli/v2/pkg/cmd/auth/shared"
+	"github.com/cli/cli/v2/pkg/cmd/auth/shared/contract"
+	"github.com/cli/cli/v2/pkg/cmd/auth/shared/gitcredentials"
+)
+
+func TestFakeHelperConfigContract(t *testing.T) {
+	// Note that this being mutated by `NewHelperConfig` makes these tests not parallelizable
+	var fhc *gitcredentials.FakeHelperConfig
+
+	contract.HelperConfig{
+		NewHelperConfig: func(t *testing.T) shared.HelperConfig {
+			// Mutate the closed over fhc so that ConfigureHelper is able to configure helpers
+			// for tests. An alternative would be to provide the Helper as an argument back to ConfigureHelper
+			// but then we'd have to type assert it back to *FakeHelperConfig, which is probably more trouble than
+			// it's worth to parallelize these tests, sinced it's not even possible to parallelize the real Helperconfig
+			// ones due to them using t.Setenv
+			fhc = &gitcredentials.FakeHelperConfig{
+				SelfExecutablePath: "/path/to/gh",
+				Helpers:            map[string]gitcredentials.Helper{},
+			}
+			return fhc
+		},
+		ConfigureHelper: func(t *testing.T, hostname string) {
+			fhc.Helpers[hostname] = gitcredentials.Helper{Cmd: "test-helper"}
+		},
+	}.Test(t)
+}
diff --git a/pkg/cmd/auth/shared/gitcredentials/helper_config_test.go b/pkg/cmd/auth/shared/gitcredentials/helper_config_test.go
index b7ddbb883..80ffac85a 100644
--- a/pkg/cmd/auth/shared/gitcredentials/helper_config_test.go
+++ b/pkg/cmd/auth/shared/gitcredentials/helper_config_test.go
@@ -7,6 +7,8 @@ import (
 	"testing"
 
 	"github.com/cli/cli/v2/git"
+	"github.com/cli/cli/v2/pkg/cmd/auth/shared"
+	"github.com/cli/cli/v2/pkg/cmd/auth/shared/contract"
 	"github.com/cli/cli/v2/pkg/cmd/auth/shared/gitcredentials"
 	"github.com/stretchr/testify/require"
 )
@@ -33,105 +35,40 @@ func configureTestCredentialHelper(t *testing.T, key string) {
 	require.NoError(t, cmd.Run())
 }
 
-func TestConfigureOursNoPreexistingHelpersConfigured(t *testing.T) {
-	// Given there are no credential helpers configured
-	withIsolatedGitConfig(t)
-
-	// When we configure ourselves as the git credential helper
-	hc := gitcredentials.HelperConfig{
-		SelfExecutablePath: "/path/to/gh",
-		GitClient:          &git.Client{},
-	}
-	require.NoError(t, hc.ConfigureOurs("github.com"))
+func TestHelperConfigContract(t *testing.T) {
+	contract.HelperConfig{
+		NewHelperConfig: func(t *testing.T) shared.HelperConfig {
+			withIsolatedGitConfig(t)
 
-	// Then the git config should be updated to use our credential helper for both repos and gists
-	repoHelper, err := hc.ConfiguredHelper("github.com")
-	require.NoError(t, err)
-	require.True(t, repoHelper.IsConfigured(), "expected our helper to be configured")
-	require.True(t, repoHelper.IsOurs(), "expected the helper to be ours but was %q", repoHelper.Cmd)
-
-	gistHelper, err := hc.ConfiguredHelper("gist.github.com")
-	require.NoError(t, err)
-	require.True(t, gistHelper.IsConfigured(), "expected our helper to be configured")
-	require.True(t, gistHelper.IsOurs(), "expected the helper to be ours but was %q", gistHelper.Cmd)
+			return &gitcredentials.HelperConfig{
+				SelfExecutablePath: "/path/to/gh",
+				GitClient:          &git.Client{},
+			}
+		},
+		ConfigureHelper: func(t *testing.T, hostname string) {
+			configureTestCredentialHelper(t, hostname)
+		},
+	}.Test(t)
 }
 
-func TestConfigureOursWithPreexistingHelpersConfigured(t *testing.T) {
-	// Given there are other credential helpers configured
+// This is a whitebox test unlike the contract because although we don't use the exact configured command, it's
+// important that it is exactly right since git uses it.
+func TestSetsCorrectCommandInGitConfig(t *testing.T) {
 	withIsolatedGitConfig(t)
-	configureTestCredentialHelper(t, "credential.helper")
-	configureTestCredentialHelper(t, "credential.https://github.com.helper")
 
-	// When we configure ourselves as the git credential helper
-	hc := gitcredentials.HelperConfig{
+	gc := &git.Client{}
+	hc := &gitcredentials.HelperConfig{
 		SelfExecutablePath: "/path/to/gh",
-		GitClient:          &git.Client{},
+		GitClient:          gc,
 	}
 	require.NoError(t, hc.ConfigureOurs("github.com"))
 
-	// Then the git config should be updated to use our credential repoHelper for both repos and gists
-	repoHelper, err := hc.ConfiguredHelper("github.com")
+	// Check that the correct command was set in the git config
+	cmd, err := gc.Command(context.Background(), "config", "--get", "credential.https://github.com.helper")
 	require.NoError(t, err)
-	require.True(t, repoHelper.IsConfigured(), "expected our helper to be configured")
-	require.True(t, repoHelper.IsOurs(), "expected the helper to be ours but was %q", repoHelper.Cmd)
-
-	gistHelper, err := hc.ConfiguredHelper("gist.github.com")
-	require.NoError(t, err)
-	require.True(t, gistHelper.IsConfigured(), "expected our helper to be configured")
-	require.True(t, gistHelper.IsOurs(), "expected the helper to be ours but was %q", gistHelper.Cmd)
-}
-
-func TestConfiguredHelperNoPreexistingHelpersConfigured(t *testing.T) {
-	// Given there are no credential helpers configured
-	withIsolatedGitConfig(t)
-
-	// When we check the configured helper for a hostname
-	hc := gitcredentials.HelperConfig{
-		SelfExecutablePath: "/path/to/gh",
-		GitClient:          &git.Client{},
-	}
-	helper, err := hc.ConfiguredHelper("github.com")
-
-	// Then the helper should not be configured and not ours
-	require.NoError(t, err)
-	require.False(t, helper.IsConfigured(), "expected no helper to be configured")
-	require.False(t, helper.IsOurs(), "expected the helper not to be ours but was %q", helper.Cmd)
-}
-
-func TestConfiguredHelperWithPreexistingGlobalHelperConfigured(t *testing.T) {
-	// Given there is a global credential helper configured
-	withIsolatedGitConfig(t)
-	configureTestCredentialHelper(t, "credential.helper")
-
-	// When we check the configured helper for a hostname
-	hc := gitcredentials.HelperConfig{
-		SelfExecutablePath: "/path/to/gh",
-		GitClient:          &git.Client{},
-	}
-	helper, err := hc.ConfiguredHelper("github.com")
-
-	// Then the helper should be configured, but not ours
-	require.NoError(t, err)
-	require.True(t, helper.IsConfigured(), "expected no helper to be configured")
-	require.False(t, helper.IsOurs(), "expected the helper not to be ours but was %q", helper.Cmd)
-}
-
-func TestConfiguredHelperWithPreexistingHostHelperConfigured(t *testing.T) {
-	// Given there is a credential helper configured for a hostname
-	withIsolatedGitConfig(t)
-	configureTestCredentialHelper(t, "credential.https://github.com.helper")
-
-	// When we check the configured helper for a hostname
-	hc := gitcredentials.HelperConfig{
-		SelfExecutablePath: "/path/to/gh",
-		GitClient:          &git.Client{},
-	}
-	helper, err := hc.ConfiguredHelper("github.com")
-
-	// Then the helper should be configured, but not ours
+	output, err := cmd.Output()
 	require.NoError(t, err)
-	require.True(t, helper.IsConfigured(), "expected no helper to be configured")
-	require.False(t, helper.IsOurs(), "expected the helper not to be ours but was %q", helper.Cmd)
+	require.Equal(t, "!/path/to/gh auth git-credential\n", string(output))
 }
 
 func TestHelperIsOurs(t *testing.T) {
diff --git a/pkg/cmd/auth/shared/login_flow_test.go b/pkg/cmd/auth/shared/login_flow_test.go
index b9396a1a1..8c8ba5d72 100644
--- a/pkg/cmd/auth/shared/login_flow_test.go
+++ b/pkg/cmd/auth/shared/login_flow_test.go
@@ -10,10 +10,12 @@ import (
 	"github.com/MakeNowJust/heredoc"
 	"github.com/cli/cli/v2/internal/prompter"
 	"github.com/cli/cli/v2/internal/run"
+	"github.com/cli/cli/v2/pkg/cmd/auth/shared/gitcredentials"
 	"github.com/cli/cli/v2/pkg/httpmock"
 	"github.com/cli/cli/v2/pkg/iostreams"
 	"github.com/cli/cli/v2/pkg/ssh"
 	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 )
 
 type tinyConfig map[string]string
@@ -255,6 +257,11 @@ func TestLogin(t *testing.T) {
 			tt.opts.IO = ios
 			tt.opts.Config = &cfg
 			tt.opts.HTTPClient = &http.Client{Transport: reg}
+			tt.opts.CredentialFlow = &GitCredentialFlow{
+				// Intentionally not instantiating anything in here because the tests do not hit this code path.
+				// Right now it's better to panic if we write a test that hits the code than say, start calling
+				// out to git unintentionally.
+			}
 
 			if tt.runStubs != nil {
 				rs, runRestore := run.Stub()
@@ -282,6 +289,61 @@ func TestLogin(t *testing.T) {
 	}
 }
 
+func TestAuthenticatingGitCredentials(t *testing.T) {
+	// Given we have no host or global credential helpers configured
+	// And given they have chosen https as their git protocol
+	// When they choose to authenticate git with their GitHub credentials
+	// Then gh is configured as their credential helper for that host
+	ios, _, _, _ := iostreams.Test()
+
+	reg := &httpmock.Registry{}
+	defer reg.Verify(t)
+	reg.Register(
+		httpmock.REST("GET", "api/v3/"),
+		httpmock.ScopesResponder("repo,read:org"))
+	reg.Register(
+		httpmock.GraphQL(`query UserCurrent\b`),
+		httpmock.StringResponse(`{"data":{"viewer":{ "login": "monalisa" }}}`))
+
+	opts := &LoginOptions{
+		IO:          ios,
+		Config:      tinyConfig{},
+		HTTPClient:  &http.Client{Transport: reg},
+		Hostname:    "example.com",
+		Interactive: true,
+		GitProtocol: "https",
+		Prompter: &prompter.PrompterMock{
+			SelectFunc: func(prompt, _ string, opts []string) (int, error) {
+				if prompt == "How would you like to authenticate GitHub CLI?" {
+					return prompter.IndexFor(opts, "Paste an authentication token")
+				}
+				return -1, prompter.NoSuchPromptErr(prompt)
+			},
+			AuthTokenFunc: func() (string, error) {
+				return "ATOKEN", nil
+			},
+		},
+		CredentialFlow: &GitCredentialFlow{
+			Prompter: &prompter.PrompterMock{
+				ConfirmFunc: func(prompt string, _ bool) (bool, error) {
+					return true, nil
+				},
+			},
+			HelperConfig: &gitcredentials.FakeHelperConfig{
+				SelfExecutablePath: "/path/to/gh",
+				Helpers:            map[string]gitcredentials.Helper{},
+			},
+			// Updater not required for this test as we will be setting gh as the helper
+		},
+	}
+
+	require.NoError(t, Login(opts))
+
+	helper, err := opts.CredentialFlow.HelperConfig.ConfiguredHelper("example.com")
+	require.NoError(t, err)
+	require.True(t, helper.IsOurs(), "expected gh to be the configured helper")
+}
+
 func Test_scopesSentence(t *testing.T) {
 	type args struct {
 		scopes []string