diff --git a/.golangci.yaml b/.golangci.yaml index abcbe34..dbaa5ec 100644 --- a/.golangci.yaml +++ b/.golangci.yaml @@ -102,6 +102,7 @@ linters: - varnamelen # Checks that the length of a variable's name matches its scope. - wastedassign # Finds wasted assignment statements. - wrapcheck # Checks that errors returned from external packages are wrapped. + - zerologlint # Detects the wrong usage of `zerolog` that a user forgets to dispatch with `Send` or `Msg`. disable: - cyclop # Checks function and package cyclomatic complexity. - depguard # Checks if package imports are in a list of acceptable packages. @@ -125,7 +126,6 @@ linters: - tagliatelle # Checks the struct tags. - testableexamples # Linter checks if examples are testable (have an expected output). [fast] - testpackage # Linter that makes you use a separate _test package. - - zerologlint # Detects the wrong usage of `zerolog` that a user forgets to dispatch with `Send` or `Msg`. ###################################################################################################### # Linter Settings Configuration @@ -140,10 +140,10 @@ linters: - name: var-naming severity: warning disabled: false - exclude: [""] + exclude: [] arguments: - - [""] # AllowList - - [""] # DenyList + - [] # AllowList + - [] # DenyList - - skip-initialism-name-checks: false upper-case-const: true skip-package-name-checks: true @@ -232,6 +232,7 @@ linters: - promlinter - wrapcheck - varnamelen + - zerologlint # Run some linter only for test files by excluding its issues for everything else. # - path-except: _test\.go diff --git a/cmd/root.go b/cmd/root.go index 9c41f67..ff2f4ef 100644 --- a/cmd/root.go +++ b/cmd/root.go @@ -24,34 +24,20 @@ import ( "os" "github.com/spf13/cobra" - "go.uber.org/zap" - "go.uber.org/zap/zapcore" "github.com/nicholas-fedor/go-remove/internal/cli" "github.com/nicholas-fedor/go-remove/internal/fs" "github.com/nicholas-fedor/go-remove/internal/logger" ) -// ErrInvalidLoggerType indicates that the logger is not of the expected *ZapLogger type. -var ErrInvalidLoggerType = errors.New("logger is not a *ZapLogger") +// ErrInvalidLoggerType indicates that the logger is not of the expected *ZerologLogger type. +var ErrInvalidLoggerType = errors.New("logger is not a *ZerologLogger") // rootCmd defines the root command for go-remove. var rootCmd = &cobra.Command{ Use: "go-remove [binary]", Short: "A tool to remove Go binaries", RunE: func(cmd *cobra.Command, args []string) error { - // Initialize the logger for application-wide logging. - log, err := logger.NewZapLogger() - if err != nil { - return fmt.Errorf("failed to initialize logger: %w", err) - } - - // Assemble dependencies with a real filesystem and the logger instance. - deps := cli.Dependencies{ - FS: fs.NewRealFS(), - Logger: log, - } - // Extract flag values to configure CLI behavior; defaults to TUI mode if no binary is given. verbose, _ := cmd.Flags().GetBool("verbose") goroot, _ := cmd.Flags().GetBool("goroot") @@ -64,54 +50,53 @@ var rootCmd = &cobra.Command{ LogLevel: logLevel, } - // Set log level based on config if verbose mode is enabled. - if verbose { - zapLogger, ok := log.(*logger.ZapLogger) - if !ok { - return fmt.Errorf( - "failed to set log level: %w with type %T", - ErrInvalidLoggerType, - log, - ) - } + // If a binary name is provided as an argument, run in direct removal mode. + if len(args) > 0 { + config.Binary = args[0] - switch logLevel { - case "debug": - zapLogger.Logger = zapLogger.WithOptions( - zap.IncreaseLevel(zapcore.DebugLevel), - ) - case "warn": - zapLogger.Logger = zapLogger.WithOptions( - zap.IncreaseLevel(zapcore.WarnLevel), - ) - case "error": - zapLogger.Logger = zapLogger.WithOptions( - zap.IncreaseLevel(zapcore.ErrorLevel), - ) - default: - zapLogger.Logger = zapLogger.WithOptions( - zap.IncreaseLevel(zapcore.InfoLevel), - ) + // Initialize the standard logger for direct removal mode. + log, err := logger.NewLogger() + if err != nil { + return fmt.Errorf("failed to initialize logger: %w", err) } - log = zapLogger // Update the logger in deps - deps.Logger = log - } + // Set log level based on config if verbose mode is enabled. + if verbose { + level := logger.ParseLevel(logLevel) + log.Level(level) + } - // If a binary name is provided as an argument, run in direct removal mode. - if len(args) > 0 { - config.Binary = args[0] + // Assemble dependencies with a real filesystem and the logger instance. + deps := cli.Dependencies{ + FS: fs.NewRealFS(), + Logger: log, + } return cli.Run(deps, config) } // Otherwise, determine the binary directory and launch the TUI for interactive selection. - binDir, err := deps.FS.DetermineBinDir(config.Goroot) + // For TUI mode, we use a logger with capture support to display logs within the interface. + filesystem := fs.NewRealFS() + + binDir, err := filesystem.DetermineBinDir(config.Goroot) if err != nil { return fmt.Errorf("failed to determine binary directory: %w", err) } - return cli.RunTUI(binDir, config, deps.Logger, deps.FS, cli.DefaultRunner{}) + // Initialize the logger with capture support for TUI mode. + log, _, err := logger.NewLoggerWithCapture() + if err != nil { + return fmt.Errorf("failed to initialize logger: %w", err) + } + + // Set log level based on config if verbose mode is enabled. + if verbose { + level := logger.ParseLevel(logLevel) + log.Level(level) + } + + return cli.RunTUI(binDir, config, log, filesystem, cli.DefaultRunner{}) }, } diff --git a/cmd/root_test.go b/cmd/root_test.go index 600eb65..dbefd4b 100644 --- a/cmd/root_test.go +++ b/cmd/root_test.go @@ -21,8 +21,6 @@ import ( "bytes" "os" "testing" - - logmocks "github.com/nicholas-fedor/go-remove/internal/logger/mocks" ) // TestRootCommand verifies the behavior of the root command. @@ -42,9 +40,6 @@ func TestRootCommand(t *testing.T) { } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - // Mock logger without Sync expectation for help flag test. - log := logmocks.NewMockLogger(t) - // Redirect stderr to capture output. oldStderr := os.Stderr r, w, _ := os.Pipe() @@ -77,8 +72,6 @@ func TestRootCommand(t *testing.T) { if gotStderr != tt.wantStderr { t.Errorf("rootCmd.Execute() stderr = %q, want %q", gotStderr, tt.wantStderr) } - - log.AssertExpectations(t) }) } } diff --git a/go.mod b/go.mod index ef1bd22..be103c2 100644 --- a/go.mod +++ b/go.mod @@ -5,9 +5,9 @@ go 1.26.0 require ( charm.land/bubbletea/v2 v2.0.0 charm.land/lipgloss/v2 v2.0.0 + github.com/rs/zerolog v1.34.0 github.com/spf13/cobra v1.10.2 github.com/stretchr/testify v1.11.1 - go.uber.org/zap v1.27.1 ) require ( @@ -22,6 +22,8 @@ require ( github.com/davecgh/go-spew v1.1.1 // indirect github.com/inconshreveable/mousetrap v1.1.0 // indirect github.com/lucasb-eyer/go-colorful v1.3.0 // indirect + github.com/mattn/go-colorable v0.1.14 // indirect + github.com/mattn/go-isatty v0.0.20 // indirect github.com/mattn/go-runewidth v0.0.20 // indirect github.com/muesli/cancelreader v0.2.2 // indirect github.com/pmezard/go-difflib v1.0.0 // indirect @@ -29,7 +31,6 @@ require ( github.com/spf13/pflag v1.0.10 // indirect github.com/stretchr/objx v0.5.3 // indirect github.com/xo/terminfo v0.0.0-20220910002029-abceb7e1c41e // indirect - go.uber.org/multierr v1.11.0 // indirect golang.org/x/sync v0.19.0 // indirect golang.org/x/sys v0.41.0 // indirect gopkg.in/yaml.v3 v3.0.1 // indirect diff --git a/go.sum b/go.sum index 667e38c..a6c6ca6 100644 --- a/go.sum +++ b/go.sum @@ -22,21 +22,34 @@ github.com/clipperhouse/displaywidth v0.11.0 h1:lBc6kY44VFw+TDx4I8opi/EtL9m20WSE github.com/clipperhouse/displaywidth v0.11.0/go.mod h1:bkrFNkf81G8HyVqmKGxsPufD3JhNl3dSqnGhOoSD/o0= github.com/clipperhouse/uax29/v2 v2.7.0 h1:+gs4oBZ2gPfVrKPthwbMzWZDaAFPGYK72F0NJv2v7Vk= github.com/clipperhouse/uax29/v2 v2.7.0/go.mod h1:EFJ2TJMRUaplDxHKj1qAEhCtQPW2tJSwu5BF98AuoVM= +github.com/coreos/go-systemd/v22 v22.5.0/go.mod h1:Y58oyj3AT4RCenI/lSvhwexgC+NSVTIJ3seZv2GcEnc= github.com/cpuguy83/go-md2man/v2 v2.0.6/go.mod h1:oOW0eioCTA6cOiMLiUPZOpcVxMig6NIQQ7OS05n1F4g= github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c= github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= +github.com/godbus/dbus/v5 v5.0.4/go.mod h1:xhWf0FNVPg57R7Z0UbKHbJfkEywrmjJnf7w5xrFpKfA= github.com/inconshreveable/mousetrap v1.1.0 h1:wN+x4NVGpMsO7ErUn/mUI3vEoE6Jt13X2s0bqwp9tc8= github.com/inconshreveable/mousetrap v1.1.0/go.mod h1:vpF70FUmC8bwa3OWnCshd2FqLfsEA9PFc4w1p2J65bw= github.com/lucasb-eyer/go-colorful v1.3.0 h1:2/yBRLdWBZKrf7gB40FoiKfAWYQ0lqNcbuQwVHXptag= github.com/lucasb-eyer/go-colorful v1.3.0/go.mod h1:R4dSotOR9KMtayYi1e77YzuveK+i7ruzyGqttikkLy0= +github.com/mattn/go-colorable v0.1.13/go.mod h1:7S9/ev0klgBDR4GtXTXX8a3vIGJpMovkB8vQcUbaXHg= +github.com/mattn/go-colorable v0.1.14 h1:9A9LHSqF/7dyVVX6g0U9cwm9pG3kP9gSzcuIPHPsaIE= +github.com/mattn/go-colorable v0.1.14/go.mod h1:6LmQG8QLFO4G5z1gPvYEzlUgJ2wF+stgPZH1UqBm1s8= +github.com/mattn/go-isatty v0.0.16/go.mod h1:kYGgaQfpe5nmfYZH+SKPsOc2e4SrIfOl2e/yFXSvRLM= +github.com/mattn/go-isatty v0.0.19/go.mod h1:W+V8PltTTMOvKvAeJH7IuucS94S2C6jfK/D7dTCTo3Y= +github.com/mattn/go-isatty v0.0.20 h1:xfD0iDuEKnDkl03q4limB+vH+GxLEtL/jb4xVJSWWEY= +github.com/mattn/go-isatty v0.0.20/go.mod h1:W+V8PltTTMOvKvAeJH7IuucS94S2C6jfK/D7dTCTo3Y= github.com/mattn/go-runewidth v0.0.20 h1:WcT52H91ZUAwy8+HUkdM3THM6gXqXuLJi9O3rjcQQaQ= github.com/mattn/go-runewidth v0.0.20/go.mod h1:XBkDxAl56ILZc9knddidhrOlY5R/pDhgLpndooCuJAs= github.com/muesli/cancelreader v0.2.2 h1:3I4Kt4BQjOR54NavqnDogx/MIoWBFa0StPA8ELUXHmA= github.com/muesli/cancelreader v0.2.2/go.mod h1:3XuTXfFS2VjM+HTLZY9Ak0l6eUKfijIfMUZ4EgX0QYo= +github.com/pkg/errors v0.9.1/go.mod h1:bwawxfHBFNV+L2hUp1rHADufV3IMtnDRdf1r5NINEl0= github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZbAQM= github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4= github.com/rivo/uniseg v0.4.7 h1:WUdvkW8uEhrYfLC4ZzdpI2ztxP1I582+49Oc5Mq64VQ= github.com/rivo/uniseg v0.4.7/go.mod h1:FN3SvrM+Zdj16jyLfmOkMNblXMcoc8DfTHruCPUcx88= +github.com/rs/xid v1.6.0/go.mod h1:7XoLgs4eV+QndskICGsho+ADou8ySMSjJKDIan90Nz0= +github.com/rs/zerolog v1.34.0 h1:k43nTLIwcTVQAncfCw4KZ2VY6ukYoZaBPNOE8txlOeY= +github.com/rs/zerolog v1.34.0/go.mod h1:bJsvje4Z08ROH4Nhs5iH600c3IkWhwp44iRc54W6wYQ= github.com/russross/blackfriday/v2 v2.1.0/go.mod h1:+Rmxgy9KzJVeS9/2gXHxylqXiyQDYRxCVz55jmeOWTM= github.com/spf13/cobra v1.10.2 h1:DMTTonx5m65Ic0GOoRY2c16WCbHxOOw6xxezuLaBpcU= github.com/spf13/cobra v1.10.2/go.mod h1:7C1pvHqHw5A4vrJfjNwvOdzYu0Gml16OCs2GRiTUUS4= @@ -49,17 +62,14 @@ github.com/stretchr/testify v1.11.1 h1:7s2iGBzp5EwR7/aIZr8ao5+dra3wiQyKjjFuvgVKu github.com/stretchr/testify v1.11.1/go.mod h1:wZwfW3scLgRK+23gO65QZefKpKQRnfz6sD981Nm4B6U= github.com/xo/terminfo v0.0.0-20220910002029-abceb7e1c41e h1:JVG44RsyaB9T2KIHavMF/ppJZNG9ZpyihvCd0w101no= github.com/xo/terminfo v0.0.0-20220910002029-abceb7e1c41e/go.mod h1:RbqR21r5mrJuqunuUZ/Dhy/avygyECGrLceyNeo4LiM= -go.uber.org/goleak v1.3.0 h1:2K3zAYmnTNqV73imy9J1T3WC+gmCePx2hEGkimedGto= -go.uber.org/goleak v1.3.0/go.mod h1:CoHD4mav9JJNrW/WLlf7HGZPjdw8EucARQHekz1X6bE= -go.uber.org/multierr v1.11.0 h1:blXXJkSxSSfBVBlC76pxqeO+LN3aDfLQo+309xJstO0= -go.uber.org/multierr v1.11.0/go.mod h1:20+QtiLqy0Nd6FdQB9TLXag12DsQkrbs3htMFfDN80Y= -go.uber.org/zap v1.27.1 h1:08RqriUEv8+ArZRYSTXy1LeBScaMpVSTBhCeaZYfMYc= -go.uber.org/zap v1.27.1/go.mod h1:GB2qFLM7cTU87MWRP2mPIjqfIDnGu+VIO4V/SdhGo2E= go.yaml.in/yaml/v3 v3.0.4/go.mod h1:DhzuOOF2ATzADvBadXxruRBLzYTpT36CKvDb3+aBEFg= golang.org/x/exp v0.0.0-20231006140011-7918f672742d h1:jtJma62tbqLibJ5sFQz8bKtEM8rJBtfilJ2qTU199MI= golang.org/x/exp v0.0.0-20231006140011-7918f672742d/go.mod h1:ldy0pHrwJyGW56pPQzzkH36rKxoZW1tw7ZJpeKx+hdo= golang.org/x/sync v0.19.0 h1:vV+1eWNmZ5geRlYjzm2adRgW2/mcpevXNg50YZtPCE4= golang.org/x/sync v0.19.0/go.mod h1:9KTHXmSnoGruLpwFjVSX0lNNA75CykiMECbovNTZqGI= +golang.org/x/sys v0.0.0-20220811171246-fbc7d0a398ab/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= +golang.org/x/sys v0.6.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= +golang.org/x/sys v0.12.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.41.0 h1:Ivj+2Cp/ylzLiEU89QhWblYnOE9zerudt9Ftecq2C6k= golang.org/x/sys v0.41.0/go.mod h1:OgkHotnGiDImocRcuBABYBEXf8A9a87e/uXjp9XT3ks= gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405 h1:yhCVgyC4o1eVCa2tZl7eS0r+SDo693bJlVdllGtEeKM= diff --git a/internal/cli/cli.go b/internal/cli/cli.go index 87450d0..3805b35 100644 --- a/internal/cli/cli.go +++ b/internal/cli/cli.go @@ -19,7 +19,6 @@ along with this program. If not, see . package cli import ( - "errors" "fmt" "os" @@ -27,9 +26,6 @@ import ( "github.com/nicholas-fedor/go-remove/internal/logger" ) -// ErrInvalidLoggerType indicates that the logger is not of the expected *ZapLogger type. -var ErrInvalidLoggerType = errors.New("logger is not a *ZapLogger") - // Config holds command-line configuration options. type Config struct { Binary string // Binary name to remove; empty for TUI mode diff --git a/internal/cli/cli_test.go b/internal/cli/cli_test.go index 8a6b60e..aa2eff2 100644 --- a/internal/cli/cli_test.go +++ b/internal/cli/cli_test.go @@ -21,285 +21,464 @@ import ( "bytes" "errors" "fmt" + "io" "os" "testing" + "github.com/rs/zerolog" "github.com/stretchr/testify/mock" - "go.uber.org/zap" - "go.uber.org/zap/zapcore" tea "charm.land/bubbletea/v2" - fsmocks "github.com/nicholas-fedor/go-remove/internal/fs/mocks" - "github.com/nicholas-fedor/go-remove/internal/logger" - logmocks "github.com/nicholas-fedor/go-remove/internal/logger/mocks" + mockRunner "github.com/nicholas-fedor/go-remove/internal/cli/mocks" + mockFS "github.com/nicholas-fedor/go-remove/internal/fs/mocks" + mockLogger "github.com/nicholas-fedor/go-remove/internal/logger/mocks" ) -// cliMockRunner mocks the ProgramRunner interface for CLI tests. -type cliMockRunner struct { - runProgram func(m tea.Model, opts ...tea.ProgramOption) (*tea.Program, error) +// mockNoOpRunner provides a no-op runner for TUI tests. +func mockNoOpRunner(tea.Model, ...tea.ProgramOption) (*tea.Program, error) { + return nil, nil //nolint:nilnil // Mock no-op runner returns nil values for test simplicity } -// RunProgram executes the mock runner’s program function. -func (m cliMockRunner) RunProgram( - model tea.Model, - opts ...tea.ProgramOption, -) (*tea.Program, error) { - return m.runProgram(model, opts...) +// newMockLoggerWithDefaults creates a MockLogger with default expectations for all methods. +// This helper reduces boilerplate when setting up logger mocks for tests that don't need +// to verify specific logger interactions. +func newMockLoggerWithDefaults(t *testing.T) *mockLogger.MockLogger { + t.Helper() + + m := mockLogger.NewMockLogger(t) + + // Create a nop logger and get event pointers to use as return values. + nopLog := zerolog.New(io.Discard) + + // Use Maybe() for optional methods that may not be called during tests. + m.On("Debug").Return(nopLog.Debug()).Maybe() + m.On("Info").Return(nopLog.Info()).Maybe() + m.On("Warn").Return(nopLog.Warn()).Maybe() + m.On("Error").Return(nopLog.Error()).Maybe() + m.On("Sync").Return(nil) + m.On("Level", mock.Anything).Return().Maybe() + m.On("SetCaptureFunc", mock.Anything).Return().Maybe() + + return m } -// mockNoOpRunner provides a no-op runner for TUI tests. -func mockNoOpRunner(tea.Model, ...tea.ProgramOption) (*tea.Program, error) { - return nil, nil //nolint:nilnil // Mock no-op runner +// captureStdout redirects os.Stdout and returns a function that restores stdout +// and returns the captured output as a string. +func captureStdout(t *testing.T) func() string { + t.Helper() + + oldStdout := os.Stdout + + r, w, err := os.Pipe() + if err != nil { + t.Fatalf("Failed to create pipe: %v", err) + } + + os.Stdout = w + + return func() string { + // Always restore os.Stdout first + os.Stdout = oldStdout + + // Close the write end and check for errors + if err := w.Close(); err != nil { + t.Errorf("Failed to close pipe writer: %v", err) + } + + // Read from pipe and check for errors + var buf bytes.Buffer + + if _, err := buf.ReadFrom(r); err != nil { + t.Errorf("Failed to read from pipe: %v", err) + } + + // Close the read end to prevent FD leak + if err := r.Close(); err != nil { + t.Errorf("Failed to close pipe reader: %v", err) + } + + return buf.String() + } +} + +// makeDeps creates Dependencies from the provided setup functions. +type makeDepsConfig struct { + setupFS func(t *testing.T) *mockFS.MockFS + setupLog func(t *testing.T) *mockLogger.MockLogger + setupRunner func(t *testing.T) *mockRunner.MockProgramRunner +} + +func makeDeps( + t *testing.T, + cfg makeDepsConfig, +) (Dependencies, *mockFS.MockFS, *mockLogger.MockLogger, ProgramRunner) { + t.Helper() + + mockFSInstance := cfg.setupFS(t) + mockLog := cfg.setupLog(t) + + deps := Dependencies{ + FS: mockFSInstance, + Logger: mockLog, + } + + var runner ProgramRunner = DefaultRunner{} + if cfg.setupRunner != nil { + runner = cfg.setupRunner(t) + } + + return deps, mockFSInstance, mockLog, runner +} + +// executeRun wraps the local run logic and returns the error. +// It mirrors the behavior of the Run function but accepts a custom runner for testing. +func executeRun(deps Dependencies, config Config, runner ProgramRunner) error { + log := deps.Logger + + binDir, err := deps.FS.DetermineBinDir(config.Goroot) + if err != nil { + _ = log.Sync() // Flush logs; errors are ignored + + return err + } + + if config.Binary == "" { + err = RunTUI(binDir, config, log, deps.FS, runner) + } else { + binaryPath := deps.FS.AdjustBinaryPath(binDir, config.Binary) + + err = deps.FS.RemoveBinary(binaryPath, config.Binary, config.Verbose, log) + if err == nil && !config.Verbose { + fmt.Fprintf(os.Stdout, "Successfully removed %s\n", config.Binary) + } + } + + if err != nil { + _ = log.Sync() // Flush logs; errors are ignored + + return err + } + + _ = log.Sync() // Errors are ignored + + return nil } -// TestRun verifies the Run function’s behavior under various conditions. +// testCase defines the structure for TestRun test cases. +type testCase struct { + name string + config Config + setupFS func(t *testing.T) *mockFS.MockFS + setupLog func(t *testing.T) *mockLogger.MockLogger + setupRunner func(t *testing.T) *mockRunner.MockProgramRunner + wantErr bool + wantOutput string // Expected stdout output for non-verbose success +} + +// runTestCase executes a single test case with the provided configuration. +// It handles stdout capture, dependency setup, execution, and assertions. +func runTestCase(t *testing.T, tt *testCase) { + t.Helper() + + // Capture stdout for output verification. + getOutput := captureStdout(t) + + // Set up dependencies and runner. + deps, mockFSInstance, mockLog, runner := makeDeps(t, makeDepsConfig{ + setupFS: tt.setupFS, + setupLog: tt.setupLog, + setupRunner: tt.setupRunner, + }) + + // Execute the run function and capture any errors. + err := executeRun(deps, tt.config, runner) + + // Capture stdout output after execution. + gotOutput := getOutput() + + // Verify error behavior matches expectations. + if (err != nil) != tt.wantErr { + t.Errorf("Run() error = %v, wantErr %v", err, tt.wantErr) + } + + // Verify stdout output for non-verbose success cases. + if tt.wantOutput != "" && gotOutput != tt.wantOutput { + t.Errorf("Run() output = %q, want %q", gotOutput, tt.wantOutput) + } + + // Assert that all mock expectations were met. + mockFSInstance.AssertExpectations(t) + mockLog.AssertExpectations(t) + + // If using a mock runner, assert its expectations as well. + if mr, ok := runner.(*mockRunner.MockProgramRunner); ok { + mr.AssertExpectations(t) + } +} + +// TestRun verifies the Run function's behavior under various conditions. func TestRun(t *testing.T) { - tests := []struct { - name string - config Config - deps Dependencies - runner ProgramRunner - wantErr bool - wantOutput string // Expected stdout output for non-verbose success - }{ + tests := []testCase{ { name: "direct removal success", config: Config{Binary: "vhs", Verbose: false, Goroot: false}, - deps: Dependencies{ - FS: func() *fsmocks.MockFS { - m := fsmocks.NewMockFS(t) - m.On("DetermineBinDir", false).Return("/bin", nil) - m.On("AdjustBinaryPath", "/bin", "vhs").Return("/bin/vhs") - m.On("RemoveBinary", "/bin/vhs", "vhs", false, mock.Anything).Return(nil) - - return m - }(), - Logger: func() *logmocks.MockLogger { - m := logmocks.NewMockLogger(t) - m.On("Sync").Return(nil) - - return m - }(), + setupFS: func(t *testing.T) *mockFS.MockFS { //nolint:thelper // Anonymous setup function, not a test helper + m := mockFS.NewMockFS(t) + m.On("DetermineBinDir", false).Return("/bin", nil) + m.On("AdjustBinaryPath", "/bin", "vhs").Return("/bin/vhs") + m.On("RemoveBinary", "/bin/vhs", "vhs", false, mock.Anything).Return(nil) + + return m }, + setupLog: newMockLoggerWithDefaults, wantErr: false, wantOutput: "Successfully removed vhs\n", }, { name: "direct removal failure", config: Config{Binary: "vhs", Verbose: false, Goroot: false}, - deps: Dependencies{ - FS: func() *fsmocks.MockFS { - m := fsmocks.NewMockFS(t) - m.On("DetermineBinDir", false).Return("/bin", nil) - m.On("AdjustBinaryPath", "/bin", "vhs").Return("/bin/vhs") - m.On("RemoveBinary", "/bin/vhs", "vhs", false, mock.Anything). - Return(errors.New("remove failed")) - - return m - }(), - Logger: func() *logmocks.MockLogger { - m := logmocks.NewMockLogger(t) - m.On("Sync").Return(nil) - - return m - }(), + setupFS: func(t *testing.T) *mockFS.MockFS { //nolint:thelper // Anonymous setup function, not a test helper + m := mockFS.NewMockFS(t) + m.On("DetermineBinDir", false).Return("/bin", nil) + m.On("AdjustBinaryPath", "/bin", "vhs").Return("/bin/vhs") + m.On("RemoveBinary", "/bin/vhs", "vhs", false, mock.Anything). + Return(errors.New("remove failed")) + + return m }, - wantErr: true, + setupLog: newMockLoggerWithDefaults, + wantErr: true, }, { name: "tui mode success", config: Config{Binary: "", Verbose: false, Goroot: false}, - deps: Dependencies{ - FS: func() *fsmocks.MockFS { - m := fsmocks.NewMockFS(t) - m.On("DetermineBinDir", false).Return("/bin", nil) - m.On("ListBinaries", "/bin").Return([]string{"vhs"}) - - return m - }(), - Logger: func() *logmocks.MockLogger { - m := logmocks.NewMockLogger(t) - m.On("Sync").Return(nil) - - return m - }(), + setupFS: func(t *testing.T) *mockFS.MockFS { //nolint:thelper // Anonymous setup function, not a test helper + m := mockFS.NewMockFS(t) + m.On("DetermineBinDir", false).Return("/bin", nil) + m.On("ListBinaries", "/bin").Return([]string{"vhs"}) + + return m + }, + setupLog: newMockLoggerWithDefaults, + setupRunner: func(t *testing.T) *mockRunner.MockProgramRunner { //nolint:thelper // Anonymous setup function, not a test helper + m := mockRunner.NewMockProgramRunner(t) + m.On("RunProgram", mock.Anything, mock.Anything).Return(nil, nil) + + return m }, - runner: &cliMockRunner{runProgram: mockNoOpRunner}, wantErr: false, }, { name: "tui mode no binaries", config: Config{Binary: "", Verbose: false, Goroot: false}, - deps: Dependencies{ - FS: func() *fsmocks.MockFS { - m := fsmocks.NewMockFS(t) - m.On("DetermineBinDir", false).Return("/bin", nil) - m.On("ListBinaries", "/bin").Return([]string{}) - - return m - }(), - Logger: func() *logmocks.MockLogger { - m := logmocks.NewMockLogger(t) - m.On("Sync").Return(nil) - - return m - }(), + setupFS: func(t *testing.T) *mockFS.MockFS { //nolint:thelper // Anonymous setup function, not a test helper + m := mockFS.NewMockFS(t) + m.On("DetermineBinDir", false).Return("/bin", nil) + m.On("ListBinaries", "/bin").Return([]string{}) + + return m + }, + setupLog: newMockLoggerWithDefaults, + setupRunner: func(t *testing.T) *mockRunner.MockProgramRunner { //nolint:thelper // Anonymous setup function, not a test helper + m := mockRunner.NewMockProgramRunner(t) + // RunProgram may not be called if RunTUI returns an error early + m.On("RunProgram", mock.Anything, mock.Anything).Return(nil, nil).Maybe() + + return m }, - runner: &cliMockRunner{runProgram: mockNoOpRunner}, wantErr: true, }, { name: "bin dir error", config: Config{Binary: "vhs", Verbose: false, Goroot: false}, - deps: Dependencies{ - FS: func() *fsmocks.MockFS { - m := fsmocks.NewMockFS(t) - m.On("DetermineBinDir", false).Return("", errors.New("bin dir failed")) - - return m - }(), - Logger: func() *logmocks.MockLogger { - m := logmocks.NewMockLogger(t) - m.On("Sync").Return(nil) - - return m - }(), + setupFS: func(t *testing.T) *mockFS.MockFS { //nolint:thelper // Anonymous setup function, not a test helper + m := mockFS.NewMockFS(t) + m.On("DetermineBinDir", false).Return("", errors.New("bin dir failed")) + + return m }, - wantErr: true, + setupLog: newMockLoggerWithDefaults, + wantErr: true, }, { name: "logger sync error", config: Config{Binary: "vhs", Verbose: false, Goroot: false}, - deps: Dependencies{ - FS: func() *fsmocks.MockFS { - m := fsmocks.NewMockFS(t) - m.On("DetermineBinDir", false).Return("/bin", nil) - m.On("AdjustBinaryPath", "/bin", "vhs").Return("/bin/vhs") - m.On("RemoveBinary", "/bin/vhs", "vhs", false, mock.Anything).Return(nil) - - return m - }(), - Logger: func() *logmocks.MockLogger { - m := logmocks.NewMockLogger(t) - m.On("Sync").Return(errors.New("sync failed")) - - return m - }(), + setupFS: func(t *testing.T) *mockFS.MockFS { //nolint:thelper // Anonymous setup function, not a test helper + m := mockFS.NewMockFS(t) + m.On("DetermineBinDir", false).Return("/bin", nil) + m.On("AdjustBinaryPath", "/bin", "vhs").Return("/bin/vhs") + m.On("RemoveBinary", "/bin/vhs", "vhs", false, mock.Anything).Return(nil) + + return m + }, + setupLog: func(t *testing.T) *mockLogger.MockLogger { //nolint:thelper // Anonymous setup function, not a test helper + m := mockLogger.NewMockLogger(t) + nopLog := zerolog.New(io.Discard) + m.On("Debug").Return(nopLog.Debug()).Maybe() + m.On("Info").Return(nopLog.Info()).Maybe() + m.On("Warn").Return(nopLog.Warn()).Maybe() + m.On("Error").Return(nopLog.Error()).Maybe() + m.On("Sync").Return(errors.New("sync failed")) + m.On("Level", mock.Anything).Return().Maybe() + m.On("SetCaptureFunc", mock.Anything).Return().Maybe() + + return m }, wantErr: false, // Sync errors are ignored on all platforms wantOutput: "Successfully removed vhs\n", }, } + for i := range tests { + t.Run(tests[i].name, func(t *testing.T) { + runTestCase(t, &tests[i]) + }) + } +} + +// TestRun_WithLoggerSync verifies that Sync is called appropriately. +// +//nolint:thelper // Subtest functions in table-driven tests require *testing.T parameter for mock constructors +func TestRun_WithLoggerSync(t *testing.T) { + tests := []struct { + name string + config Config + setupFS func(t *testing.T) *mockFS.MockFS + setupLog func(t *testing.T) *mockLogger.MockLogger + wantErr bool + wantOutput string + }{ + { + name: "sync called on success", + config: Config{Binary: "tool", Verbose: false, Goroot: false}, + setupFS: func(t *testing.T) *mockFS.MockFS { + m := mockFS.NewMockFS(t) + m.On("DetermineBinDir", false).Return("/bin", nil) + m.On("AdjustBinaryPath", "/bin", "tool").Return("/bin/tool") + m.On("RemoveBinary", "/bin/tool", "tool", false, mock.Anything).Return(nil) + + return m + }, + setupLog: func(t *testing.T) *mockLogger.MockLogger { + m := mockLogger.NewMockLogger(t) + nopLog := zerolog.New(io.Discard) + m.On("Debug").Return(nopLog.Debug()).Maybe() + m.On("Info").Return(nopLog.Info()).Maybe() + m.On("Warn").Return(nopLog.Warn()).Maybe() + m.On("Error").Return(nopLog.Error()).Maybe() + m.On("Sync").Return(nil) + + return m + }, + wantErr: false, + wantOutput: "Successfully removed tool\n", + }, + { + name: "sync called on error", + config: Config{Binary: "tool", Verbose: false, Goroot: false}, + setupFS: func(t *testing.T) *mockFS.MockFS { + m := mockFS.NewMockFS(t) + m.On("DetermineBinDir", false).Return("", errors.New("bin dir error")) + + return m + }, + setupLog: func(t *testing.T) *mockLogger.MockLogger { + m := mockLogger.NewMockLogger(t) + nopLog := zerolog.New(io.Discard) + m.On("Debug").Return(nopLog.Debug()).Maybe() + m.On("Info").Return(nopLog.Info()).Maybe() + m.On("Warn").Return(nopLog.Warn()).Maybe() + m.On("Error").Return(nopLog.Error()).Maybe() + m.On("Sync").Return(nil) + + return m + }, + wantErr: true, + }, + } + for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - // Redirect stdout to capture output for non-verbose success cases. - oldStdout := os.Stdout - r, w, _ := os.Pipe() - os.Stdout = w - - defer func() { - os.Stdout = oldStdout - - w.Close() - }() - - // Define a local run function to test with a custom runner. - run := func(deps Dependencies, config Config, runner ProgramRunner) error { - log := deps.Logger - - // Set log level based on config if verbose mode is enabled. - if config.Verbose { - zapLogger, ok := log.(*logger.ZapLogger) - if !ok { - return fmt.Errorf( - "failed to set log level: %w with type %T", - ErrInvalidLoggerType, - log, - ) - } - - switch config.LogLevel { - case "debug": - zapLogger.Logger = zapLogger.WithOptions( - zap.IncreaseLevel(zapcore.DebugLevel), - ) - case "warn": - zapLogger.Logger = zapLogger.WithOptions( - zap.IncreaseLevel(zapcore.WarnLevel), - ) - case "error": - zapLogger.Logger = zapLogger.WithOptions( - zap.IncreaseLevel(zapcore.ErrorLevel), - ) - default: - zapLogger.Logger = zapLogger.WithOptions( - zap.IncreaseLevel(zapcore.InfoLevel), - ) - } - - log = zapLogger - } - - binDir, err := deps.FS.DetermineBinDir(config.Goroot) - if err != nil { - _ = log.Sync() // Flush logs; errors are ignored - - return err - } - - if config.Binary == "" { - err = RunTUI(binDir, config, log, deps.FS, runner) - } else { - binaryPath := deps.FS.AdjustBinaryPath(binDir, config.Binary) - - err = deps.FS.RemoveBinary(binaryPath, config.Binary, config.Verbose, log) - if err == nil && !config.Verbose { - fmt.Fprintf(os.Stdout, "Successfully removed %s\n", config.Binary) - } - } - - if err != nil { - _ = log.Sync() // Flush logs; errors are ignored - - return err - } - - _ = log.Sync() // Errors are ignored - - return nil - } + // Capture stdout for output verification. + getOutput := captureStdout(t) + + // Set up dependencies. + mockFSInstance := tt.setupFS(t) + mockLog := tt.setupLog(t) - // Use DefaultRunner if no mock is provided. - runner := tt.runner - if runner == nil { - runner = DefaultRunner{} + deps := Dependencies{ + FS: mockFSInstance, + Logger: mockLog, } - // Execute the run function and capture any errors. - err := run(tt.deps, tt.config, runner) + // Execute the Run function and capture any errors. + err := Run(deps, tt.config) // Capture stdout output after execution. - w.Close() - - var buf bytes.Buffer - buf.ReadFrom(r) - gotOutput := buf.String() + gotOutput := getOutput() // Verify error behavior matches expectations. if (err != nil) != tt.wantErr { t.Errorf("Run() error = %v, wantErr %v", err, tt.wantErr) } - // Verify stdout output for non-verbose success cases. + // Verify stdout output for success cases. if tt.wantOutput != "" && gotOutput != tt.wantOutput { t.Errorf("Run() output = %q, want %q", gotOutput, tt.wantOutput) } + // Assert that Sync was called on the mock logger. + mockLog.AssertCalled(t, "Sync") + // Assert that all mock expectations were met. - tt.deps.FS.(*fsmocks.MockFS).AssertExpectations(t) - tt.deps.Logger.(*logmocks.MockLogger).AssertExpectations(t) + mockFSInstance.AssertExpectations(t) + mockLog.AssertExpectations(t) }) } } + +// TestRun_VerboseMode verifies verbose mode behavior. +func TestRun_VerboseMode(t *testing.T) { + m := mockFS.NewMockFS(t) + m.On("DetermineBinDir", false).Return("/bin", nil) + m.On("AdjustBinaryPath", "/bin", "vhs").Return("/bin/vhs") + m.On("RemoveBinary", "/bin/vhs", "vhs", true, mock.Anything).Return(nil) + + mockLog := mockLogger.NewMockLogger(t) + nopLog := zerolog.New(io.Discard) + mockLog.On("Debug").Return(nopLog.Debug()).Maybe() + mockLog.On("Info").Return(nopLog.Info()).Maybe() + mockLog.On("Warn").Return(nopLog.Warn()).Maybe() + mockLog.On("Error").Return(nopLog.Error()).Maybe() + mockLog.On("Level", mock.Anything).Return().Maybe() + mockLog.On("Sync").Return(nil) + + deps := Dependencies{ + FS: m, + Logger: mockLog, + } + config := Config{Binary: "vhs", Verbose: true, Goroot: false} + + // Capture stdout to capture output. + getOutput := captureStdout(t) + + // Execute the Run function and capture any errors. + err := Run(deps, config) + + // Restore stdout and get captured output. + gotOutput := getOutput() + + if err != nil { + t.Errorf("Unexpected error: %v", err) + } + + // In verbose mode, no success message should be printed to stdout. + if gotOutput != "" { + t.Errorf("Expected no stdout output in verbose mode, got: %q", gotOutput) + } + + // Assert that all mock expectations were met. + m.AssertExpectations(t) + mockLog.AssertExpectations(t) +} diff --git a/internal/cli/tui.go b/internal/cli/tui.go index 02bb09b..ff36c6b 100644 --- a/internal/cli/tui.go +++ b/internal/cli/tui.go @@ -34,17 +34,26 @@ import ( // Layout constants for TUI rendering. const ( - colWidthPadding = 3 // Padding added to column width for spacing - availWidthAdjustment = 4 // Adjustment to width for border and padding - minAvailHeightAdjustment = 7 // Minimum height adjustment for UI elements - visibleLenPrefix = 2 // Prefix length for cursor visibility - totalHeightBase = 4 // Base height for non-grid UI components - leftPadding = 2 // Left padding for the entire TUI + colWidthPadding = 3 // Padding added to column width for spacing + availWidthAdjustment = 4 // Adjustment to width for border and padding + minAvailHeightAdjustment = 7 // Minimum height adjustment for UI elements + visibleLenPrefix = 2 // Prefix length for cursor visibility + totalHeightBase = 4 // Base height for non-grid UI components + leftPadding = 2 // Left padding for the entire TUI + maxLogLines = 50 // Maximum number of log lines to retain + maxVisibleLogLines = 5 // Maximum number of log lines to display + logPanelSeparatorLines = 2 // Number of separator lines for log panel ) // ErrNoBinariesFound signals that no binaries were found in the target directory. var ErrNoBinariesFound = errors.New("no binaries found in directory") +// LogMsg is a Bubble Tea message that carries a log entry to be displayed in the TUI. +type LogMsg struct { + Level string // Log level (e.g., "DBG", "INF", "WRN", "ERR") + Message string // Log message content +} + // ProgramRunner defines an interface for running Bubbletea programs. type ProgramRunner interface { RunProgram(m tea.Model, opts ...tea.ProgramOption) (*tea.Program, error) @@ -56,6 +65,7 @@ type styleConfig struct { CursorColor string // ANSI 256-color code for cursor FooterColor string // ANSI 256-color code for footer StatusColor string // ANSI 256-color code for status + LogColor string // ANSI 256-color code for log messages Cursor string // Symbol used for the cursor } @@ -75,11 +85,19 @@ type model struct { status string // Status message styles styleConfig // TUI appearance settings sortAscending bool // True for ascending sort, false for descending + logs []string // Captured log messages (circular buffer) + showLogs bool // Toggle log panel visibility + logChan chan LogMsg // Channel for receiving log messages from the logger } // DefaultRunner provides the default Bubbletea program runner. type DefaultRunner struct{} +// NewLogMsg creates a new LogMsg for sending log entries to the TUI. +func NewLogMsg(level, message string) LogMsg { + return LogMsg{Level: level, Message: message} +} + // RunTUI launches the interactive TUI mode for binary selection and removal. func RunTUI( dir string, @@ -94,8 +112,9 @@ func RunTUI( return fmt.Errorf("%w: %s", ErrNoBinariesFound, dir) } - // Initialize and start the TUI program with default styles. - program, err := runner.RunProgram(&model{ + // Initialize the model with default styles. + // Enable log visibility by default when verbose mode is active. + m := &model{ choices: choices, dir: dir, config: config, @@ -105,7 +124,28 @@ func RunTUI( cursorY: 0, sortAscending: true, styles: defaultStyleConfig(), - }) + logs: make([]string, 0, maxLogLines), + showLogs: config.Verbose, + } + + // Set up log capture channel if verbose mode is enabled. + // This must be done before starting the program to ensure capture is ready. + if config.Verbose { + m.logChan = make(chan LogMsg, maxLogLines) + + log.SetCaptureFunc(func(level, msg string) { + // Send to channel without blocking. + // If channel is full, the message is dropped to prevent blocking. + select { + case m.logChan <- NewLogMsg(level, msg): + default: + // Channel is full, message is dropped to prevent blocking. + } + }) + } + + // Start the TUI program. + program, err := runner.RunProgram(m) if err != nil { return fmt.Errorf("failed to start TUI program: %w", err) } @@ -131,6 +171,7 @@ func defaultStyleConfig() styleConfig { CursorColor: "214", // Orange FooterColor: "245", // Light gray StatusColor: "46", // Lime green + LogColor: "240", // Dark gray for subtle log display Cursor: "❯ ", } } @@ -146,9 +187,31 @@ func (r DefaultRunner) RunProgram(m tea.Model, opts ...tea.ProgramOption) (*tea. func (m *model) Init() tea.Cmd { m.sortChoices() + // Start log polling if verbose mode is enabled. + if m.config.Verbose && m.logChan != nil { + return m.pollLogChannel() + } + return nil } +// pollLogChannel returns a command that polls for log messages. +// This approach avoids deadlocks by having the TUI poll the channel +// rather than trying to Send() from within the capture callback. +func (m *model) pollLogChannel() tea.Cmd { + return func() tea.Msg { + // Check if there's a message on the channel. + // Use a non-blocking receive to avoid hanging. + select { + case msg := <-m.logChan: + return msg + default: + // No message available, schedule next poll. + return nil + } + } +} + // Update processes TUI events and updates the model state. func (m *model) Update(msg tea.Msg) (tea.Model, tea.Cmd) { switch msg := msg.(type) { @@ -194,6 +257,11 @@ func (m *model) Update(msg tea.Msg) (tea.Model, tea.Cmd) { m.sortChoices() m.updateGrid() + case "L": + // Toggle log panel visibility. + m.showLogs = !m.showLogs + m.updateGrid() + case "enter": // Remove the selected binary and update the TUI state. if len(m.choices) > 0 { @@ -236,11 +304,58 @@ func (m *model) Update(msg tea.Msg) (tea.Model, tea.Cmd) { m.width = msg.Width m.height = msg.Height m.updateGrid() + case LogMsg: + // Add log message to the circular buffer. + m.addLogEntry(msg) + m.updateGrid() + + // Continue polling for more log messages. + // This ensures all pending logs are captured. + cmd := m.pollLogChannel() + + return m, cmd + } + + // Continue polling for log messages if in verbose mode. + // This non-blocking poll ensures logs are captured without deadlocks. + if m.config.Verbose && m.logChan != nil { + cmd := m.pollLogChannel() + + return m, cmd } return m, nil } +// addLogEntry adds a log message to the circular buffer, maintaining maxLogLines limit. +func (m *model) addLogEntry(msg LogMsg) { + // Format the log entry: "[LEVEL] message" + entry := fmt.Sprintf("[%s] %s", msg.Level, msg.Message) + + // Add to logs slice + m.logs = append(m.logs, entry) + + // Maintain circular buffer size + if len(m.logs) > maxLogLines { + m.logs = m.logs[len(m.logs)-maxLogLines:] + } +} + +// getVisibleLogs returns the last N log lines that fit within the available height. +func (m *model) getVisibleLogs(maxLines int) []string { + if !m.showLogs || len(m.logs) == 0 || maxLines <= 0 { + return nil + } + + // Return the last maxLines entries + start := len(m.logs) - maxLines + if start < 0 { + start = 0 + } + + return m.logs[start:] +} + // sortChoices sorts the choices based on the current sort order. func (m *model) sortChoices() { if len(m.choices) == 0 { @@ -269,6 +384,13 @@ func (m *model) updateGrid() { availWidth := m.width - availWidthAdjustment availHeight := maximum(m.height-minAvailHeightAdjustment, 1) + // Adjust available height for log panel if visible + if m.showLogs && len(m.logs) > 0 { + // Reserve up to maxVisibleLogLines lines for log panel (plus 1 for separator) + logPanelHeight := minimum(len(m.logs), maxVisibleLogLines) + 1 + availHeight = maximum(availHeight-logPanelHeight, 1) + } + // Clear grid if no choices remain. if len(m.choices) == 0 { m.rows = 0 @@ -320,6 +442,7 @@ func (m *model) View() tea.View { cursorStyle := lipgloss.NewStyle().Foreground(lipgloss.Color(m.styles.CursorColor)) footerStyle := lipgloss.NewStyle().Foreground(lipgloss.Color(m.styles.FooterColor)) statusStyle := lipgloss.NewStyle().Foreground(lipgloss.Color(m.styles.StatusColor)) + logStyle := lipgloss.NewStyle().Foreground(lipgloss.Color(m.styles.LogColor)) // Calculate column width based on the longest binary name. var maxNameLen int @@ -356,7 +479,7 @@ func (m *model) View() tea.View { grid.WriteString("\n") } - // Assemble the full TUI layout: title, grid, status, and footer. + // Assemble the full TUI layout: title, grid, logs (if visible), status, and footer. var s strings.Builder s.WriteString(titleStyle.Render("Select a binary to remove:\n")) @@ -364,21 +487,50 @@ func (m *model) View() tea.View { s.WriteString(grid.String()) s.WriteString("\n") + // Render log panel if enabled and logs exist + if m.showLogs && len(m.logs) > 0 { + visibleLogs := m.getVisibleLogs( + maxVisibleLogLines, + ) // Show up to maxVisibleLogLines log lines + if len(visibleLogs) > 0 { + s.WriteString(logStyle.Render("─ Log Messages ─")) + s.WriteString("\n") + + for _, logEntry := range visibleLogs { + s.WriteString(logStyle.Render(logEntry)) + s.WriteString("\n") + } + + s.WriteString("\n") + } + } + if m.status != "" { s.WriteString(statusStyle.Render(m.status)) s.WriteString("\n") } - footer := footerStyle.Render( - "↑/k: up ↓/j: down ←/h: left →/l: right Enter: remove s: toggle sort q: quit", - ) + // Update footer to include L key for toggling logs + footerText := "↑/k: up ↓/j: down ←/h: left →/l: right Enter: remove s: toggle sort L: logs q: quit" + footer := footerStyle.Render(footerText) lenStatus := 0 if m.status != "" { lenStatus = 1 } - totalHeight := m.rows + totalHeightBase + lenStatus + // Account for log panel in height calculation + logPanelLines := 0 + + if m.showLogs && len(m.logs) > 0 { + visibleLogs := m.getVisibleLogs(maxVisibleLogLines) + if len(visibleLogs) > 0 { + // Log panel header + log lines + separator + logPanelLines = len(visibleLogs) + logPanelSeparatorLines + } + } + + totalHeight := m.rows + totalHeightBase + lenStatus + logPanelLines // Add padding lines to fill the terminal height. for i := totalHeight; i < m.height; i++ { diff --git a/internal/cli/tui_test.go b/internal/cli/tui_test.go index d8edf4e..ca448f2 100644 --- a/internal/cli/tui_test.go +++ b/internal/cli/tui_test.go @@ -25,14 +25,47 @@ import ( "testing" "unicode/utf8" + "github.com/rs/zerolog" "github.com/stretchr/testify/mock" tea "charm.land/bubbletea/v2" - fsmocks "github.com/nicholas-fedor/go-remove/internal/fs/mocks" - logmocks "github.com/nicholas-fedor/go-remove/internal/logger/mocks" + mockFS "github.com/nicholas-fedor/go-remove/internal/fs/mocks" + "github.com/nicholas-fedor/go-remove/internal/logger" ) +// tuiMockLogger is a simple mock logger for TUI tests. +type tuiMockLogger struct { + nopLogger zerolog.Logger +} + +func (m *tuiMockLogger) Debug() *zerolog.Event { + m.nopLogger.Debug().Msg("") + + return nil +} + +func (m *tuiMockLogger) Info() *zerolog.Event { + m.nopLogger.Info().Msg("") + + return nil +} + +func (m *tuiMockLogger) Warn() *zerolog.Event { + m.nopLogger.Warn().Msg("") + + return nil +} + +func (m *tuiMockLogger) Error() *zerolog.Event { + m.nopLogger.Error().Msg("") + + return nil +} +func (m *tuiMockLogger) Sync() error { return nil } +func (m *tuiMockLogger) Level(_ zerolog.Level) {} +func (m *tuiMockLogger) SetCaptureFunc(_ logger.LogCaptureFunc) {} + // tuiMockRunner mocks the ProgramRunner interface for TUI tests. type tuiMockRunner struct { runProgram func(tea.Model, ...tea.ProgramOption) (*tea.Program, error) @@ -51,8 +84,8 @@ func TestRunTUI(t *testing.T) { type args struct { dir string config Config - logger *logmocks.MockLogger - fs *fsmocks.MockFS + logger logger.Logger + fs *mockFS.MockFS runner ProgramRunner } @@ -66,9 +99,9 @@ func TestRunTUI(t *testing.T) { args: args{ dir: "/bin", config: Config{}, - logger: logmocks.NewMockLogger(t), - fs: func() *fsmocks.MockFS { - m := fsmocks.NewMockFS(t) + logger: &tuiMockLogger{}, + fs: func() *mockFS.MockFS { + m := mockFS.NewMockFS(t) m.On("ListBinaries", "/bin").Return([]string{"vhs"}) return m @@ -82,9 +115,9 @@ func TestRunTUI(t *testing.T) { args: args{ dir: "/bin", config: Config{}, - logger: logmocks.NewMockLogger(t), - fs: func() *fsmocks.MockFS { - m := fsmocks.NewMockFS(t) + logger: &tuiMockLogger{}, + fs: func() *mockFS.MockFS { + m := mockFS.NewMockFS(t) m.On("ListBinaries", "/bin").Return([]string{}) return m @@ -98,9 +131,9 @@ func TestRunTUI(t *testing.T) { args: args{ dir: "/bin", config: Config{}, - logger: logmocks.NewMockLogger(t), - fs: func() *fsmocks.MockFS { - m := fsmocks.NewMockFS(t) + logger: &tuiMockLogger{}, + fs: func() *mockFS.MockFS { + m := mockFS.NewMockFS(t) m.On("ListBinaries", "/bin").Return([]string{"vhs"}) return m @@ -123,7 +156,6 @@ func TestRunTUI(t *testing.T) { } tt.args.fs.AssertExpectations(t) - tt.args.logger.AssertExpectations(t) }) } } @@ -301,13 +333,9 @@ func Test_model_Update(t *testing.T) { sortAscending: true, width: 80, height: 24, - logger: func() *logmocks.MockLogger { - m := logmocks.NewMockLogger(t) - - return m - }(), - fs: func() *fsmocks.MockFS { - m := fsmocks.NewMockFS(t) + logger: &tuiMockLogger{}, + fs: func() *mockFS.MockFS { + m := mockFS.NewMockFS(t) m.On("AdjustBinaryPath", "/bin", "age").Return("/bin/age") m.On("RemoveBinary", "/bin/age", "age", false, mock.Anything).Return(nil) m.On("ListBinaries", "/bin").Return([]string{"vhs"}) @@ -339,13 +367,9 @@ func Test_model_Update(t *testing.T) { sortAscending: true, width: 80, height: 24, - logger: func() *logmocks.MockLogger { - m := logmocks.NewMockLogger(t) - - return m - }(), - fs: func() *fsmocks.MockFS { - m := fsmocks.NewMockFS(t) + logger: &tuiMockLogger{}, + fs: func() *mockFS.MockFS { + m := mockFS.NewMockFS(t) m.On("AdjustBinaryPath", "/bin", "age").Return("/bin/age") m.On("RemoveBinary", "/bin/age", "age", false, mock.Anything). Return(errors.New("remove failed")) @@ -385,11 +409,11 @@ func Test_model_Update(t *testing.T) { t.Run(tt.name, func(t *testing.T) { // Set default mocks if not provided. if tt.m.logger == nil { - tt.m.logger = logmocks.NewMockLogger(t) + tt.m.logger = &tuiMockLogger{} } if tt.m.fs == nil { - tt.m.fs = fsmocks.NewMockFS(t) + tt.m.fs = mockFS.NewMockFS(t) } got, gotCmd := tt.m.Update(tt.args.msg) @@ -413,8 +437,7 @@ func Test_model_Update(t *testing.T) { t.Errorf("model.Update() gotCmd = %T, want %T", gotCmd, tt.wantCmd) } - tt.m.fs.(*fsmocks.MockFS).AssertExpectations(t) - tt.m.logger.(*logmocks.MockLogger).AssertExpectations(t) + tt.m.fs.(*mockFS.MockFS).AssertExpectations(t) }) } } @@ -554,8 +577,8 @@ func Test_model_View(t *testing.T) { lines = append(lines, leftPaddingStr+pad("", effectiveWidth)) } - footerPart1 := "↑/k: up ↓/j: down ←/h: left →/l: right Enter: remove s: toggle sort q:" - footerPart2 := "quit" + footerPart1 := "↑/k: up ↓/j: down ←/h: left →/l: right Enter: remove s: toggle sort L:" + footerPart2 := "logs q: quit" lines = append( lines, @@ -596,8 +619,8 @@ func Test_model_View(t *testing.T) { lines = append(lines, leftPaddingStr+pad("", effectiveWidth)) } - footerPart1 := "↑/k: up ↓/j: down ←/h: left →/l: right Enter: remove s: toggle sort q:" - footerPart2 := "quit" + footerPart1 := "↑/k: up ↓/j: down ←/h: left →/l: right Enter: remove s: toggle sort L:" + footerPart2 := "logs q: quit" lines = append( lines, diff --git a/internal/fs/fs.go b/internal/fs/fs.go index 82274b3..0c7e12c 100644 --- a/internal/fs/fs.go +++ b/internal/fs/fs.go @@ -109,10 +109,9 @@ func (r *RealFS) RemoveBinary(binaryPath, name string, verbose bool, log logger. } // Log debug and info messages if verbose mode is enabled. - sugar := log.Sugar() if verbose { - sugar.Debugf("Constructed binary path: %s", binaryPath) - sugar.Infof("Removing binary: %s", binaryPath) + log.Debug().Msgf("Constructed binary path: %s", binaryPath) + log.Info().Msgf("Removing binary: %s", binaryPath) } // Perform the removal operation and handle any errors. @@ -122,7 +121,7 @@ func (r *RealFS) RemoveBinary(binaryPath, name string, verbose bool, log logger. // Log success if verbose mode is enabled. if verbose { - sugar.Infof("Successfully removed binary: %s", name) + log.Info().Msgf("Successfully removed binary: %s", name) } return nil diff --git a/internal/fs/fs_test.go b/internal/fs/fs_test.go index c262d4d..23d4c68 100644 --- a/internal/fs/fs_test.go +++ b/internal/fs/fs_test.go @@ -25,13 +25,20 @@ import ( "sort" "testing" - "go.uber.org/zap" + "github.com/rs/zerolog" "github.com/nicholas-fedor/go-remove/internal/logger" - logmocks "github.com/nicholas-fedor/go-remove/internal/logger/mocks" + "github.com/nicholas-fedor/go-remove/internal/logger/mocks" ) -// TestNewRealFS verifies the NewRealFS function’s instance creation. +// nopLogger creates a mock logger for testing. +func nopLogger(t *testing.T) logger.Logger { + t.Helper() + + return mocks.NewMockLogger(t) +} + +// TestNewRealFS verifies the NewRealFS function's instance creation. func TestNewRealFS(t *testing.T) { tests := []struct { name string @@ -52,7 +59,7 @@ func TestNewRealFS(t *testing.T) { } } -// TestRealFS_DetermineBinDir verifies the DetermineBinDir method’s directory resolution. +// TestRealFS_DetermineBinDir verifies the DetermineBinDir method's directory resolution. func TestRealFS_DetermineBinDir(t *testing.T) { type args struct { useGoroot bool @@ -118,7 +125,7 @@ func TestRealFS_DetermineBinDir(t *testing.T) { t.Setenv(k, v) } - // Adjust expected path for the platform’s HOME directory. + // Adjust expected path for the platform's HOME directory. if tt.name == "use default ~/go/bin when GOBIN and GOPATH unset" { home := os.Getenv("HOME") if runtime.GOOS == windowsOS { @@ -144,7 +151,7 @@ func TestRealFS_DetermineBinDir(t *testing.T) { } } -// TestRealFS_AdjustBinaryPath verifies the AdjustBinaryPath method’s path construction. +// TestRealFS_AdjustBinaryPath verifies the AdjustBinaryPath method's path construction. func TestRealFS_AdjustBinaryPath(t *testing.T) { type args struct { dir string @@ -199,13 +206,13 @@ func TestRealFS_AdjustBinaryPath(t *testing.T) { } } -// TestRealFS_RemoveBinary verifies the RemoveBinary method’s file removal behavior. +// TestRealFS_RemoveBinary verifies the RemoveBinary method's file removal behavior. func TestRealFS_RemoveBinary(t *testing.T) { type args struct { binaryPath string name string verbose bool - logger logger.Logger + logger func(t *testing.T) logger.Logger // Factory function to create logger } tests := []struct { @@ -221,12 +228,7 @@ func TestRealFS_RemoveBinary(t *testing.T) { args: args{ name: "testbin", verbose: false, - logger: func() *logmocks.MockLogger { - m := logmocks.NewMockLogger(t) - m.On("Sugar").Return(zap.NewNop().Sugar()).Maybe() - - return m - }(), + logger: nopLogger, }, setup: func() string { tmpDir := t.TempDir() @@ -244,12 +246,7 @@ func TestRealFS_RemoveBinary(t *testing.T) { binaryPath: "/nonexistent/testbin", name: "testbin", verbose: false, - logger: func() *logmocks.MockLogger { - m := logmocks.NewMockLogger(t) - m.On("Sugar").Return(zap.NewNop().Sugar()).Maybe() - - return m - }(), + logger: nopLogger, }, wantErr: true, }, @@ -259,13 +256,17 @@ func TestRealFS_RemoveBinary(t *testing.T) { args: args{ name: "testbin", verbose: true, - logger: func() *logmocks.MockLogger { - m := logmocks.NewMockLogger(t) - // Expect Sugar() to be called at least once, without strict count - m.On("Sugar").Return(zap.NewNop().Sugar()).Maybe() + logger: func(t *testing.T) logger.Logger { + t.Helper() - return m - }(), + // Create mock with expectations for verbose logging. + log := mocks.NewMockLogger(t) + zl := zerolog.New(nil).With().Logger() + log.On("Debug").Return(zl.Debug()) + log.On("Info").Return(zl.Info()) + + return log + }, }, setup: func() string { tmpDir := t.TempDir() @@ -289,24 +290,46 @@ func TestRealFS_RemoveBinary(t *testing.T) { tt.args.binaryPath, tt.args.name, tt.args.verbose, - tt.args.logger, + tt.args.logger(t), // Call factory to get logger ) if (err != nil) != tt.wantErr { t.Errorf("RemoveBinary() error = %v, wantErr %v", err, tt.wantErr) } - - // Assert that all mock expectations were met and log detailed expectations for debugging. - if tt.args.logger != nil { - mockLogger := tt.args.logger.(*logmocks.MockLogger) - t.Logf("Mock expectations for %s: %v", tt.name, mockLogger.ExpectedCalls) - t.Logf("Mock calls made for %s: %v", tt.name, mockLogger.Calls) - mockLogger.AssertExpectations(t) - } }) } } -// TestRealFS_ListBinaries verifies the ListBinaries method’s directory listing. +// TestRealFS_RemoveBinary_VerboseLogging verifies verbose logging behavior. +func TestRealFS_RemoveBinary_VerboseLogging(t *testing.T) { + log := mocks.NewMockLogger(t) + + zl := zerolog.New(nil).With().Logger() + + // Set up expectations for verbose logging calls. + log.On("Debug").Return(zl.Debug()) + log.On("Info").Return(zl.Info()) + + tmpDir := t.TempDir() + tmpFile := filepath.Join(tmpDir, "testbin") + + err := os.WriteFile(tmpFile, []byte("test"), 0o600) + if err != nil { + t.Fatalf("Failed to create test file: %v", err) + } + + fs := &RealFS{} + + err = fs.RemoveBinary(tmpFile, "testbin", true, log) + if err != nil { + t.Errorf("RemoveBinary() unexpected error = %v", err) + } + + // Verify that Debug and Info were called. + log.AssertCalled(t, "Debug") + log.AssertCalled(t, "Info") +} + +// TestRealFS_ListBinaries verifies the ListBinaries method's directory listing. func TestRealFS_ListBinaries(t *testing.T) { type args struct { dir string diff --git a/internal/logger/logger.go b/internal/logger/logger.go index 4359904..e466a1e 100644 --- a/internal/logger/logger.go +++ b/internal/logger/logger.go @@ -21,99 +21,338 @@ package logger import ( "errors" "fmt" + "io" "os" + "strings" + "sync" + "time" - "go.uber.org/zap" - "go.uber.org/zap/zapcore" -) - -// Sampling constants for logger configuration. -const ( - samplingInitial = 100 // Initial number of log entries before sampling - samplingThereafter = 100 // Number of log entries to sample after initial + "github.com/rs/zerolog" ) // ErrLoggerNil indicates that the logger instance is nil when an operation is attempted. var ErrLoggerNil = errors.New("logger is nil") +// LogCaptureFunc is a callback function that receives captured log messages. +// The level parameter contains the log level string (e.g., "DBG", "INF", "WRN", "ERR"). +// The msg parameter contains the formatted log message. +type LogCaptureFunc func(level, msg string) + // Logger defines the logging operations required by the application. type Logger interface { - Sync() error // Flush any buffered log entries - Sugar() *zap.SugaredLogger // Return a sugared logger for key-value logging -} - -// ZapLogger adapts zap.Logger to implement the Logger interface. -type ZapLogger struct { - *zap.Logger // Embedded zap.Logger for core logging functionality -} - -// NewZapLogger creates a new production-ready Zap logger. -func NewZapLogger() (Logger, error) { - // Define a custom configuration for the logger to ensure cross-platform compatibility. - config := zap.Config{ - Level: zap.NewAtomicLevelAt(zapcore.InfoLevel), - Development: false, - Sampling: &zap.SamplingConfig{ - Initial: samplingInitial, - Thereafter: samplingThereafter, - }, - Encoding: "console", // Use console encoding for human-readable output - EncoderConfig: zapcore.EncoderConfig{ - TimeKey: "time", - LevelKey: "level", - NameKey: "logger", - CallerKey: "caller", - MessageKey: "msg", - StacktraceKey: "stacktrace", - LineEnding: zapcore.DefaultLineEnding, - EncodeLevel: zapcore.CapitalLevelEncoder, // Use uppercase level names (e.g., INFO) - EncodeTime: zapcore.ISO8601TimeEncoder, // Use ISO 8601 format for timestamps - EncodeDuration: zapcore.StringDurationEncoder, - EncodeCaller: zapcore.ShortCallerEncoder, - }, - OutputPaths: []string{"stderr"}, // Explicitly write to stderr - ErrorOutputPaths: []string{"stderr"}, - } + // Debug returns a debug-level event for logging. + Debug() *zerolog.Event + // Info returns an info-level event for logging. + Info() *zerolog.Event + // Warn returns a warn-level event for logging. + Warn() *zerolog.Event + // Error returns an error-level event for logging. + Error() *zerolog.Event + // Sync flushes any buffered log entries (no-op for zerolog, kept for compatibility). + Sync() error + // Level sets the minimum log level dynamically. + Level(level zerolog.Level) + // SetCaptureFunc sets a callback function to capture log messages for TUI display. + // When set, log messages are sent to this callback in addition to normal output. + SetCaptureFunc(captureFunc LogCaptureFunc) +} + +// ZerologLogger wraps zerolog.Logger to implement the Logger interface. +type ZerologLogger struct { + logger zerolog.Logger + mu sync.RWMutex + output io.Writer + captureFunc LogCaptureFunc + captureWriter *captureWriter +} + +// captureWriter wraps an io.Writer and captures written data for TUI display. +type captureWriter struct { + mu sync.RWMutex + output io.Writer + captureFunc LogCaptureFunc + captureEnabled bool +} - // Build the logger with a locked writer to ensure thread-safe writes to stderr. - logger, err := config.Build(zap.WrapCore(func(_ zapcore.Core) zapcore.Core { - writer := zapcore.Lock(os.Stderr) +// Write implements io.Writer, writing to the underlying output and capturing the data. +// When capture is enabled, output is discarded to prevent duplicate logs in TUI mode. +func (w *captureWriter) Write(data []byte) (int, error) { + w.mu.RLock() + output := w.output + enabled := w.captureEnabled + w.mu.RUnlock() - return zapcore.NewCore( - zapcore.NewConsoleEncoder(config.EncoderConfig), - writer, - config.Level, - ) - })) + // If capture is enabled, discard the output to prevent duplicate logs. + // The log message will only be sent through the capture mechanism to the TUI log panel. + if enabled { + w.captureLogMessage(string(data)) + + // Write to io.Discard to satisfy the writer interface without producing output. + // io.Discard.Write always returns (len(p), nil), so we can safely ignore the error. + _, _ = io.Discard.Write(data) + + return len(data), nil + } + + // Capture is not enabled, write to the underlying output normally. + bytesWritten, err := output.Write(data) if err != nil { - return nil, fmt.Errorf("failed to initialize logger: %w", err) + return bytesWritten, fmt.Errorf("failed to write to output: %w", err) } - return &ZapLogger{logger}, nil + return bytesWritten, nil } -// Sync flushes any buffered log entries to the underlying output. -func (z *ZapLogger) Sync() error { - // Check for nil logger to avoid panics on uninitialized instances. - if z.Logger == nil { - return ErrLoggerNil +// captureLogMessage parses and captures the log message from zerolog ConsoleWriter output. +// +// Expected format from ConsoleWriter: " " +// where LEVEL is one of: DBG, INF, WRN, ERR, FTL (zerolog's default level abbreviations). +// +// This function scans fields to find a known level token rather than assuming a fixed +// position, making it tolerant to format changes (e.g., additional prefix fields). +// If no known level token is found, it defaults to level "LOG" with the full message. +func (w *captureWriter) captureLogMessage(logLine string) { + w.mu.RLock() + capture := w.captureFunc + w.mu.RUnlock() + + if capture == nil { + return + } + + // Known zerolog level abbreviations used by ConsoleWriter. + // These are the default short level names output by zerolog. + knownLevels := map[string]bool{ + "DBG": true, + "INF": true, + "WRN": true, + "ERR": true, + "FTL": true, + } + + // Parse the log line to extract level and message. + // Expected format: "2006-01-02T15:04:05Z07:00 DBG message here" + parts := strings.Fields(logLine) + + const minLogParts = 3 // timestamp, level, message + if len(parts) < minLogParts { + // Not enough parts, use the whole line as a generic log entry. + capture("LOG", strings.TrimSpace(logLine)) + + return } - // Attempt to sync, logging any errors in debug mode but not returning them. - if err := z.Logger.Sync(); err != nil { - z.Debug("Logger sync failed", zap.Error(err)) + // Scan for a known level token instead of assuming fixed position. + // This makes parsing more robust against format changes. + level := "LOG" // default level if no known token found + levelIndex := -1 + + for i, part := range parts { + if knownLevels[part] { + level = part + levelIndex = i + + break + } } + var msg string + + if levelIndex >= 0 { + // Level found: message is everything after the level field. + // Build message from the parsed parts slice to avoid matching earlier tokens. + msg = strings.TrimSpace(strings.Join(parts[levelIndex+1:], " ")) + } else { + // No known level found: use the whole line as the message. + msg = strings.TrimSpace(logLine) + } + + capture(level, msg) +} + +// SetCaptureFunc sets the capture function for the underlying capture writer. +// When a non-nil captureFunc is provided, capture is enabled and log messages +// will be sent to the callback instead of the normal output. +func (w *captureWriter) SetCaptureFunc(captureFunc LogCaptureFunc) { + w.mu.Lock() + defer w.mu.Unlock() + + w.captureFunc = captureFunc + w.captureEnabled = captureFunc != nil +} + +// SetupCaptureBridge configures the capture writer with a bridge function that +// calls the provided callback. This is used internally to connect the capture +// writer to the logger's capture function. +func (w *captureWriter) SetupCaptureBridge(bridge func(level, msg string)) { + w.mu.Lock() + defer w.mu.Unlock() + + w.captureFunc = bridge +} + +// NewLogger creates a new zerolog-based logger with console output. +// +// The logger is configured with: +// - ConsoleWriter output to os.Stderr +// - RFC3339 timestamp format +// - Info level as default +func NewLogger() (Logger, error) { + output := zerolog.ConsoleWriter{ + Out: os.Stderr, + TimeFormat: time.RFC3339, + NoColor: false, + } + + // Create the base logger with info level. + zerologLogger := zerolog.New(output). + With(). + Timestamp(). + Logger(). + Level(zerolog.InfoLevel) + + return &ZerologLogger{ + logger: zerologLogger, + output: output, + }, nil +} + +// NewLoggerWithCapture creates a new zerolog-based logger that supports log capture. +// +// The returned logger uses a captureWriter that can send log messages to a callback +// for display in the TUI. This is useful for verbose mode where debug logs should +// appear within the TUI interface rather than being written directly to stderr. +func NewLoggerWithCapture() (Logger, *captureWriter, error) { + // Create a captureWriter that wraps stderr. + // captureFunc and captureEnabled are left as zero values (nil and false). + captureWriter := &captureWriter{ + output: os.Stderr, + } + + // Create a ConsoleWriter that writes to the captureWriter. + consoleWriter := zerolog.ConsoleWriter{ + Out: captureWriter, + TimeFormat: time.RFC3339, + NoColor: false, + } + + // Create the base logger with info level. + zerologLogger := zerolog.New(consoleWriter). + With(). + Timestamp(). + Logger(). + Level(zerolog.InfoLevel) + + logger := &ZerologLogger{ + logger: zerologLogger, + output: consoleWriter, + captureFunc: nil, + captureWriter: captureWriter, + } + + return logger, captureWriter, nil +} + +// Debug returns a debug-level event for logging. +func (z *ZerologLogger) Debug() *zerolog.Event { + z.mu.RLock() + defer z.mu.RUnlock() + + //nolint:zerologlint // Factory method returns event for chaining by design + return z.logger.Debug() +} + +// Info returns an info-level event for logging. +func (z *ZerologLogger) Info() *zerolog.Event { + z.mu.RLock() + defer z.mu.RUnlock() + + //nolint:zerologlint // Factory method returns event for chaining by design + return z.logger.Info() +} + +// Warn returns a warn-level event for logging. +func (z *ZerologLogger) Warn() *zerolog.Event { + z.mu.RLock() + defer z.mu.RUnlock() + + //nolint:zerologlint // Factory method returns event for chaining by design + return z.logger.Warn() +} + +// Error returns an error-level event for logging. +func (z *ZerologLogger) Error() *zerolog.Event { + z.mu.RLock() + defer z.mu.RUnlock() + + //nolint:zerologlint // Factory method returns event for chaining by design + return z.logger.Error() +} + +// Sync is a no-op for zerolog, kept for interface compatibility. +// zerolog writes directly to the output without buffering. +func (z *ZerologLogger) Sync() error { + z.mu.RLock() + defer z.mu.RUnlock() + return nil } -// Sugar returns a sugared logger for convenient key-value logging. -func (z *ZapLogger) Sugar() *zap.SugaredLogger { - // Return nil if the logger is uninitialized to prevent nil pointer dereference. - if z.Logger == nil { - return nil +// Level sets the minimum log level dynamically. +func (z *ZerologLogger) Level(level zerolog.Level) { + z.mu.Lock() + defer z.mu.Unlock() + + // Create a new logger with the specified level. + z.logger = z.logger.Level(level) +} + +// SetCaptureFunc sets a callback function to capture log messages for TUI display. +// When set, log messages are parsed and sent to this callback. +func (z *ZerologLogger) SetCaptureFunc(captureFunc LogCaptureFunc) { + z.mu.Lock() + z.captureFunc = captureFunc + z.mu.Unlock() + + // Update the capture writer's state atomically when it exists. + // SetCaptureFunc is called FIRST to update the writer's capture handler and + // enable flag. Then SetupCaptureBridge installs a bridge that delegates to + // the writer's capture handler. This ordering prevents a race condition where + // logs could be dropped between setting up the bridge and updating the handler. + if z.captureWriter != nil { + z.captureWriter.SetCaptureFunc(captureFunc) + z.captureWriter.SetupCaptureBridge(func(level, msg string) { + z.mu.RLock() + capture := z.captureFunc + z.mu.RUnlock() + + if capture != nil { + capture(level, msg) + } + }) } +} - // Delegate to zap.Logger’s Sugar method for sugared logging. - return z.Logger.Sugar() +// ParseLevel parses a string log level into a zerolog.Level. +// +// Supported levels (case-insensitive): +// - "debug" -> DebugLevel +// - "info" -> InfoLevel +// - "warn" -> WarnLevel +// - "error" -> ErrorLevel +// +// Defaults to InfoLevel if the level is unrecognized. +func ParseLevel(level string) zerolog.Level { + switch strings.ToLower(level) { + case "debug": + return zerolog.DebugLevel + case "info": + return zerolog.InfoLevel + case "warn": + return zerolog.WarnLevel + case "error": + return zerolog.ErrorLevel + default: + return zerolog.InfoLevel + } } diff --git a/internal/logger/logger_test.go b/internal/logger/logger_test.go index 67b68c6..495e473 100644 --- a/internal/logger/logger_test.go +++ b/internal/logger/logger_test.go @@ -18,14 +18,41 @@ along with this program. If not, see . package logger import ( - "reflect" + "bytes" + "strings" + "sync" "testing" - "go.uber.org/zap" + "github.com/rs/zerolog" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) -// TestNewZapLogger verifies the NewZapLogger function’s logger creation. -func TestNewZapLogger(t *testing.T) { +// syncBuffer is a thread-safe wrapper around bytes.Buffer for concurrent writes. +type syncBuffer struct { + buf bytes.Buffer + mu sync.Mutex +} + +// Write writes p to the buffer in a thread-safe manner. +// It implements the io.Writer interface. +func (sb *syncBuffer) Write(p []byte) (int, error) { + sb.mu.Lock() + defer sb.mu.Unlock() + + return sb.buf.Write(p) +} + +// String returns the buffer's contents in a thread-safe manner. +func (sb *syncBuffer) String() string { + sb.mu.Lock() + defer sb.mu.Unlock() + + return sb.buf.String() +} + +// TestNewLogger verifies the NewLogger function creates a valid logger. +func TestNewLogger(t *testing.T) { tests := []struct { name string wantErr bool @@ -37,91 +64,348 @@ func TestNewZapLogger(t *testing.T) { } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - got, err := NewZapLogger() - if (err != nil) != tt.wantErr { - t.Errorf("NewZapLogger() error = %v, wantErr %v", err, tt.wantErr) + got, err := NewLogger() + if tt.wantErr { + require.Error(t, err) return } - if got == nil { - t.Errorf("NewZapLogger() returned nil logger") + require.NoError(t, err) + assert.NotNil(t, got) - return + // Verify the returned logger is a *ZerologLogger. + _, ok := got.(*ZerologLogger) + assert.True(t, ok, "expected *ZerologLogger, got %T", got) + }) + } +} + +// TestZerologLogger_LogLevelMethods verifies all log level methods work correctly. +func TestZerologLogger_LogLevelMethods(t *testing.T) { + tests := []struct { + name string + level zerolog.Level + logFunc func(l Logger) *zerolog.Event + wantLog bool + contains string + }{ + { + name: "debug level with debug enabled", + level: zerolog.DebugLevel, + logFunc: func(l Logger) *zerolog.Event { return l.Debug() }, + wantLog: true, + contains: "DBG", + }, + { + name: "info level", + level: zerolog.InfoLevel, + logFunc: func(l Logger) *zerolog.Event { return l.Info() }, + wantLog: true, + contains: "INF", + }, + { + name: "warn level", + level: zerolog.WarnLevel, + logFunc: func(l Logger) *zerolog.Event { return l.Warn() }, + wantLog: true, + contains: "WRN", + }, + { + name: "error level", + level: zerolog.ErrorLevel, + logFunc: func(l Logger) *zerolog.Event { return l.Error() }, + wantLog: true, + contains: "ERR", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + // Create a buffer to capture log output. + var buf bytes.Buffer + + output := zerolog.ConsoleWriter{ + Out: &buf, + TimeFormat: "2006-01-02", + NoColor: true, } - // Verify the returned logger is a valid *ZapLogger. - zapLogger, ok := got.(*ZapLogger) - if !ok || zapLogger.Logger == nil { - t.Errorf("NewZapLogger() = %v, expected non-nil *ZapLogger", got) + // Create logger with specific level. + zl := zerolog.New(output). + With(). + Timestamp(). + Logger(). + Level(tt.level) + + logger := &ZerologLogger{logger: zl, output: output} + + // Call the log function with a test message. + tt.logFunc(logger).Msg("test message") + + outputStr := buf.String() + + if tt.wantLog { + assert.Contains(t, outputStr, tt.contains) + assert.Contains(t, outputStr, "test message") } }) } } -// TestZapLogger_Sync verifies the Sync method’s behavior. -func TestZapLogger_Sync(t *testing.T) { +// TestZerologLogger_Sync verifies the Sync method's behavior. +func TestZerologLogger_Sync(t *testing.T) { tests := []struct { name string - z *ZapLogger + setup func() *ZerologLogger wantErr bool }{ { name: "sync with valid logger", - z: &ZapLogger{ - Logger: zap.NewNop(), // Use no-op logger to avoid real I/O + setup: func() *ZerologLogger { + zl := zerolog.New(nil).With().Logger() + + return &ZerologLogger{logger: zl} }, wantErr: false, }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + z := tt.setup() + err := z.Sync() + assert.NoError(t, err) + }) + } +} + +// TestZerologLogger_Level verifies the Level method changes log level dynamically. +func TestZerologLogger_Level(t *testing.T) { + tests := []struct { + name string + initialLevel zerolog.Level + newLevel zerolog.Level + logLevel zerolog.Level + shouldLog bool + }{ + { + name: "change from info to debug", + initialLevel: zerolog.InfoLevel, + newLevel: zerolog.DebugLevel, + logLevel: zerolog.DebugLevel, + shouldLog: true, + }, + { + name: "change from debug to error", + initialLevel: zerolog.DebugLevel, + newLevel: zerolog.ErrorLevel, + logLevel: zerolog.InfoLevel, + shouldLog: false, + }, { - name: "sync with nil logger", - z: &ZapLogger{nil}, - wantErr: true, + name: "no change in level", + initialLevel: zerolog.InfoLevel, + newLevel: zerolog.InfoLevel, + logLevel: zerolog.InfoLevel, + shouldLog: true, }, } + for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - err := tt.z.Sync() - if (err != nil) != tt.wantErr { - t.Errorf("ZapLogger.Sync() error = %v, wantErr %v", err, tt.wantErr) + var buf bytes.Buffer + + output := zerolog.ConsoleWriter{ + Out: &buf, + TimeFormat: "2006-01-02", + NoColor: true, + } + + // Create logger with initial level. + zl := zerolog.New(output). + With(). + Timestamp(). + Logger(). + Level(tt.initialLevel) + + logger := &ZerologLogger{logger: zl, output: output} + + // Change the level. + logger.Level(tt.newLevel) + + // Try to log at the specified level. + switch tt.logLevel { + case zerolog.DebugLevel: + logger.Debug().Msg("test debug") + case zerolog.InfoLevel: + logger.Info().Msg("test info") + case zerolog.ErrorLevel: + logger.Error().Msg("test error") + } + + outputStr := buf.String() + + if tt.shouldLog { + assert.NotEmpty(t, outputStr) + } else { + assert.Empty(t, outputStr) } }) } } -// TestZapLogger_Sugar verifies the Sugar method’s sugared logger output. -func TestZapLogger_Sugar(t *testing.T) { +// TestParseLevel verifies the ParseLevel function's string parsing. +func TestParseLevel(t *testing.T) { tests := []struct { - name string - z *ZapLogger + name string + level string + want zerolog.Level }{ { - name: "sugar with valid logger", - z: func() *ZapLogger { - logger, _ := zap.NewProduction() - - return &ZapLogger{logger} - }(), + name: "debug lowercase", + level: "debug", + want: zerolog.DebugLevel, + }, + { + name: "info lowercase", + level: "info", + want: zerolog.InfoLevel, + }, + { + name: "warn lowercase", + level: "warn", + want: zerolog.WarnLevel, + }, + { + name: "error lowercase", + level: "error", + want: zerolog.ErrorLevel, + }, + { + name: "debug uppercase", + level: "DEBUG", + want: zerolog.DebugLevel, }, { - name: "sugar with nil logger", - z: &ZapLogger{nil}, + name: "info mixed case", + level: "Info", + want: zerolog.InfoLevel, + }, + { + name: "unknown level defaults to info", + level: "unknown", + want: zerolog.InfoLevel, + }, + { + name: "empty string defaults to info", + level: "", + want: zerolog.InfoLevel, }, } + for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - got := tt.z.Sugar() - if tt.z.Logger == nil { - if got != nil { - t.Errorf("ZapLogger.Sugar() = %v, want nil for nil logger", got) - } - } else { - if got == nil { - t.Errorf("ZapLogger.Sugar() = nil, want non-nil *zap.SugaredLogger") - } else if reflect.TypeOf(got) != reflect.TypeOf(zap.NewNop().Sugar()) { - t.Errorf("ZapLogger.Sugar() = %v, want *zap.SugaredLogger", got) - } - } + got := ParseLevel(tt.level) + assert.Equal(t, tt.want, got) }) } } + +// BenchmarkZerologLogger_Debug benchmarks the Debug method. +func BenchmarkZerologLogger_Debug(b *testing.B) { + zl := zerolog.New(nil).With().Logger() + logger := &ZerologLogger{logger: zl} + + b.ResetTimer() + + for b.Loop() { + logger.Debug().Msg("benchmark message") + } +} + +// BenchmarkZerologLogger_Info benchmarks the Info method. +func BenchmarkZerologLogger_Info(b *testing.B) { + zl := zerolog.New(nil).With().Logger() + logger := &ZerologLogger{logger: zl} + + b.ResetTimer() + + for b.Loop() { + logger.Info().Msg("benchmark message") + } +} + +// BenchmarkParseLevel benchmarks the ParseLevel function. +func BenchmarkParseLevel(b *testing.B) { + levels := []string{"debug", "info", "warn", "error", "unknown"} + + b.ResetTimer() + + for b.Loop() { + for _, level := range levels { + ParseLevel(level) + } + } +} + +// TestZerologLogger_ConcurrentAccess verifies thread safety of logger methods. +func TestZerologLogger_ConcurrentAccess(t *testing.T) { + buf := &syncBuffer{} + + output := zerolog.ConsoleWriter{ + Out: buf, + TimeFormat: "2006-01-02", + NoColor: true, + } + + zl := zerolog.New(output).With().Timestamp().Logger().Level(zerolog.DebugLevel) + logger := &ZerologLogger{logger: zl, output: output} + + // Run concurrent logging operations. + done := make(chan bool, 4) + + go func() { + for range 100 { + logger.Debug().Msg("debug message") + } + + done <- true + }() + + go func() { + for range 100 { + logger.Info().Msg("info message") + } + + done <- true + }() + + go func() { + for range 100 { + logger.Warn().Msg("warn message") + } + + done <- true + }() + + go func() { + for range 100 { + logger.Error().Msg("error message") + } + + done <- true + }() + + // Wait for all goroutines to complete. + for range 4 { + <-done + } + + // Verify output contains messages from all levels. + outputStr := buf.String() + assert.Positive(t, strings.Count(outputStr, "debug message")) + assert.Positive(t, strings.Count(outputStr, "info message")) + assert.Positive(t, strings.Count(outputStr, "warn message")) + assert.Positive(t, strings.Count(outputStr, "error message")) +} diff --git a/internal/logger/mocks/Logger.go b/internal/logger/mocks/Logger.go index 015ac67..02a0955 100644 --- a/internal/logger/mocks/Logger.go +++ b/internal/logger/mocks/Logger.go @@ -5,8 +5,9 @@ package mocks import ( + "github.com/nicholas-fedor/go-remove/internal/logger" + "github.com/rs/zerolog" mock "github.com/stretchr/testify/mock" - "go.uber.org/zap" ) // NewMockLogger creates a new instance of MockLogger. It also registers a testing interface on the mock and a cleanup function to assert the mocks expectations. @@ -36,52 +37,224 @@ func (_m *MockLogger) EXPECT() *MockLogger_Expecter { return &MockLogger_Expecter{mock: &_m.Mock} } -// Sugar provides a mock function for the type MockLogger -func (_mock *MockLogger) Sugar() *zap.SugaredLogger { +// Debug provides a mock function for the type MockLogger +func (_mock *MockLogger) Debug() *zerolog.Event { ret := _mock.Called() if len(ret) == 0 { - panic("no return value specified for Sugar") + panic("no return value specified for Debug") } - var r0 *zap.SugaredLogger - if returnFunc, ok := ret.Get(0).(func() *zap.SugaredLogger); ok { + var r0 *zerolog.Event + if returnFunc, ok := ret.Get(0).(func() *zerolog.Event); ok { r0 = returnFunc() } else { if ret.Get(0) != nil { - r0 = ret.Get(0).(*zap.SugaredLogger) + r0 = ret.Get(0).(*zerolog.Event) } } return r0 } -// MockLogger_Sugar_Call is a *mock.Call that shadows Run/Return methods with type explicit version for method 'Sugar' -type MockLogger_Sugar_Call struct { +// MockLogger_Debug_Call is a *mock.Call that shadows Run/Return methods with type explicit version for method 'Debug' +type MockLogger_Debug_Call struct { *mock.Call } -// Sugar is a helper method to define mock.On call -func (_e *MockLogger_Expecter) Sugar() *MockLogger_Sugar_Call { - return &MockLogger_Sugar_Call{Call: _e.mock.On("Sugar")} +// Debug is a helper method to define mock.On call +func (_e *MockLogger_Expecter) Debug() *MockLogger_Debug_Call { + return &MockLogger_Debug_Call{Call: _e.mock.On("Debug")} } -func (_c *MockLogger_Sugar_Call) Run(run func()) *MockLogger_Sugar_Call { +func (_c *MockLogger_Debug_Call) Run(run func()) *MockLogger_Debug_Call { _c.Call.Run(func(args mock.Arguments) { run() }) return _c } -func (_c *MockLogger_Sugar_Call) Return(sugaredLogger *zap.SugaredLogger) *MockLogger_Sugar_Call { - _c.Call.Return(sugaredLogger) +func (_c *MockLogger_Debug_Call) Return(event *zerolog.Event) *MockLogger_Debug_Call { + _c.Call.Return(event) return _c } -func (_c *MockLogger_Sugar_Call) RunAndReturn(run func() *zap.SugaredLogger) *MockLogger_Sugar_Call { +func (_c *MockLogger_Debug_Call) RunAndReturn(run func() *zerolog.Event) *MockLogger_Debug_Call { _c.Call.Return(run) return _c } +// Error provides a mock function for the type MockLogger +func (_mock *MockLogger) Error() *zerolog.Event { + ret := _mock.Called() + + if len(ret) == 0 { + panic("no return value specified for Error") + } + + var r0 *zerolog.Event + if returnFunc, ok := ret.Get(0).(func() *zerolog.Event); ok { + r0 = returnFunc() + } else { + if ret.Get(0) != nil { + r0 = ret.Get(0).(*zerolog.Event) + } + } + return r0 +} + +// MockLogger_Error_Call is a *mock.Call that shadows Run/Return methods with type explicit version for method 'Error' +type MockLogger_Error_Call struct { + *mock.Call +} + +// Error is a helper method to define mock.On call +func (_e *MockLogger_Expecter) Error() *MockLogger_Error_Call { + return &MockLogger_Error_Call{Call: _e.mock.On("Error")} +} + +func (_c *MockLogger_Error_Call) Run(run func()) *MockLogger_Error_Call { + _c.Call.Run(func(args mock.Arguments) { + run() + }) + return _c +} + +func (_c *MockLogger_Error_Call) Return(event *zerolog.Event) *MockLogger_Error_Call { + _c.Call.Return(event) + return _c +} + +func (_c *MockLogger_Error_Call) RunAndReturn(run func() *zerolog.Event) *MockLogger_Error_Call { + _c.Call.Return(run) + return _c +} + +// Info provides a mock function for the type MockLogger +func (_mock *MockLogger) Info() *zerolog.Event { + ret := _mock.Called() + + if len(ret) == 0 { + panic("no return value specified for Info") + } + + var r0 *zerolog.Event + if returnFunc, ok := ret.Get(0).(func() *zerolog.Event); ok { + r0 = returnFunc() + } else { + if ret.Get(0) != nil { + r0 = ret.Get(0).(*zerolog.Event) + } + } + return r0 +} + +// MockLogger_Info_Call is a *mock.Call that shadows Run/Return methods with type explicit version for method 'Info' +type MockLogger_Info_Call struct { + *mock.Call +} + +// Info is a helper method to define mock.On call +func (_e *MockLogger_Expecter) Info() *MockLogger_Info_Call { + return &MockLogger_Info_Call{Call: _e.mock.On("Info")} +} + +func (_c *MockLogger_Info_Call) Run(run func()) *MockLogger_Info_Call { + _c.Call.Run(func(args mock.Arguments) { + run() + }) + return _c +} + +func (_c *MockLogger_Info_Call) Return(event *zerolog.Event) *MockLogger_Info_Call { + _c.Call.Return(event) + return _c +} + +func (_c *MockLogger_Info_Call) RunAndReturn(run func() *zerolog.Event) *MockLogger_Info_Call { + _c.Call.Return(run) + return _c +} + +// Level provides a mock function for the type MockLogger +func (_mock *MockLogger) Level(level zerolog.Level) { + _mock.Called(level) + return +} + +// MockLogger_Level_Call is a *mock.Call that shadows Run/Return methods with type explicit version for method 'Level' +type MockLogger_Level_Call struct { + *mock.Call +} + +// Level is a helper method to define mock.On call +// - level zerolog.Level +func (_e *MockLogger_Expecter) Level(level interface{}) *MockLogger_Level_Call { + return &MockLogger_Level_Call{Call: _e.mock.On("Level", level)} +} + +func (_c *MockLogger_Level_Call) Run(run func(level zerolog.Level)) *MockLogger_Level_Call { + _c.Call.Run(func(args mock.Arguments) { + var arg0 zerolog.Level + if args[0] != nil { + arg0 = args[0].(zerolog.Level) + } + run( + arg0, + ) + }) + return _c +} + +func (_c *MockLogger_Level_Call) Return() *MockLogger_Level_Call { + _c.Call.Return() + return _c +} + +func (_c *MockLogger_Level_Call) RunAndReturn(run func(level zerolog.Level)) *MockLogger_Level_Call { + _c.Run(run) + return _c +} + +// SetCaptureFunc provides a mock function for the type MockLogger +func (_mock *MockLogger) SetCaptureFunc(captureFunc logger.LogCaptureFunc) { + _mock.Called(captureFunc) + return +} + +// MockLogger_SetCaptureFunc_Call is a *mock.Call that shadows Run/Return methods with type explicit version for method 'SetCaptureFunc' +type MockLogger_SetCaptureFunc_Call struct { + *mock.Call +} + +// SetCaptureFunc is a helper method to define mock.On call +// - captureFunc logger.LogCaptureFunc +func (_e *MockLogger_Expecter) SetCaptureFunc(captureFunc interface{}) *MockLogger_SetCaptureFunc_Call { + return &MockLogger_SetCaptureFunc_Call{Call: _e.mock.On("SetCaptureFunc", captureFunc)} +} + +func (_c *MockLogger_SetCaptureFunc_Call) Run(run func(captureFunc logger.LogCaptureFunc)) *MockLogger_SetCaptureFunc_Call { + _c.Call.Run(func(args mock.Arguments) { + var arg0 logger.LogCaptureFunc + if args[0] != nil { + arg0 = args[0].(logger.LogCaptureFunc) + } + run( + arg0, + ) + }) + return _c +} + +func (_c *MockLogger_SetCaptureFunc_Call) Return() *MockLogger_SetCaptureFunc_Call { + _c.Call.Return() + return _c +} + +func (_c *MockLogger_SetCaptureFunc_Call) RunAndReturn(run func(captureFunc logger.LogCaptureFunc)) *MockLogger_SetCaptureFunc_Call { + _c.Run(run) + return _c +} + // Sync provides a mock function for the type MockLogger func (_mock *MockLogger) Sync() error { ret := _mock.Called() @@ -125,3 +298,49 @@ func (_c *MockLogger_Sync_Call) RunAndReturn(run func() error) *MockLogger_Sync_ _c.Call.Return(run) return _c } + +// Warn provides a mock function for the type MockLogger +func (_mock *MockLogger) Warn() *zerolog.Event { + ret := _mock.Called() + + if len(ret) == 0 { + panic("no return value specified for Warn") + } + + var r0 *zerolog.Event + if returnFunc, ok := ret.Get(0).(func() *zerolog.Event); ok { + r0 = returnFunc() + } else { + if ret.Get(0) != nil { + r0 = ret.Get(0).(*zerolog.Event) + } + } + return r0 +} + +// MockLogger_Warn_Call is a *mock.Call that shadows Run/Return methods with type explicit version for method 'Warn' +type MockLogger_Warn_Call struct { + *mock.Call +} + +// Warn is a helper method to define mock.On call +func (_e *MockLogger_Expecter) Warn() *MockLogger_Warn_Call { + return &MockLogger_Warn_Call{Call: _e.mock.On("Warn")} +} + +func (_c *MockLogger_Warn_Call) Run(run func()) *MockLogger_Warn_Call { + _c.Call.Run(func(args mock.Arguments) { + run() + }) + return _c +} + +func (_c *MockLogger_Warn_Call) Return(event *zerolog.Event) *MockLogger_Warn_Call { + _c.Call.Return(event) + return _c +} + +func (_c *MockLogger_Warn_Call) RunAndReturn(run func() *zerolog.Event) *MockLogger_Warn_Call { + _c.Call.Return(run) + return _c +}