From a55f50b1154ee391f991d6677949c1091c3970d5 Mon Sep 17 00:00:00 2001 From: Andrew Feller Date: Sun, 8 Dec 2024 20:01:26 -0500 Subject: [PATCH] Remove Extension.FullName() from interface - this was something I did in the original implementation of this improved extension update notification logic - discovering other parts of the extension manager code that were working with non-prefixed extension names motivated a different approach - the extension manager code that requires the extension be prefixed has been enhanced to use the centralized ensurePrefixed() logic, making the need for this on the extension unnecessary --- cmd/gen-docs/main.go | 4 ++++ internal/update/update.go | 2 +- internal/update/update_test.go | 6 ++--- pkg/cmd/extension/manager.go | 14 ++++-------- pkg/cmd/extension/manager_test.go | 11 +-------- pkg/cmd/root/extension_test.go | 3 --- pkg/extensions/extension.go | 5 ++--- pkg/extensions/extension_mock.go | 37 ------------------------------- 8 files changed, 14 insertions(+), 68 deletions(-) diff --git a/cmd/gen-docs/main.go b/cmd/gen-docs/main.go index 0c9590fac..60fd8af58 100644 --- a/cmd/gen-docs/main.go +++ b/cmd/gen-docs/main.go @@ -127,3 +127,7 @@ func (e *em) Create(_ string, _ extensions.ExtTemplateType) error { } func (e *em) EnableDryRunMode() {} + +func (e *em) UpdateDir(_ string) string { + return "" +} diff --git a/internal/update/update.go b/internal/update/update.go index 6010689c7..f9a97917a 100644 --- a/internal/update/update.go +++ b/internal/update/update.go @@ -48,7 +48,7 @@ func CheckForExtensionUpdate(em extensions.ExtensionManager, ext extensions.Exte return nil, nil } - stateFilePath := filepath.Join(em.UpdateDir(ext.FullName()), "state.yml") // TODO: See what removing FullName() changes + stateFilePath := filepath.Join(em.UpdateDir(ext.Name()), "state.yml") stateEntry, _ := getStateEntry(stateFilePath) if stateEntry != nil && now.Sub(stateEntry.CheckedForUpdateAt).Hours() < 24 { return nil, nil diff --git a/internal/update/update_test.go b/internal/update/update_test.go index 6f7db6e70..28a31aa41 100644 --- a/internal/update/update_test.go +++ b/internal/update/update_test.go @@ -203,6 +203,7 @@ func TestCheckForExtensionUpdate(t *testing.T) { expectedReleaseInfo: nil, }, // TODO: Local extension with no previous state entry + // TODO: Capture git and binary specific entries } for _, tt := range tests { @@ -218,9 +219,6 @@ func TestCheckForExtensionUpdate(t *testing.T) { NameFunc: func() string { return "extension-update-test" }, - FullNameFunc: func() string { - return "gh-extension-update-test" - }, CurrentVersionFunc: func() string { return tt.extCurrentVersion }, @@ -248,7 +246,7 @@ func TestCheckForExtensionUpdate(t *testing.T) { } // Setup previous state file for test as necessary - stateFilePath := filepath.Join(em.UpdateDir(ext.FullName()), "state.yml") + stateFilePath := filepath.Join(em.UpdateDir(ext.Name()), "state.yml") if tt.previousStateEntry != nil { require.NoError(t, setStateEntry(stateFilePath, tt.previousStateEntry.CheckedForUpdateAt, tt.previousStateEntry.LatestRelease)) } diff --git a/pkg/cmd/extension/manager.go b/pkg/cmd/extension/manager.go index 4670e120f..ccb4b9bfa 100644 --- a/pkg/cmd/extension/manager.go +++ b/pkg/cmd/extension/manager.go @@ -542,10 +542,7 @@ func (m *Manager) upgradeBinExtension(ext *Extension) error { } func (m *Manager) Remove(name string) error { - name, err := ensurePrefixed(name) - if err != nil { - return err - } + name = ensurePrefixed(name) targetDir := filepath.Join(m.installDir(), name) if _, err := os.Lstat(targetDir); os.IsNotExist(err) { return fmt.Errorf("no extension found: %q", targetDir) @@ -565,7 +562,7 @@ func (m *Manager) installDir() string { // UpdateDir returns the extension-specific directory where updates are stored. func (m *Manager) UpdateDir(name string) string { - return filepath.Join(m.updateDir(), name) + return filepath.Join(m.updateDir(), ensurePrefixed(name)) } //go:embed ext_tmpls/goBinMain.go.txt @@ -842,12 +839,9 @@ func (m *Manager) cleanExtensionUpdateDir(name string) error { } // ensurePrefixed just makes sure that the provided extension name is prefixed with "gh-". -func ensurePrefixed(name string) (string, error) { - if name == "" { - return "", errors.New("failed prefixing extension name: must not be empty") - } +func ensurePrefixed(name string) string { if !strings.HasPrefix(name, "gh-") { name = "gh-" + name } - return name, nil + return name } diff --git a/pkg/cmd/extension/manager_test.go b/pkg/cmd/extension/manager_test.go index 9564fc6fa..af8264639 100644 --- a/pkg/cmd/extension/manager_test.go +++ b/pkg/cmd/extension/manager_test.go @@ -1331,20 +1331,11 @@ func Test_ensurePrefixed(t *testing.T) { input: "gh-purrfect", expected: "gh-purrfect", }, - { - name: "empty string", - wantErr: true, - }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - actual, err := ensurePrefixed(tt.input) - if tt.wantErr { - require.Error(t, err) - } else { - require.Equal(t, tt.expected, actual) - } + require.Equal(t, tt.expected, ensurePrefixed(tt.input)) }) } } diff --git a/pkg/cmd/root/extension_test.go b/pkg/cmd/root/extension_test.go index e71c63317..1d74c1297 100644 --- a/pkg/cmd/root/extension_test.go +++ b/pkg/cmd/root/extension_test.go @@ -137,9 +137,6 @@ func TestNewCmdExtension_Updates(t *testing.T) { CurrentVersionFunc: func() string { return tt.extCurrentVersion }, - FullNameFunc: func() string { - return tt.extFullName - }, IsPinnedFunc: func() bool { return tt.extIsPinned }, diff --git a/pkg/extensions/extension.go b/pkg/extensions/extension.go index db6c75098..1b9c34612 100644 --- a/pkg/extensions/extension.go +++ b/pkg/extensions/extension.go @@ -16,9 +16,8 @@ const ( //go:generate moq -rm -out extension_mock.go . Extension type Extension interface { - Name() string // Extension Name without gh- - FullName() string // Extension Name with gh- - Path() string // Path to executable + Name() string // Extension Name without gh- + Path() string // Path to executable URL() string CurrentVersion() string LatestVersion() string diff --git a/pkg/extensions/extension_mock.go b/pkg/extensions/extension_mock.go index 2488c44eb..9b473e20a 100644 --- a/pkg/extensions/extension_mock.go +++ b/pkg/extensions/extension_mock.go @@ -20,9 +20,6 @@ var _ Extension = &ExtensionMock{} // CurrentVersionFunc: func() string { // panic("mock out the CurrentVersion method") // }, -// FullNameFunc: func() string { -// panic("mock out the FullName method") -// }, // IsBinaryFunc: func() bool { // panic("mock out the IsBinary method") // }, @@ -60,9 +57,6 @@ type ExtensionMock struct { // CurrentVersionFunc mocks the CurrentVersion method. CurrentVersionFunc func() string - // FullNameFunc mocks the FullName method. - FullNameFunc func() string - // IsBinaryFunc mocks the IsBinary method. IsBinaryFunc func() bool @@ -95,9 +89,6 @@ type ExtensionMock struct { // CurrentVersion holds details about calls to the CurrentVersion method. CurrentVersion []struct { } - // FullName holds details about calls to the FullName method. - FullName []struct { - } // IsBinary holds details about calls to the IsBinary method. IsBinary []struct { } @@ -127,7 +118,6 @@ type ExtensionMock struct { } } lockCurrentVersion sync.RWMutex - lockFullName sync.RWMutex lockIsBinary sync.RWMutex lockIsLocal sync.RWMutex lockIsPinned sync.RWMutex @@ -166,33 +156,6 @@ func (mock *ExtensionMock) CurrentVersionCalls() []struct { return calls } -// FullName calls FullNameFunc. -func (mock *ExtensionMock) FullName() string { - if mock.FullNameFunc == nil { - panic("ExtensionMock.FullNameFunc: method is nil but Extension.FullName was just called") - } - callInfo := struct { - }{} - mock.lockFullName.Lock() - mock.calls.FullName = append(mock.calls.FullName, callInfo) - mock.lockFullName.Unlock() - return mock.FullNameFunc() -} - -// FullNameCalls gets all the calls that were made to FullName. -// Check the length with: -// -// len(mockedExtension.FullNameCalls()) -func (mock *ExtensionMock) FullNameCalls() []struct { -} { - var calls []struct { - } - mock.lockFullName.RLock() - calls = mock.calls.FullName - mock.lockFullName.RUnlock() - return calls -} - // IsBinary calls IsBinaryFunc. func (mock *ExtensionMock) IsBinary() bool { if mock.IsBinaryFunc == nil {