From 01337d3de69c2aeed201f18fe9f2ba57e5ccdb97 Mon Sep 17 00:00:00 2001 From: Daniel Cavalcante Date: Sun, 16 Aug 2026 06:19:29 -0300 Subject: [PATCH 1/2] fix(cli): print farewell after successful cleanup --- internal/cli/root.go | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/internal/cli/root.go b/internal/cli/root.go index b329b58..7f3dbc3 100644 --- a/internal/cli/root.go +++ b/internal/cli/root.go @@ -81,12 +81,10 @@ func NewRootCmd(ctx context.Context, cliCtx *CliContext) *cobra.Command { if err := cliCtx.App.Start(ctx); err != nil { return err } - cliCtx.Printer.Println("Thanks for using Pgxcli.") - cliCtx.Printer.Println("see you next time.") return nil }, - PersistentPostRunE: func(_ *cobra.Command, _ []string) error { + PersistentPostRunE: func(cmd *cobra.Command, _ []string) error { if cliCtx.App != nil { if err := cliCtx.App.Close(); err != nil { return err @@ -102,6 +100,11 @@ func NewRootCmd(ctx context.Context, cliCtx *CliContext) *cobra.Command { return err } } + // Keep future non-interactive subcommand output free of the interactive farewell. + if cmd == cmd.Root() { + cliCtx.Printer.Println("Thanks for using Pgxcli.") + cliCtx.Printer.Println("see you next time.") + } return nil }, } From 584a3e948d79f691371d2c4dd42dbb5a7f2c60f2 Mon Sep 17 00:00:00 2001 From: Daniel Cavalcante Date: Wed, 19 Aug 2026 17:53:05 -0300 Subject: [PATCH 2/2] fix(cli): collect cleanup errors --- internal/cli/root.go | 50 +++++++++++++++------ internal/cli/root_test.go | 91 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 127 insertions(+), 14 deletions(-) diff --git a/internal/cli/root.go b/internal/cli/root.go index 7f3dbc3..8a6da2d 100644 --- a/internal/cli/root.go +++ b/internal/cli/root.go @@ -3,6 +3,7 @@ package cli import ( "bufio" "context" + "errors" "fmt" "os" "os/user" @@ -85,20 +86,28 @@ func NewRootCmd(ctx context.Context, cliCtx *CliContext) *cobra.Command { }, PersistentPostRunE: func(cmd *cobra.Command, _ []string) error { - if cliCtx.App != nil { - if err := cliCtx.App.Close(); err != nil { - return err - } - } - if cliCtx.Client != nil { - if err := cliCtx.Client.Close(ctx); err != nil { - return err - } - } - if cliCtx.Logger != nil { - if err := cliCtx.Logger.Close(); err != nil { - return err - } + cleanupErr := closeResources( + func() error { + if cliCtx.App == nil { + return nil + } + return cliCtx.App.Close() + }, + func() error { + if cliCtx.Client == nil { + return nil + } + return cliCtx.Client.Close(ctx) + }, + func() error { + if cliCtx.Logger == nil { + return nil + } + return cliCtx.Logger.Close() + }, + ) + if cleanupErr != nil { + return cleanupErr } // Keep future non-interactive subcommand output free of the interactive farewell. if cmd == cmd.Root() { @@ -128,6 +137,19 @@ func NewRootCmd(ctx context.Context, cliCtx *CliContext) *cobra.Command { return rootCmd } +// Close every resource so failures do not prevent later cleanup. +func closeResources(closers ...func() error) error { + var errs []error + + for _, close := range closers { + if err := close(); err != nil { + errs = append(errs, err) + } + } + + return errors.Join(errs...) +} + type connectionParams struct { database string user string diff --git a/internal/cli/root_test.go b/internal/cli/root_test.go index 1c143a7..e146942 100644 --- a/internal/cli/root_test.go +++ b/internal/cli/root_test.go @@ -1,9 +1,15 @@ package cli import ( + "bytes" + "context" + "errors" + "io" "os" + "strings" "testing" + "github.com/balajz/pgxcli/internal/cliio" "github.com/spf13/cobra" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -19,6 +25,91 @@ type dbAndUserTestCase struct { expectedUser string } +type testApp struct { + closeErr error + closeCalled bool +} + +func (a *testApp) Start(context.Context) error { return nil } + +func (a *testApp) Close() error { + a.closeCalled = true + return a.closeErr +} + +func TestCloseResourcesClosesEverythingAndJoinsErrors(t *testing.T) { + t.Parallel() + + appErr := errors.New("app close failed") + loggerErr := errors.New("logger close failed") + var calls []string + + err := closeResources( + func() error { + calls = append(calls, "app") + return appErr + }, + func() error { + calls = append(calls, "client") + return nil + }, + func() error { + calls = append(calls, "logger") + return loggerErr + }, + ) + + assert.Equal(t, []string{"app", "client", "logger"}, calls) + assert.ErrorIs(t, err, appErr) + assert.ErrorIs(t, err, loggerErr) +} + +func TestPersistentPostRunCleanupAndFarewell(t *testing.T) { + t.Parallel() + + testCases := []struct { + name string + childCommand bool + cleanupFails bool + wantFarewell bool + }{ + {name: "root success", wantFarewell: true}, + {name: "root failure", cleanupFails: true}, + {name: "child success", childCommand: true}, + {name: "child failure", childCommand: true, cleanupFails: true}, + } + + for _, testCase := range testCases { + t.Run(testCase.name, func(t *testing.T) { + var output bytes.Buffer + var closeErr error + if testCase.cleanupFails { + closeErr = errors.New("history save failed") + } + testApplication := &testApp{closeErr: closeErr} + cliCtx := &CliContext{ + App: testApplication, + Printer: cliio.NewPgxPrinter(&output, io.Discard), + } + rootCmd := NewRootCmd(context.Background(), cliCtx) + cmd := rootCmd + if testCase.childCommand { + cmd = &cobra.Command{Use: "export"} + rootCmd.AddCommand(cmd) + } + + err := rootCmd.PersistentPostRunE(cmd, nil) + if closeErr != nil { + require.ErrorIs(t, err, closeErr) + } else { + require.NoError(t, err) + } + assert.True(t, testApplication.closeCalled) + assert.Equal(t, testCase.wantFarewell, strings.Contains(output.String(), "Thanks for using Pgxcli.")) + }) + } +} + func TestPromptPasswordFallsBackToFullLineInput(t *testing.T) { oldStdin := os.Stdin stdin, writer, err := os.Pipe()