From db5bbf799fde38097889f016bb5447fad2ab2724 Mon Sep 17 00:00:00 2001 From: vilmibm Date: Wed, 22 Sep 2021 16:11:12 -0500 Subject: [PATCH] use manager io in Upgrade --- pkg/cmd/extension/command.go | 2 +- pkg/cmd/extension/command_test.go | 9 ++++---- pkg/cmd/extension/manager.go | 8 +++---- pkg/cmd/extension/manager_test.go | 37 ++++++++++++++----------------- pkg/extensions/extension.go | 2 +- pkg/extensions/manager_mock.go | 36 ++++++++++-------------------- 6 files changed, 39 insertions(+), 55 deletions(-) diff --git a/pkg/cmd/extension/command.go b/pkg/cmd/extension/command.go index ad92a8f14..264945026 100644 --- a/pkg/cmd/extension/command.go +++ b/pkg/cmd/extension/command.go @@ -132,7 +132,7 @@ func NewCmdExtension(f *cmdutil.Factory) *cobra.Command { if len(args) > 0 { name = normalizeExtensionSelector(args[0]) } - return m.Upgrade(name, flagForce, io.Out, io.ErrOut) + return m.Upgrade(name, flagForce) }, } cmd.Flags().BoolVar(&flagAll, "all", false, "Upgrade all extensions") diff --git a/pkg/cmd/extension/command_test.go b/pkg/cmd/extension/command_test.go index fac5d0e9a..789b57b91 100644 --- a/pkg/cmd/extension/command_test.go +++ b/pkg/cmd/extension/command_test.go @@ -1,7 +1,6 @@ package extension import ( - "io" "io/ioutil" "net/http" "os" @@ -94,7 +93,7 @@ func TestNewCmdExtension(t *testing.T) { name: "upgrade an extension", args: []string{"upgrade", "hello"}, managerStubs: func(em *extensions.ExtensionManagerMock) func(*testing.T) { - em.UpgradeFunc = func(name string, force bool, out, errOut io.Writer) error { + em.UpgradeFunc = func(name string, force bool) error { return nil } return func(t *testing.T) { @@ -108,7 +107,7 @@ func TestNewCmdExtension(t *testing.T) { name: "upgrade an extension gh-prefix", args: []string{"upgrade", "gh-hello"}, managerStubs: func(em *extensions.ExtensionManagerMock) func(*testing.T) { - em.UpgradeFunc = func(name string, force bool, out, errOut io.Writer) error { + em.UpgradeFunc = func(name string, force bool) error { return nil } return func(t *testing.T) { @@ -122,7 +121,7 @@ func TestNewCmdExtension(t *testing.T) { name: "upgrade an extension full name", args: []string{"upgrade", "monalisa/gh-hello"}, managerStubs: func(em *extensions.ExtensionManagerMock) func(*testing.T) { - em.UpgradeFunc = func(name string, force bool, out, errOut io.Writer) error { + em.UpgradeFunc = func(name string, force bool) error { return nil } return func(t *testing.T) { @@ -136,7 +135,7 @@ func TestNewCmdExtension(t *testing.T) { name: "upgrade all", args: []string{"upgrade", "--all"}, managerStubs: func(em *extensions.ExtensionManagerMock) func(*testing.T) { - em.UpgradeFunc = func(name string, force bool, out, errOut io.Writer) error { + em.UpgradeFunc = func(name string, force bool) error { return nil } return func(t *testing.T) { diff --git a/pkg/cmd/extension/manager.go b/pkg/cmd/extension/manager.go index 7e1f403e2..d8ec1d94a 100644 --- a/pkg/cmd/extension/manager.go +++ b/pkg/cmd/extension/manager.go @@ -306,7 +306,7 @@ func (m *Manager) installGit(cloneURL string, stdout, stderr io.Writer) error { var localExtensionUpgradeError = errors.New("local extensions can not be upgraded") -func (m *Manager) Upgrade(name string, force bool, stdout, stderr io.Writer) error { +func (m *Manager) Upgrade(name string, force bool) error { exe, err := m.lookPath("git") if err != nil { return err @@ -320,14 +320,14 @@ func (m *Manager) Upgrade(name string, force bool, stdout, stderr io.Writer) err someUpgraded := false for _, f := range exts { if name == "" { - fmt.Fprintf(stdout, "[%s]: ", f.Name()) + fmt.Fprintf(m.io.Out, "[%s]: ", f.Name()) } else if f.Name() != name { continue } if f.IsLocal() { if name == "" { - fmt.Fprintf(stdout, "%s\n", localExtensionUpgradeError) + fmt.Fprintf(m.io.Out, "%s\n", localExtensionUpgradeError) } else { err = localExtensionUpgradeError } @@ -344,7 +344,7 @@ func (m *Manager) Upgrade(name string, force bool, stdout, stderr io.Writer) err pullCmd := m.newCommand(exe, "-C", dir, "--git-dir="+filepath.Join(dir, ".git"), "pull", "--ff-only") cmds = []*exec.Cmd{pullCmd} } - if e := runCmds(cmds, stdout, stderr); e != nil { + if e := runCmds(cmds, m.io.Out, m.io.ErrOut); e != nil { err = e } someUpgraded = true diff --git a/pkg/cmd/extension/manager_test.go b/pkg/cmd/extension/manager_test.go index d78f4c2e6..19e9e1965 100644 --- a/pkg/cmd/extension/manager_test.go +++ b/pkg/cmd/extension/manager_test.go @@ -108,11 +108,11 @@ func TestManager_Upgrade_AllExtensions(t *testing.T) { assert.NoError(t, stubExtension(filepath.Join(tempDir, "extensions", "gh-two", "gh-two"))) assert.NoError(t, stubLocalExtension(tempDir, filepath.Join(tempDir, "extensions", "gh-local", "gh-local"))) - m := newTestManager(tempDir, nil, nil) + io, _, stdout, stderr := iostreams.Test() - stdout := &bytes.Buffer{} - stderr := &bytes.Buffer{} - err := m.Upgrade("", false, stdout, stderr) + m := newTestManager(tempDir, nil, io) + + err := m.Upgrade("", false) assert.NoError(t, err) assert.Equal(t, heredoc.Docf( @@ -133,11 +133,11 @@ func TestManager_Upgrade_RemoteExtension(t *testing.T) { tempDir := t.TempDir() assert.NoError(t, stubExtension(filepath.Join(tempDir, "extensions", "gh-remote", "gh-remote"))) - m := newTestManager(tempDir, nil, nil) + io, _, stdout, stderr := iostreams.Test() - stdout := &bytes.Buffer{} - stderr := &bytes.Buffer{} - err := m.Upgrade("remote", false, stdout, stderr) + m := newTestManager(tempDir, nil, io) + + err := m.Upgrade("remote", false) assert.NoError(t, err) assert.Equal(t, heredoc.Docf( ` @@ -153,11 +153,10 @@ func TestManager_Upgrade_LocalExtension(t *testing.T) { tempDir := t.TempDir() assert.NoError(t, stubLocalExtension(tempDir, filepath.Join(tempDir, "extensions", "gh-local", "gh-local"))) - m := newTestManager(tempDir, nil, nil) + io, _, stdout, stderr := iostreams.Test() + m := newTestManager(tempDir, nil, io) - stdout := &bytes.Buffer{} - stderr := &bytes.Buffer{} - err := m.Upgrade("local", false, stdout, stderr) + err := m.Upgrade("local", false) assert.EqualError(t, err, "local extensions can not be upgraded") assert.Equal(t, "", stdout.String()) assert.Equal(t, "", stderr.String()) @@ -170,11 +169,10 @@ func TestManager_Upgrade_Force(t *testing.T) { assert.NoError(t, stubExtension(filepath.Join(tempDir, "extensions", "gh-remote", "gh-remote"))) - m := newTestManager(tempDir, nil, nil) + io, _, stdout, stderr := iostreams.Test() + m := newTestManager(tempDir, nil, io) - stdout := &bytes.Buffer{} - stderr := &bytes.Buffer{} - err := m.Upgrade("remote", true, stdout, stderr) + err := m.Upgrade("remote", true) assert.NoError(t, err) assert.Equal(t, heredoc.Docf( ` @@ -192,11 +190,10 @@ func TestManager_Upgrade_Force(t *testing.T) { func TestManager_Upgrade_NoExtensions(t *testing.T) { tempDir := t.TempDir() - m := newTestManager(tempDir, nil, nil) + io, _, stdout, stderr := iostreams.Test() + m := newTestManager(tempDir, nil, io) - stdout := &bytes.Buffer{} - stderr := &bytes.Buffer{} - err := m.Upgrade("", false, stdout, stderr) + err := m.Upgrade("", false) assert.EqualError(t, err, "no extensions installed") assert.Equal(t, "", stdout.String()) assert.Equal(t, "", stderr.String()) diff --git a/pkg/extensions/extension.go b/pkg/extensions/extension.go index 4e9ce89b5..c9f1472f9 100644 --- a/pkg/extensions/extension.go +++ b/pkg/extensions/extension.go @@ -20,7 +20,7 @@ type ExtensionManager interface { List(includeMetadata bool) []Extension Install(ghrepo.Interface) error InstallLocal(dir string) error - Upgrade(name string, force bool, stdout, stderr io.Writer) error + Upgrade(name string, force bool) error Remove(name string) error Dispatch(args []string, stdin io.Reader, stdout, stderr io.Writer) (bool, error) Create(name string) error diff --git a/pkg/extensions/manager_mock.go b/pkg/extensions/manager_mock.go index 96d76cdf7..c681efe7d 100644 --- a/pkg/extensions/manager_mock.go +++ b/pkg/extensions/manager_mock.go @@ -37,7 +37,7 @@ var _ ExtensionManager = &ExtensionManagerMock{} // RemoveFunc: func(name string) error { // panic("mock out the Remove method") // }, -// UpgradeFunc: func(name string, force bool, stdout io.Writer, stderr io.Writer) error { +// UpgradeFunc: func(name string, force bool) error { // panic("mock out the Upgrade method") // }, // } @@ -66,7 +66,7 @@ type ExtensionManagerMock struct { RemoveFunc func(name string) error // UpgradeFunc mocks the Upgrade method. - UpgradeFunc func(name string, force bool, stdout io.Writer, stderr io.Writer) error + UpgradeFunc func(name string, force bool) error // calls tracks calls to the methods. calls struct { @@ -112,10 +112,6 @@ type ExtensionManagerMock struct { Name string // Force is the force argument value. Force bool - // Stdout is the stdout argument value. - Stdout io.Writer - // Stderr is the stderr argument value. - Stderr io.Writer } } lockCreate sync.RWMutex @@ -326,41 +322,33 @@ func (mock *ExtensionManagerMock) RemoveCalls() []struct { } // Upgrade calls UpgradeFunc. -func (mock *ExtensionManagerMock) Upgrade(name string, force bool, stdout io.Writer, stderr io.Writer) error { +func (mock *ExtensionManagerMock) Upgrade(name string, force bool) error { if mock.UpgradeFunc == nil { panic("ExtensionManagerMock.UpgradeFunc: method is nil but ExtensionManager.Upgrade was just called") } callInfo := struct { - Name string - Force bool - Stdout io.Writer - Stderr io.Writer + Name string + Force bool }{ - Name: name, - Force: force, - Stdout: stdout, - Stderr: stderr, + Name: name, + Force: force, } mock.lockUpgrade.Lock() mock.calls.Upgrade = append(mock.calls.Upgrade, callInfo) mock.lockUpgrade.Unlock() - return mock.UpgradeFunc(name, force, stdout, stderr) + return mock.UpgradeFunc(name, force) } // UpgradeCalls gets all the calls that were made to Upgrade. // Check the length with: // len(mockedExtensionManager.UpgradeCalls()) func (mock *ExtensionManagerMock) UpgradeCalls() []struct { - Name string - Force bool - Stdout io.Writer - Stderr io.Writer + Name string + Force bool } { var calls []struct { - Name string - Force bool - Stdout io.Writer - Stderr io.Writer + Name string + Force bool } mock.lockUpgrade.RLock() calls = mock.calls.Upgrade