Make extension upgrade output more friendly

This commit is contained in:
Sam Coe 2021-10-05 11:17:35 -07:00
parent 968b093eda
commit defbf0f306
No known key found for this signature in database
GPG key ID: 8E322C20F811D086
4 changed files with 202 additions and 130 deletions

View file

@ -132,7 +132,24 @@ func NewCmdExtension(f *cmdutil.Factory) *cobra.Command {
if len(args) > 0 {
name = normalizeExtensionSelector(args[0])
}
return m.Upgrade(name, flagForce)
cs := io.ColorScheme()
err := m.Upgrade(name, flagForce)
if err != nil {
if name != "" {
fmt.Fprintf(io.ErrOut, "%s Failed upgrading extension %s: %s", cs.FailureIcon(), name, err)
} else {
fmt.Fprintf(io.ErrOut, "%s Failed upgrading extensions", cs.FailureIcon())
}
return cmdutil.SilentError
}
if io.IsStdoutTTY() {
if name != "" {
fmt.Fprintf(io.Out, "%s Successfully upgraded extension %s\n", cs.SuccessIcon(), name)
} else {
fmt.Fprintf(io.Out, "%s Successfully upgraded extensions\n", cs.SuccessIcon())
}
}
return nil
},
}
cmd.Flags().BoolVar(&flagAll, "all", false, "Upgrade all extensions")

View file

