Make ssh tests behavioural with a shared fake ssh

internal/testutils/ssh_stub.go

@@ -0,0 +1,133 @@
+package testutils
+
+import (
+	"encoding/json"
+	"fmt"
+	"io"
+	"os"
+	"path/filepath"
+	"runtime"
+	"strings"
+	"testing"
+
+	"github.com/stretchr/testify/require"
+)
+
+type sshResponseKind string
+
+const (
+	sshReportsHostname   sshResponseKind = "hostname"
+	sshReportsNoHostname sshResponseKind = "no-hostname"
+	sshFails             sshResponseKind = "fails"
+)
+
+// SSHResponse is how the fake ssh installed by StubSSH answers `ssh -G` for a host.
+// Build one with SSHReportsHostname, SSHReportsNoHostname, or SSHFails. The zero value is invalid.
+type SSHResponse struct {
+	kind     sshResponseKind
+	hostname string
+}
+
+// SSHReportsHostname makes the fake ssh report hostname for a host.
+func SSHReportsHostname(hostname string) SSHResponse {
+	return SSHResponse{kind: sshReportsHostname, hostname: hostname}
+}
+
+// SSHReportsNoHostname makes the fake ssh succeed without reporting a hostname for a host.
+func SSHReportsNoHostname() SSHResponse {
+	return SSHResponse{kind: sshReportsNoHostname}
+}
+
+// SSHFails makes the fake ssh exit with an error for a host. It still prints a hostname first,
+// so callers must honour the exit status rather than the output.
+func SSHFails() SSHResponse {
+	return SSHResponse{kind: sshFails}
+}
+
+type wireSSHResponse struct {
+	Kind     sshResponseKind `json:"kind"`
+	Hostname string          `json:"hostname,omitempty"`
+}
+
+const failingSSHHostname = "failed-ssh.invalid"
+
+const fakeSSHResponsesEnv = "GO_GH_TEST_FAKE_SSH_RESPONSES"
+
+// StubSSH puts a fake ssh first on PATH. Its `ssh -G HOST` answers with responses[HOST], or reports
+// HOST itself when unmapped, like ssh without a matching config. The fake is the running test binary,
+// so the test package's TestMain must call RunFakeSSHIfRequested.
+func StubSSH(t *testing.T, responses map[string]SSHResponse) {
+	t.Helper()
+	wire := make(map[string]wireSSHResponse, len(responses))
+	for host, r := range responses {
+		require.NotEmpty(t, r.kind, "fake ssh response for %q must be built with an SSH* constructor", host)
+		wire[host] = wireSSHResponse{Kind: r.kind, Hostname: r.hostname}
+	}
+	encoded, err := json.Marshal(wire)
+	require.NoError(t, err, "encoding fake ssh responses")
+	dir := t.TempDir()
+	name := "ssh"
+	if runtime.GOOS == "windows" {
+		name += ".exe"
+	}
+	copyTestBinary(t, filepath.Join(dir, name))
+	t.Setenv(fakeSSHResponsesEnv, string(encoded))
+	t.Setenv("PATH", dir+string(os.PathListSeparator)+os.Getenv("PATH"))
+}
+
+// RunFakeSSHIfRequested acts as `ssh -G` and exits when this test binary was started as the fake ssh
+// installed by StubSSH, that is, when it runs under the name ssh with the fake's responses set.
+// Otherwise it returns, so other re-execs of the test binary are unaffected. Call it first in TestMain.
+func RunFakeSSHIfRequested() {
+	if strings.TrimSuffix(filepath.Base(os.Args[0]), ".exe") != "ssh" {
+		return
+	}
+	encoded, ok := os.LookupEnv(fakeSSHResponsesEnv)
+	if !ok {
+		return
+	}
+	var responses map[string]wireSSHResponse
+	if err := json.Unmarshal([]byte(encoded), &responses); err != nil {
+		fmt.Fprintf(os.Stderr, "fake ssh: decoding responses: %v\n", err)
+		os.Exit(1)
+	}
+	args := os.Args[1:]
+	if len(args) != 2 || args[0] != "-G" {
+		fmt.Fprintf(os.Stderr, "fake ssh: unexpected arguments %q\n", args)
+		os.Exit(1)
+	}
+	host := args[1]
+	response, mapped := responses[host]
+	if !mapped {
+		fmt.Printf("hostname %s\n", host)
+		os.Exit(0)
+	}
+	switch response.Kind {
+	case sshReportsHostname:
+		fmt.Printf("hostname %s\n", response.Hostname)
+	case sshReportsNoHostname:
+	case sshFails:
+		fmt.Printf("hostname %s\n", failingSSHHostname)
+		fmt.Fprintf(os.Stderr, "fake ssh: failing for %s\n", host)
+		os.Exit(1)
+	default:
+		fmt.Fprintf(os.Stderr, "fake ssh: unknown response kind %q for %s\n", response.Kind, host)
+		os.Exit(1)
+	}
+	os.Exit(0)
+}
+
+func copyTestBinary(t *testing.T, dst string) {
+	t.Helper()
+	src, err := os.Executable()
+	require.NoError(t, err, "locating test binary")
+	in, err := os.Open(src)
+	require.NoError(t, err, "opening test binary")
+	defer in.Close()
+	out, err := os.OpenFile(dst, os.O_CREATE|os.O_WRONLY, 0o755)
+	require.NoError(t, err, "creating fake ssh")
+	_, err = io.Copy(out, in)
+	closeErr := out.Close()
+	require.NoError(t, err, "copying test binary")
+	require.NoError(t, closeErr, "closing fake ssh")
+}

