Add HelperConfig contract test and FakeHelperConfig

pkg/cmd/auth/shared/contract/helper_config.go

@@ -0,0 +1,59 @@
+package contract
+
+import (
+	"testing"
+
+	"github.com/cli/cli/v2/pkg/cmd/auth/shared"
+	"github.com/stretchr/testify/require"
+)
+
+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)
+	})
+}

pkg/cmd/auth/shared/git_credential.go

@@ -7,6 +7,11 @@ 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
 

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
+}

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)
+}

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
+func TestHelperConfigContract(t *testing.T) {
+	contract.HelperConfig{
+		NewHelperConfig: func(t *testing.T) shared.HelperConfig {
+			withIsolatedGitConfig(t)
+
+			return &gitcredentials.HelperConfig{
+				SelfExecutablePath: "/path/to/gh",
+				GitClient:          &git.Client{},
+			}
+		},
+		ConfigureHelper: func(t *testing.T, hostname string) {
+			configureTestCredentialHelper(t, hostname)
+		},
+	}.Test(t)
+}
+
+// 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)
 
-	// 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 helper 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")
+	output, err := cmd.Output()
 	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 TestConfigureOursWithPreexistingHelpersConfigured(t *testing.T) {
-	// Given there are other credential helpers configured
-	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{
-		SelfExecutablePath: "/path/to/gh",
-		GitClient:          &git.Client{},
-	}
-	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")
-	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
-	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) {