From bfc63a14cc2cc0da8665387e65f2b0c6b55fe803 Mon Sep 17 00:00:00 2001 From: Andy Feller Date: Fri, 13 Dec 2024 23:09:18 -0500 Subject: [PATCH] Implement remaining PR feedback After merging in upstream changes regarding local extensions, this commit addresses remaining PR feedback while also bringing the newly merged tests into alignment with other changes. --- internal/update/update_test.go | 4 +- pkg/cmd/extension/manager_other_test.go | 36 ---------------- pkg/cmd/extension/manager_test.go | 51 ++++++++++++++++------- pkg/cmd/extension/manager_windows_test.go | 38 ----------------- pkg/cmd/root/extension_test.go | 8 ---- 5 files changed, 37 insertions(+), 100 deletions(-) delete mode 100644 pkg/cmd/extension/manager_other_test.go delete mode 100644 pkg/cmd/extension/manager_windows_test.go diff --git a/internal/update/update_test.go b/internal/update/update_test.go index e40a09c0c..ebddee810 100644 --- a/internal/update/update_test.go +++ b/internal/update/update_test.go @@ -368,7 +368,7 @@ func TestCheckForExtensionUpdate(t *testing.T) { } } -func TestShouldCheckForExtensionUpdate(t *testing.T) { +func TestShouldCheckForUpdate(t *testing.T) { tests := []struct { name string env map[string]string @@ -419,7 +419,7 @@ func TestShouldCheckForExtensionUpdate(t *testing.T) { os.Setenv(k, v) } - actual := ShouldCheckForExtensionUpdate() + actual := ShouldCheckForUpdate() require.Equal(t, tt.expected, actual) }) } diff --git a/pkg/cmd/extension/manager_other_test.go b/pkg/cmd/extension/manager_other_test.go deleted file mode 100644 index 416545c4b..000000000 --- a/pkg/cmd/extension/manager_other_test.go +++ /dev/null @@ -1,36 +0,0 @@ -//go:build !windows - -package extension - -import ( - "os" - "path/filepath" - "testing" - - "github.com/cli/cli/v2/pkg/iostreams" - "github.com/stretchr/testify/require" -) - -func TestManager_InstallLocal(t *testing.T) { - dataDir := t.TempDir() - updateDir := t.TempDir() - - extDir := filepath.Join(t.TempDir(), "gh-local") - err := os.MkdirAll(extDir, 0755) - require.NoErrorf(t, err, "failed to create local extension") - require.NoError(t, stubExtensionUpdate(filepath.Join(updateDir, "gh-local"))) - - ios, _, stdout, stderr := iostreams.Test() - m := newTestManager(dataDir, updateDir, nil, nil, ios) - - err = m.InstallLocal(extDir) - require.NoError(t, err) - require.Equal(t, "", stdout.String()) - require.Equal(t, "", stderr.String()) - - fm, err := os.Lstat(filepath.Join(dataDir, "extensions", "gh-local")) - require.NoErrorf(t, err, "data directory missing local extension symlink") - isSymlink := fm.Mode() & os.ModeSymlink - require.True(t, isSymlink != 0) - require.NoDirExistsf(t, filepath.Join(updateDir, "gh-local"), "update directory should be removed") -} diff --git a/pkg/cmd/extension/manager_test.go b/pkg/cmd/extension/manager_test.go index f8770eea5..53ea6b111 100644 --- a/pkg/cmd/extension/manager_test.go +++ b/pkg/cmd/extension/manager_test.go @@ -752,26 +752,31 @@ func TestManager_UpgradeExtension_GitExtension_Pinned(t *testing.T) { } func TestManager_Install_local(t *testing.T) { - extManagerDir := t.TempDir() + dataDir := t.TempDir() + updateDir := t.TempDir() ios, _, stdout, stderr := iostreams.Test() - m := newTestManager(extManagerDir, nil, nil, ios) + m := newTestManager(dataDir, updateDir, nil, nil, ios) fakeExtensionName := "local-ext" // Create a temporary directory to simulate the local extension repo - extensionLocalPath := filepath.Join(extManagerDir, fakeExtensionName) + extensionLocalPath := filepath.Join(dataDir, fakeExtensionName) require.NoError(t, os.MkdirAll(extensionLocalPath, 0755)) // Create a fake executable in the local extension directory fakeExtensionExecutablePath := filepath.Join(extensionLocalPath, fakeExtensionName) require.NoError(t, stubExtension(fakeExtensionExecutablePath)) + // Create a temporary directory to simulate the local extension update state + extensionUpdatePath := filepath.Join(updateDir, fakeExtensionName) + require.NoError(t, stubExtensionUpdate(extensionUpdatePath)) + err := m.InstallLocal(extensionLocalPath) require.NoError(t, err) // This is the path to a file: // on windows this is a file whose contents is a string describing the path to the local extension dir. // on other platforms this file is a real symlink to the local extension dir. - extensionLinkFile := filepath.Join(extManagerDir, "extensions", fakeExtensionName) + extensionLinkFile := filepath.Join(dataDir, "extensions", fakeExtensionName) if runtime.GOOS == "windows" { // We don't create true symlinks on Windows, so check if we made a @@ -787,18 +792,24 @@ func TestManager_Install_local(t *testing.T) { } assert.Equal(t, "", stdout.String()) assert.Equal(t, "", stderr.String()) + require.NoDirExistsf(t, extensionUpdatePath, "update directory should be removed") } func TestManager_Install_local_no_executable_found(t *testing.T) { - tempDir := t.TempDir() + dataDir := t.TempDir() + updateDir := t.TempDir() ios, _, stdout, stderr := iostreams.Test() - m := newTestManager(tempDir, nil, nil, ios) + m := newTestManager(dataDir, updateDir, nil, nil, ios) fakeExtensionName := "local-ext" // Create a temporary directory to simulate the local extension repo - localDir := filepath.Join(tempDir, fakeExtensionName) + localDir := filepath.Join(dataDir, fakeExtensionName) require.NoError(t, os.MkdirAll(localDir, 0755)) + // Create a temporary directory to simulate the local extension update state + extensionUpdatePath := filepath.Join(updateDir, fakeExtensionName) + require.NoError(t, stubExtensionUpdate(extensionUpdatePath)) + // Intentionally not creating an executable in the local extension repo // to simulate an attempt to install a local extension without an executable @@ -806,6 +817,7 @@ func TestManager_Install_local_no_executable_found(t *testing.T) { require.ErrorAs(t, err, new(*ErrExtensionExecutableNotFound)) assert.Equal(t, "", stdout.String()) assert.Equal(t, "", stderr.String()) + require.NoDirExistsf(t, extensionUpdatePath, "update directory should be removed") } func TestManager_Install_git(t *testing.T) { @@ -818,10 +830,14 @@ func TestManager_Install_git(t *testing.T) { ios, _, stdout, stderr := iostreams.Test() - extensionDir := filepath.Join(dataDir, "extensions", "gh-some-ext") + fakeExtensionName := "gh-some-ext" + extensionDir := filepath.Join(dataDir, "extensions", fakeExtensionName) gc := &mockGitClient{} gc.On("Clone", "https://github.com/owner/gh-some-ext.git", []string{extensionDir}).Return("", nil).Once() - assert.NoError(t, stubExtensionUpdate(filepath.Join(updateDir, "gh-some-ext"))) + + // Create a temporary directory to simulate the local extension update state + extensionUpdatePath := filepath.Join(updateDir, fakeExtensionName) + require.NoError(t, stubExtensionUpdate(extensionUpdatePath)) m := newTestManager(dataDir, updateDir, &client, gc, ios) @@ -840,7 +856,7 @@ func TestManager_Install_git(t *testing.T) { httpmock.REST("GET", "repos/owner/gh-some-ext/contents/gh-some-ext"), httpmock.StringResponse("script")) - repo := ghrepo.New("owner", "gh-some-ext") + repo := ghrepo.New("owner", fakeExtensionName) err := m.Install(repo, "") assert.NoError(t, err) @@ -848,7 +864,7 @@ func TestManager_Install_git(t *testing.T) { assert.Equal(t, "", stderr.String()) gc.AssertExpectations(t) - assert.NoDirExistsf(t, filepath.Join(updateDir, "gh-some-ext"), "update directory should be removed") + assert.NoDirExistsf(t, extensionUpdatePath, "update directory should be removed") } func TestManager_Install_git_pinned(t *testing.T) { @@ -1062,7 +1078,8 @@ func TestManager_Install_rosetta_fallback_not_found(t *testing.T) { } func TestManager_Install_binary(t *testing.T) { - repo := ghrepo.NewWithHost("owner", "gh-bin-ext", "example.com") + fakeExtensionName := "gh-bin-ext" + repo := ghrepo.NewWithHost("owner", fakeExtensionName, "example.com") reg := httpmock.Registry{} defer reg.Verify(t) @@ -1097,7 +1114,10 @@ func TestManager_Install_binary(t *testing.T) { ios, _, stdout, stderr := iostreams.Test() dataDir := t.TempDir() updateDir := t.TempDir() - assert.NoError(t, stubExtensionUpdate(filepath.Join(updateDir, "gh-bin-ext"))) + + // Create a temporary directory to simulate the local extension update state + extensionUpdatePath := filepath.Join(updateDir, fakeExtensionName) + require.NoError(t, stubExtensionUpdate(extensionUpdatePath)) m := newTestManager(dataDir, updateDir, &http.Client{Transport: ®}, nil, ios) @@ -1112,7 +1132,7 @@ func TestManager_Install_binary(t *testing.T) { assert.NoError(t, err) assert.Equal(t, binManifest{ - Name: "gh-bin-ext", + Name: fakeExtensionName, Owner: "owner", Host: "example.com", Tag: "v1.0.1", @@ -1125,8 +1145,7 @@ func TestManager_Install_binary(t *testing.T) { assert.Equal(t, "", stdout.String()) assert.Equal(t, "", stderr.String()) - - assert.NoDirExistsf(t, filepath.Join(updateDir, "gh-bin-ext"), "update directory should be removed") + require.NoDirExistsf(t, extensionUpdatePath, "update directory should be removed") } func TestManager_Install_amd64_when_supported(t *testing.T) { diff --git a/pkg/cmd/extension/manager_windows_test.go b/pkg/cmd/extension/manager_windows_test.go deleted file mode 100644 index f9142f4fc..000000000 --- a/pkg/cmd/extension/manager_windows_test.go +++ /dev/null @@ -1,38 +0,0 @@ -package extension - -import ( - "os" - "path/filepath" - "testing" - - "github.com/cli/cli/v2/pkg/iostreams" - "github.com/stretchr/testify/require" -) - -func TestManager_InstallLocal_Windows(t *testing.T) { - dataDir := t.TempDir() - updateDir := t.TempDir() - - extDir := filepath.Join(t.TempDir(), "gh-local") - err := os.MkdirAll(extDir, 0755) - require.NoErrorf(t, err, "failed to create local extension") - require.NoError(t, stubExtensionUpdate(filepath.Join(updateDir, "gh-local"))) - - ios, _, stdout, stderr := iostreams.Test() - m := newTestManager(dataDir, updateDir, nil, nil, ios) - - err = m.InstallLocal(extDir) - require.NoError(t, err) - require.Equal(t, "", stdout.String()) - require.Equal(t, "", stderr.String()) - - extDataFile := filepath.Join(dataDir, "extensions", "gh-local") - fm, err := os.Stat(extDataFile) - require.NoErrorf(t, err, "data directory missing Windows local extension file") - isSymlink := fm.Mode() & os.ModeSymlink - require.True(t, isSymlink == 0) - extDataFilePath, err := readPathFromFile(extDataFile) - require.NoErrorf(t, err, "failed to read Windows local extension path file") - require.Equal(t, extDir, extDataFilePath) - require.NoDirExistsf(t, filepath.Join(updateDir, "gh-local"), "update directory should be removed") -} diff --git a/pkg/cmd/root/extension_test.go b/pkg/cmd/root/extension_test.go index 1d74c1297..32b250de5 100644 --- a/pkg/cmd/root/extension_test.go +++ b/pkg/cmd/root/extension_test.go @@ -17,7 +17,6 @@ func TestNewCmdExtension_Updates(t *testing.T) { tests := []struct { name string extCurrentVersion string - extFullName string extIsPinned bool extLatestVersion string extName string @@ -28,7 +27,6 @@ func TestNewCmdExtension_Updates(t *testing.T) { { name: "no update available", extName: "no-update", - extFullName: "gh-no-update", extUpdateAvailable: false, extCurrentVersion: "1.0.0", extLatestVersion: "1.0.0", @@ -37,7 +35,6 @@ func TestNewCmdExtension_Updates(t *testing.T) { { name: "major update", extName: "major-update", - extFullName: "gh-major-update", extUpdateAvailable: true, extCurrentVersion: "1.0.0", extLatestVersion: "2.0.0", @@ -51,7 +48,6 @@ func TestNewCmdExtension_Updates(t *testing.T) { { name: "major update, pinned", extName: "major-update-pin", - extFullName: "gh-major-update-pin", extUpdateAvailable: true, extCurrentVersion: "1.0.0", extLatestVersion: "2.0.0", @@ -66,7 +62,6 @@ func TestNewCmdExtension_Updates(t *testing.T) { { name: "minor update", extName: "minor-update", - extFullName: "gh-minor-update", extUpdateAvailable: true, extCurrentVersion: "1.0.0", extLatestVersion: "1.1.0", @@ -80,7 +75,6 @@ func TestNewCmdExtension_Updates(t *testing.T) { { name: "minor update, pinned", extName: "minor-update-pin", - extFullName: "gh-minor-update-pin", extUpdateAvailable: true, extCurrentVersion: "1.0.0", extLatestVersion: "1.1.0", @@ -95,7 +89,6 @@ func TestNewCmdExtension_Updates(t *testing.T) { { name: "patch update", extName: "patch-update", - extFullName: "gh-patch-update", extUpdateAvailable: true, extCurrentVersion: "1.0.0", extLatestVersion: "1.0.1", @@ -109,7 +102,6 @@ func TestNewCmdExtension_Updates(t *testing.T) { { name: "patch update, pinned", extName: "patch-update-pin", - extFullName: "gh-patch-update-pin", extUpdateAvailable: true, extCurrentVersion: "1.0.0", extLatestVersion: "1.0.1",