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.
This commit is contained in:
Andy Feller 2024-12-13 23:09:18 -05:00
parent 5ad6ccf73a
commit bfc63a14cc
5 changed files with 37 additions and 100 deletions

View file

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

View file

@ -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")
}

View file

@ -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: &reg}, 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) {

View file

@ -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")
}

View file

@ -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",