@ -102,6 +102,23 @@ func TestNewCmdExtension(t *testing.T) {
assert.Equal(t, "hello", calls[0].Name)
}
},
isTTY: true,
wantStdout: "✓ Successfully upgraded extension hello\n",
},
{
name: "upgrade an extension notty",
args: []string{"upgrade", "hello"},
managerStubs: func(em *extensions.ExtensionManagerMock) func(*testing.T) {
em.UpgradeFunc = func(name string, force bool) error {
return nil
}
return func(t *testing.T) {
calls := em.UpgradeCalls()
assert.Equal(t, 1, len(calls))
assert.Equal(t, "hello", calls[0].Name)
}
},
isTTY: false,
},
{
name: "upgrade an extension gh-prefix",
@ -116,6 +133,8 @@ func TestNewCmdExtension(t *testing.T) {
assert.Equal(t, "hello", calls[0].Name)
}
},
isTTY: true,
wantStdout: "✓ Successfully upgraded extension hello\n",
},
{
name: "upgrade an extension full name",
@ -130,6 +149,8 @@ func TestNewCmdExtension(t *testing.T) {
assert.Equal(t, "hello", calls[0].Name)
}
},
isTTY: true,
wantStdout: "✓ Successfully upgraded extension hello\n",
},
{
name: "upgrade all",
@ -144,6 +165,23 @@ func TestNewCmdExtension(t *testing.T) {
assert.Equal(t, "", calls[0].Name)
}
},
isTTY: true,
wantStdout: "✓ Successfully upgraded extensions\n",
},
{
name: "upgrade all notty",
args: []string{"upgrade", "--all"},
managerStubs: func(em *extensions.ExtensionManagerMock) func(*testing.T) {
em.UpgradeFunc = func(name string, force bool) error {
return nil
}
return func(t *testing.T) {
calls := em.UpgradeCalls()
assert.Equal(t, 1, len(calls))
assert.Equal(t, "", calls[0].Name)
}
},
isTTY: false,
},
{
name: "remove extension tty",

View file

@ -417,58 +417,81 @@ func (m *Manager) installGit(cloneURL string, stdout, stderr io.Writer) error {
}
var localExtensionUpgradeError = errors.New("local extensions can not be upgraded")
var upToDateError = errors.New("already up to date")
var noExtensionsInstalledError = errors.New("no extensions installed")
func (m *Manager) Upgrade(name string, force bool) error {
// Fetch metadata during list only when upgrading all extensions.
// This is a performance improvement so that we don't make a
// bunch of unecessary network requests when trying to upgrade a single extension.
fetchMetadata := name == ""
exts, _ := m.list(fetchMetadata)
if len(exts) == 0 {
return noExtensionsInstalledError
}
if name == "" {
return m.upgradeExtensions(exts, force)
}
for _, f := range exts {
if f.Name() != name {
continue
}
var err error
// For single extensions manually retrieve latest version since we forgo
// doing it during list.
f.latestVersion, err = m.getLatestVersion(f)
if err != nil {
return err
}
return m.upgradeExtension(f, force)
}
return fmt.Errorf("no extension matched %q", name)
}
func (m *Manager) upgradeExtensions(exts []Extension, force bool) error {
var failed bool
for _, f := range exts {
fmt.Fprintf(m.io.Out, "[%s]: ", f.Name())
err := m.upgradeExtension(f, force)
if err != nil {
if !errors.Is(err, localExtensionUpgradeError) &&
!errors.Is(err, upToDateError) {
failed = true
}
fmt.Fprintf(m.io.Out, "%s\n", err)
continue
}
fmt.Fprintf(m.io.Out, "upgrade complete\n")
}
if failed {
return errors.New("some extensions failed to upgrade")
}
return nil
}
func (m *Manager) upgradeExtension(ext Extension, force bool) error {
if ext.isLocal {
return localExtensionUpgradeError
}
if !ext.UpdateAvailable() {
return upToDateError
}
var err error
if ext.kind == BinaryKind {
err = m.upgradeBinExtension(ext)
} else {
err = m.upgradeGitExtension(ext, force)
}
return err
}
func (m *Manager) upgradeGitExtension(ext Extension, force bool) error {
exe, err := m.lookPath("git")
if err != nil {
return err
}
exts := m.List(false)
if len(exts) == 0 {
return errors.New("no extensions installed")
}
someUpgraded := false
for _, f := range exts {
if name == "" {
fmt.Fprintf(m.io.Out, "[%s]: ", f.Name())
} else if f.Name() != name {
continue
}
if f.IsLocal() {
if name == "" {
fmt.Fprintf(m.io.Out, "%s\n", localExtensionUpgradeError)
} else {
err = localExtensionUpgradeError
}
continue
}
binManifestPath := filepath.Join(filepath.Dir(f.Path()), manifestName)
if _, e := os.Stat(binManifestPath); e == nil {
err = m.upgradeBin(f)
someUpgraded = true
continue
}
if e := m.upgradeGit(f, exe, force); e != nil {
err = e
}
someUpgraded = true
}
if err == nil && !someUpgraded {
err = fmt.Errorf("no extension matched %q", name)
}
return err
}
func (m *Manager) upgradeGit(ext extensions.Extension, exe string, force bool) error {
var cmds []*exec.Cmd
dir := filepath.Dir(ext.Path())
dir := filepath.Dir(ext.path)
if force {
fetchCmd := m.newCommand(exe, "-C", dir, "--git-dir="+filepath.Join(dir, ".git"), "fetch", "origin", "HEAD")
resetCmd := m.newCommand(exe, "-C", dir, "--git-dir="+filepath.Join(dir, ".git"), "reset", "--hard", "origin/HEAD")
@ -477,34 +500,14 @@ func (m *Manager) upgradeGit(ext extensions.Extension, exe string, force bool) e
pullCmd := m.newCommand(exe, "-C", dir, "--git-dir="+filepath.Join(dir, ".git"), "pull", "--ff-only")
cmds = []*exec.Cmd{pullCmd}
}
return runCmds(cmds, m.io.Out, m.io.ErrOut)
return runCmds(cmds)
}
func (m *Manager) upgradeBin(ext extensions.Extension) error {
manifestPath := filepath.Join(filepath.Dir(ext.Path()), manifestName)
manifest, err := os.ReadFile(manifestPath)
func (m *Manager) upgradeBinExtension(ext Extension) error {
repo, err := ghrepo.FromFullName(ext.url)
if err != nil {
return fmt.Errorf("could not open %s for reading: %w", manifestPath, err)
return fmt.Errorf("failed to parse URL %s: %w", ext.url, err)
}
var bm binManifest
err = yaml.Unmarshal(manifest, &bm)
if err != nil {
return fmt.Errorf("could not parse %s: %w", manifestPath, err)
}
repo := ghrepo.NewWithHost(bm.Owner, bm.Name, bm.Host)
var r *release
r, err = fetchLatestRelease(m.client, repo)
if err != nil {
return fmt.Errorf("failed to get release info for %s: %w", ghrepo.FullName(repo), err)
}
if bm.Tag == r.Tag {
return nil
}
return m.installBin(repo)
}
@ -590,10 +593,8 @@ func (m *Manager) Create(name string) error {
return err
}
func runCmds(cmds []*exec.Cmd, stdout, stderr io.Writer) error {
func runCmds(cmds []*exec.Cmd) error {
for _, cmd := range cmds {
cmd.Stdout = stdout
cmd.Stderr = stderr
if err := cmd.Run(); err != nil {
return err
}

View file

@ -42,6 +42,10 @@ func newTestManager(dir string, client *http.Client, io *iostreams.IOStreams) *M
newCommand: func(exe string, args ...string) *exec.Cmd {
args = append([]string{os.Args[0], "-test.run=TestHelperProcess", "--", exe}, args...)
cmd := exec.Command(args[0], args[1:]...)
if io != nil {
cmd.Stdout = io.Out
cmd.Stderr = io.ErrOut
}
cmd.Env = []string{"GH_WANT_HELPER_PROCESS=1"}
return cmd
},
@ -150,24 +154,50 @@ func TestManager_Remove(t *testing.T) {
assert.Equal(t, "gh-two", items[0].Name())
}
func TestManager_Upgrade_AllExtensions(t *testing.T) {
func TestManager_Upgrade_NoExtensions(t *testing.T) {
tempDir := t.TempDir()
io, _, stdout, stderr := iostreams.Test()
m := newTestManager(tempDir, nil, io)
err := m.Upgrade("", false)
assert.EqualError(t, err, "no extensions installed")
assert.Equal(t, "", stdout.String())
assert.Equal(t, "", stderr.String())
}
func TestManager_Upgrade_NoMatchingExtension(t *testing.T) {
tempDir := t.TempDir()
assert.NoError(t, stubExtension(filepath.Join(tempDir, "extensions", "gh-hello", "gh-hello")))
io, _, stdout, stderr := iostreams.Test()
m := newTestManager(tempDir, nil, io)
err := m.Upgrade("invalid", false)
assert.EqualError(t, err, `no extension matched "invalid"`)
assert.Equal(t, "", stdout.String())
assert.Equal(t, "", stderr.String())
}
func TestManager_UpgradeExtensions(t *testing.T) {
tempDir := t.TempDir()
assert.NoError(t, stubExtension(filepath.Join(tempDir, "extensions", "gh-hello", "gh-hello")))
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")))
io, _, stdout, stderr := iostreams.Test()
m := newTestManager(tempDir, nil, io)
err := m.Upgrade("", false)
exts, err := m.list(false)
assert.NoError(t, err)
assert.Equal(t, 3, len(exts))
for i := 0; i < 3; i++ {
exts[i].currentVersion = "old version"
exts[i].latestVersion = "new version"
}
err = m.upgradeExtensions(exts, false)
assert.NoError(t, err)
assert.Equal(t, heredoc.Docf(
`
[hello]: [git -C %s --git-dir=%s pull --ff-only]
upgrade complete
[local]: local extensions can not be upgraded
[two]: [git -C %s --git-dir=%s pull --ff-only]
upgrade complete
`,
filepath.Join(tempDir, "extensions", "gh-hello"),
filepath.Join(tempDir, "extensions", "gh-hello", ".git"),
@ -177,15 +207,33 @@ func TestManager_Upgrade_AllExtensions(t *testing.T) {
assert.Equal(t, "", stderr.String())
}
func TestManager_Upgrade_RemoteExtension(t *testing.T) {
func TestManager_UpgradeExtension_LocalExtension(t *testing.T) {
tempDir := t.TempDir()
assert.NoError(t, stubExtension(filepath.Join(tempDir, "extensions", "gh-remote", "gh-remote")))
assert.NoError(t, stubLocalExtension(tempDir, filepath.Join(tempDir, "extensions", "gh-local", "gh-local")))
io, _, stdout, stderr := iostreams.Test()
m := newTestManager(tempDir, nil, io)
exts, err := m.list(false)
assert.NoError(t, err)
assert.Equal(t, 1, len(exts))
err = m.upgradeExtension(exts[0], false)
assert.EqualError(t, err, "local extensions can not be upgraded")
assert.Equal(t, "", stdout.String())
assert.Equal(t, "", stderr.String())
}
err := m.Upgrade("remote", false)
func TestManager_UpgradeExtension_GitExtension(t *testing.T) {
tempDir := t.TempDir()
assert.NoError(t, stubExtension(filepath.Join(tempDir, "extensions", "gh-remote", "gh-remote")))
io, _, stdout, stderr := iostreams.Test()
m := newTestManager(tempDir, nil, io)
exts, err := m.list(false)
assert.NoError(t, err)
assert.Equal(t, 1, len(exts))
ext := exts[0]
ext.currentVersion = "old version"
ext.latestVersion = "new version"
err = m.upgradeExtension(ext, false)
assert.NoError(t, err)
assert.Equal(t, heredoc.Docf(
`
@ -197,30 +245,20 @@ func TestManager_Upgrade_RemoteExtension(t *testing.T) {
assert.Equal(t, "", stderr.String())
}
func TestManager_Upgrade_LocalExtension(t *testing.T) {
tempDir := t.TempDir()
assert.NoError(t, stubLocalExtension(tempDir, filepath.Join(tempDir, "extensions", "gh-local", "gh-local")))
io, _, stdout, stderr := iostreams.Test()
m := newTestManager(tempDir, nil, io)
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())
}
func TestManager_Upgrade_Force(t *testing.T) {
func TestManager_UpgradeExtension_GitExtension_Force(t *testing.T) {
tempDir := t.TempDir()
extensionDir := filepath.Join(tempDir, "extensions", "gh-remote")
gitDir := filepath.Join(tempDir, "extensions", "gh-remote", ".git")
assert.NoError(t, stubExtension(filepath.Join(tempDir, "extensions", "gh-remote", "gh-remote")))
io, _, stdout, stderr := iostreams.Test()
m := newTestManager(tempDir, nil, io)
err := m.Upgrade("remote", true)
exts, err := m.list(false)
assert.NoError(t, err)
assert.Equal(t, 1, len(exts))
ext := exts[0]
ext.currentVersion = "old version"
ext.latestVersion = "new version"
err = m.upgradeExtension(ext, true)
assert.NoError(t, err)
assert.Equal(t, heredoc.Docf(
`
@ -235,26 +273,12 @@ func TestManager_Upgrade_Force(t *testing.T) {
assert.Equal(t, "", stderr.String())
}
func TestManager_Upgrade_NoExtensions(t *testing.T) {
func TestManager_UpgradeExtension_BinaryExtension(t *testing.T) {
tempDir := t.TempDir()
io, _, stdout, stderr := iostreams.Test()
m := newTestManager(tempDir, nil, io)
err := m.Upgrade("", false)
assert.EqualError(t, err, "no extensions installed")
assert.Equal(t, "", stdout.String())
assert.Equal(t, "", stderr.String())
}
func TestManager_Upgrade_BinaryExtension(t *testing.T) {
tempDir := t.TempDir()
io, _, _, _ := iostreams.Test()
reg := httpmock.Registry{}
defer reg.Verify(t)
client := http.Client{Transport: &reg}
assert.NoError(t, stubBinaryExtension(
filepath.Join(tempDir, "extensions", "gh-bin-ext"),
binManifest{
@ -263,20 +287,7 @@ func TestManager_Upgrade_BinaryExtension(t *testing.T) {
Host: "example.com",
Tag: "v1.0.1",
}))
m := newTestManager(tempDir, &client, io)
reg.Register(
httpmock.REST("GET", "api/v3/repos/owner/gh-bin-ext/releases/latest"),
httpmock.JSONResponse(
release{
Tag: "v1.0.2",
Assets: []releaseAsset{
{
Name: "gh-bin-ext-windows-amd64",
APIURL: "https://example.com/release/cool2",
},
},
}))
reg.Register(
httpmock.REST("GET", "api/v3/repos/owner/gh-bin-ext/releases/latest"),
httpmock.JSONResponse(
@ -293,7 +304,12 @@ func TestManager_Upgrade_BinaryExtension(t *testing.T) {
httpmock.REST("GET", "release/cool2"),
httpmock.StringResponse("FAKE UPGRADED BINARY"))
err := m.Upgrade("bin-ext", false)
exts, err := m.list(false)
assert.NoError(t, err)
assert.Equal(t, 1, len(exts))
ext := exts[0]
ext.latestVersion = "v1.0.2"
err = m.upgradeExtension(ext, false)
assert.NoError(t, err)
manifest, err := os.ReadFile(filepath.Join(tempDir, "extensions/gh-bin-ext", manifestName))