pkg/ssh/ssh_test.go

@@ -1,163 +1,149 @@
 package ssh
 
 import (
-	"errors"
-	"fmt"
 	"net/url"
 	"os"
 	"os/exec"
+	"path/filepath"
 	"testing"
 
 	"github.com/MakeNowJust/heredoc"
+	"github.com/cli/go-gh/v2/internal/testutils"
 	"github.com/cli/safeexec"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 )
 
-func TestTranslator(t *testing.T) {
+func TestMain(m *testing.M) {
+	testutils.RunFakeSSHIfRequested()
+	os.Exit(m.Run())
+}
+
+func TestTranslatorWithRealSSH(t *testing.T) {
 	if _, err := safeexec.LookPath("ssh"); err != nil {
 		t.Skip("no ssh found on system")
 	}
 
 	tests := []struct {
 		name      string
 		sshConfig string
-		arg       string
+		input     string
 		want      string
 	}{
 		{
-			name: "translate SSH URL",
+			name: "translates SSH URL",
 			sshConfig: heredoc.Doc(`
 				Host github-*
 					Hostname github.com
 			`),
-			arg:  "ssh://git@github-foo/owner/repo.git",
-			want: "ssh://git@github.com/owner/repo.git",
+			input: "ssh://git@github-foo/owner/repo.git",
+			want:  "ssh://git@github.com/owner/repo.git",
 		},
 		{
 			name: "does not translate HTTPS URL",
 			sshConfig: heredoc.Doc(`
 				Host github-*
 					Hostname github.com
 			`),
-			arg:  "https://github-foo/owner/repo.git",
-			want: "https://github-foo/owner/repo.git",
+			input: "https://github-foo/owner/repo.git",
+			want:  "https://github-foo/owner/repo.git",
 		},
 		{
 			name: "treats ssh.github.com as github.com",
 			sshConfig: heredoc.Doc(`
 				Host github.com
 					Hostname ssh.github.com
 			`),
-			arg:  "ssh://git@github.com/owner/repo.git",
-			want: "ssh://git@github.com/owner/repo.git",
+			input: "ssh://git@github.com/owner/repo.git",
+			want:  "ssh://git@github.com/owner/repo.git",
 		},
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
-			f, err := os.CreateTemp("", "ssh-config.*")
-			if err != nil {
-				t.Fatalf("error creating file: %v", err)
-			}
-			_, err = f.WriteString(tt.sshConfig)
-			_ = f.Close()
-			if err != nil {
-				t.Fatalf("error writing ssh config: %v", err)
-			}
-
+			// Given an ssh config
+			configPath := filepath.Join(t.TempDir(), "ssh-config")
+			require.NoError(t, os.WriteFile(configPath, []byte(tt.sshConfig), 0o600))
 			tr := &Translator{
 				newCommand: func(exe string, args ...string) *exec.Cmd {
-					args = append([]string{"-F", f.Name()}, args...)
-					return exec.Command(exe, args...)
+					return exec.Command(exe, append([]string{"-F", configPath}, args...)...)
 				},
 			}
-			u, err := url.Parse(tt.arg)
-			if err != nil {
-				t.Fatalf("error parsing URL: %v", err)
-			}
-			res := tr.Translate(u)
-			if got := res.String(); got != tt.want {
-				t.Errorf("expected %q, got %q", tt.want, got)
-			}
+
+			// When a URL is translated
+			got := tr.Translate(mustParseURL(t, tt.input))
+
+			// Then the URL is translated as expected
+			assert.Equal(t, tt.want, got.String())
 		})
 	}
 }
 
-func TestHelperProcess(t *testing.T) {
-	if os.Getenv("GH_WANT_HELPER_PROCESS") != "1" {
-		return
-	}
-	if err := func(args []string) error {
-		if len(args) < 3 || args[2] == "error" {
-			return errors.New("fatal")
-		}
-		if args[2] == "empty.io" {
-			return nil
-		}
-		fmt.Fprintf(os.Stdout, "hostname %s\n", args[2])
-		return nil
-	}(os.Args[3:]); err != nil {
-		fmt.Fprintln(os.Stderr, err)
-		os.Exit(1)
-	}
-	os.Exit(0)
+func TestTranslatorCachesHostname(t *testing.T) {
+	// Given a translator that has already resolved an alias
+	testutils.StubSSH(t, map[string]testutils.SSHResponse{"github-work": testutils.SSHReportsHostname("github.com")})
+	tr := NewTranslator()
+	tr.Translate(mustParseURL(t, "ssh://git@github-work/owner/repo.git"))
+
+	// When ssh would now resolve the alias differently
+	testutils.StubSSH(t, map[string]testutils.SSHResponse{"github-work": testutils.SSHReportsHostname("ghe.example")})
+	got := tr.Translate(mustParseURL(t, "ssh://git@github-work/owner/repo.git"))
+
+	// Then the first resolution is reused
+	assert.Equal(t, "ssh://git@github.com/owner/repo.git", got.String())
 }
 
-func TestTranslator_caching(t *testing.T) {
-	countLookPath := 0
-	countNewCommand := 0
-	tr := &Translator{
-		lookPath: func(s string) (string, error) {
-			countLookPath++
-			return "/path/to/ssh", nil
-		},
-		newCommand: func(exe string, args ...string) *exec.Cmd {
-			args = append([]string{"-test.run=TestHelperProcess", "--", exe}, args...)
-			c := exec.Command(os.Args[0], args...)
-			c.Env = []string{"GH_WANT_HELPER_PROCESS=1"}
-			countNewCommand++
-			return c
-		},
-	}
+func TestTranslatorResolvesEachHostSeparately(t *testing.T) {
+	// Given a translator that has already resolved one alias
+	testutils.StubSSH(t, map[string]testutils.SSHResponse{
+		"github-work":       testutils.SSHReportsHostname("github.com"),
+		"github-enterprise": testutils.SSHReportsHostname("ghe.example"),
+	})
+	tr := NewTranslator()
+	tr.Translate(mustParseURL(t, "ssh://git@github-work/owner/repo.git"))
 
-	tests := []struct {
-		input  string
-		result string
-	}{
-		{
-			input:  "ssh://github1.com/owner/repo.git",
-			result: "github1.com",
-		},
-		{
-			input:  "ssh://github2.com/owner/repo.git",
-			result: "github2.com",
-		},
-		{
-			input:  "ssh://empty.io/owner/repo.git",
-			result: "empty.io",
-		},
-		{
-			input:  "ssh://error/owner/repo.git",
-			result: "error",
-		},
-	}
-	for _, tt := range tests {
-		t.Run(tt.input, func(t *testing.T) {
-			u, err := url.Parse(tt.input)
-			if err != nil {
-				t.Fatalf("error parsing URL: %v", err)
-			}
-			if res := tr.Translate(u); res.Host != tt.result {
-				t.Errorf("expected github.com, got: %q", res.Host)
-			}
-			if res := tr.Translate(u); res.Host != tt.result {
-				t.Errorf("expected github.com, got: %q (second call)", res.Host)
-			}
-		})
-	}
+	// When a URL for a different alias is translated
+	got := tr.Translate(mustParseURL(t, "ssh://git@github-enterprise/owner/repo.git"))
 
-	if countLookPath != 1 {
-		t.Errorf("expected lookPath to happen 1 time; actual: %d", countLookPath)
-	}
-	if countNewCommand != len(tests) {
-		t.Errorf("expected ssh command to shell out %d times; actual: %d", len(tests), countNewCommand)
-	}
+	// Then that alias is resolved on its own
+	assert.Equal(t, "ssh://git@ghe.example/owner/repo.git", got.String())
+}
+
+func TestTranslatorKeepsHostWhenSSHReportsNoHostname(t *testing.T) {
+	// Given ssh reports no hostname for a host
+	testutils.StubSSH(t, map[string]testutils.SSHResponse{"github-work": testutils.SSHReportsNoHostname()})
+
+	// When a URL for that host is translated
+	got := NewTranslator().Translate(mustParseURL(t, "ssh://git@github-work/owner/repo.git"))
+
+	// Then the URL is unchanged
+	assert.Equal(t, "ssh://git@github-work/owner/repo.git", got.String())
+}
+
+func TestTranslatorKeepsHostWhenSSHFails(t *testing.T) {
+	// Given ssh fails for a host
+	testutils.StubSSH(t, map[string]testutils.SSHResponse{"github-work": testutils.SSHFails()})
+
+	// When a URL for that host is translated
+	got := NewTranslator().Translate(mustParseURL(t, "ssh://git@github-work/owner/repo.git"))
+
+	// Then the URL is unchanged
+	assert.Equal(t, "ssh://git@github-work/owner/repo.git", got.String())
+}
+
+func TestTranslatorKeepsHostWhenSSHIsNotInstalled(t *testing.T) {
+	// Given ssh is not on PATH
+	t.Setenv("PATH", t.TempDir())
+
+	// When an SSH URL is translated
+	got := NewTranslator().Translate(mustParseURL(t, "ssh://git@github-work/owner/repo.git"))
+
+	// Then the URL is unchanged
+	assert.Equal(t, "ssh://git@github-work/owner/repo.git", got.String())
+}
+
+func mustParseURL(t *testing.T, s string) *url.URL {
+	t.Helper()
+	u, err := url.Parse(s)
+	require.NoError(t, err)
+	return u
 }