feat(process): add Logger interface for exec wrapper (#93)
- Define Logger interface with Debug and Error methods - Add NopLogger as default (no-op implementation) - Add SetDefaultLogger/DefaultLogger for package-level config - Add WithLogger method for per-command logger injection - Log commands at DEBUG level before execution - Log failures at ERROR level with error details - Add comprehensive tests for logger functionality Compatible with pkg/log.Logger and other structured loggers. Closes #90 Co-authored-by: Claude <noreply@anthropic.com>
This commit is contained in:
parent
e8f479f65c
commit
efd952dab6
3 changed files with 220 additions and 12 deletions
|
|
@ -37,6 +37,7 @@ type Cmd struct {
|
||||||
ctx context.Context
|
ctx context.Context
|
||||||
opts Options
|
opts Options
|
||||||
cmd *exec.Cmd
|
cmd *exec.Cmd
|
||||||
|
logger Logger
|
||||||
}
|
}
|
||||||
|
|
||||||
// WithDir sets the working directory
|
// WithDir sets the working directory
|
||||||
|
|
@ -69,17 +70,23 @@ func (c *Cmd) WithStderr(w io.Writer) *Cmd {
|
||||||
return c
|
return c
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// WithLogger sets a custom logger for this command.
|
||||||
|
// If not set, the package default logger is used.
|
||||||
|
func (c *Cmd) WithLogger(l Logger) *Cmd {
|
||||||
|
c.logger = l
|
||||||
|
return c
|
||||||
|
}
|
||||||
|
|
||||||
// Run executes the command and waits for it to finish.
|
// Run executes the command and waits for it to finish.
|
||||||
// It automatically logs the command execution at debug level.
|
// It automatically logs the command execution at debug level.
|
||||||
func (c *Cmd) Run() error {
|
func (c *Cmd) Run() error {
|
||||||
c.prepare()
|
c.prepare()
|
||||||
|
c.logDebug("executing command")
|
||||||
// TODO: Use a proper logger interface when available in pkg/process
|
|
||||||
// For now using cli.Debug which might not be visible unless verbose
|
|
||||||
// cli.Debug("Executing: %s %s", c.name, strings.Join(c.args, " "))
|
|
||||||
|
|
||||||
if err := c.cmd.Run(); err != nil {
|
if err := c.cmd.Run(); err != nil {
|
||||||
return wrapError(err, c.name, c.args)
|
wrapped := wrapError(err, c.name, c.args)
|
||||||
|
c.logError("command failed", wrapped)
|
||||||
|
return wrapped
|
||||||
}
|
}
|
||||||
return nil
|
return nil
|
||||||
}
|
}
|
||||||
|
|
@ -87,9 +94,13 @@ func (c *Cmd) Run() error {
|
||||||
// Output runs the command and returns its standard output.
|
// Output runs the command and returns its standard output.
|
||||||
func (c *Cmd) Output() ([]byte, error) {
|
func (c *Cmd) Output() ([]byte, error) {
|
||||||
c.prepare()
|
c.prepare()
|
||||||
|
c.logDebug("executing command")
|
||||||
|
|
||||||
out, err := c.cmd.Output()
|
out, err := c.cmd.Output()
|
||||||
if err != nil {
|
if err != nil {
|
||||||
return nil, wrapError(err, c.name, c.args)
|
wrapped := wrapError(err, c.name, c.args)
|
||||||
|
c.logError("command failed", wrapped)
|
||||||
|
return nil, wrapped
|
||||||
}
|
}
|
||||||
return out, nil
|
return out, nil
|
||||||
}
|
}
|
||||||
|
|
@ -97,9 +108,13 @@ func (c *Cmd) Output() ([]byte, error) {
|
||||||
// CombinedOutput runs the command and returns its combined standard output and standard error.
|
// CombinedOutput runs the command and returns its combined standard output and standard error.
|
||||||
func (c *Cmd) CombinedOutput() ([]byte, error) {
|
func (c *Cmd) CombinedOutput() ([]byte, error) {
|
||||||
c.prepare()
|
c.prepare()
|
||||||
|
c.logDebug("executing command")
|
||||||
|
|
||||||
out, err := c.cmd.CombinedOutput()
|
out, err := c.cmd.CombinedOutput()
|
||||||
if err != nil {
|
if err != nil {
|
||||||
return out, wrapError(err, c.name, c.args)
|
wrapped := wrapError(err, c.name, c.args)
|
||||||
|
c.logError("command failed", wrapped)
|
||||||
|
return out, wrapped
|
||||||
}
|
}
|
||||||
return out, nil
|
return out, nil
|
||||||
}
|
}
|
||||||
|
|
@ -144,3 +159,18 @@ func wrapError(err error, name string, args []string) error {
|
||||||
}
|
}
|
||||||
return fmt.Errorf("failed to execute %q: %w", cmdStr, err)
|
return fmt.Errorf("failed to execute %q: %w", cmdStr, err)
|
||||||
}
|
}
|
||||||
|
|
||||||
|
func (c *Cmd) getLogger() Logger {
|
||||||
|
if c.logger != nil {
|
||||||
|
return c.logger
|
||||||
|
}
|
||||||
|
return defaultLogger
|
||||||
|
}
|
||||||
|
|
||||||
|
func (c *Cmd) logDebug(msg string) {
|
||||||
|
c.getLogger().Debug(msg, "cmd", c.name, "args", strings.Join(c.args, " "))
|
||||||
|
}
|
||||||
|
|
||||||
|
func (c *Cmd) logError(msg string, err error) {
|
||||||
|
c.getLogger().Error(msg, "cmd", c.name, "args", strings.Join(c.args, " "), "err", err)
|
||||||
|
}
|
||||||
|
|
|
||||||
148
pkg/process/exec/exec_test.go
Normal file
148
pkg/process/exec/exec_test.go
Normal file
|
|
@ -0,0 +1,148 @@
|
||||||
|
package exec_test
|
||||||
|
|
||||||
|
import (
|
||||||
|
"context"
|
||||||
|
"strings"
|
||||||
|
"testing"
|
||||||
|
|
||||||
|
"github.com/host-uk/core/pkg/process/exec"
|
||||||
|
)
|
||||||
|
|
||||||
|
// mockLogger captures log calls for testing
|
||||||
|
type mockLogger struct {
|
||||||
|
debugCalls []logCall
|
||||||
|
errorCalls []logCall
|
||||||
|
}
|
||||||
|
|
||||||
|
type logCall struct {
|
||||||
|
msg string
|
||||||
|
keyvals []any
|
||||||
|
}
|
||||||
|
|
||||||
|
func (m *mockLogger) Debug(msg string, keyvals ...any) {
|
||||||
|
m.debugCalls = append(m.debugCalls, logCall{msg, keyvals})
|
||||||
|
}
|
||||||
|
|
||||||
|
func (m *mockLogger) Error(msg string, keyvals ...any) {
|
||||||
|
m.errorCalls = append(m.errorCalls, logCall{msg, keyvals})
|
||||||
|
}
|
||||||
|
|
||||||
|
func TestCommand_Run_Good_LogsDebug(t *testing.T) {
|
||||||
|
logger := &mockLogger{}
|
||||||
|
ctx := context.Background()
|
||||||
|
|
||||||
|
err := exec.Command(ctx, "echo", "hello").
|
||||||
|
WithLogger(logger).
|
||||||
|
Run()
|
||||||
|
if err != nil {
|
||||||
|
t.Fatalf("unexpected error: %v", err)
|
||||||
|
}
|
||||||
|
|
||||||
|
if len(logger.debugCalls) != 1 {
|
||||||
|
t.Fatalf("expected 1 debug call, got %d", len(logger.debugCalls))
|
||||||
|
}
|
||||||
|
if logger.debugCalls[0].msg != "executing command" {
|
||||||
|
t.Errorf("expected msg 'executing command', got %q", logger.debugCalls[0].msg)
|
||||||
|
}
|
||||||
|
if len(logger.errorCalls) != 0 {
|
||||||
|
t.Errorf("expected no error calls, got %d", len(logger.errorCalls))
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
func TestCommand_Run_Bad_LogsError(t *testing.T) {
|
||||||
|
logger := &mockLogger{}
|
||||||
|
ctx := context.Background()
|
||||||
|
|
||||||
|
err := exec.Command(ctx, "false").
|
||||||
|
WithLogger(logger).
|
||||||
|
Run()
|
||||||
|
if err == nil {
|
||||||
|
t.Fatal("expected error")
|
||||||
|
}
|
||||||
|
|
||||||
|
if len(logger.debugCalls) != 1 {
|
||||||
|
t.Fatalf("expected 1 debug call, got %d", len(logger.debugCalls))
|
||||||
|
}
|
||||||
|
if len(logger.errorCalls) != 1 {
|
||||||
|
t.Fatalf("expected 1 error call, got %d", len(logger.errorCalls))
|
||||||
|
}
|
||||||
|
if logger.errorCalls[0].msg != "command failed" {
|
||||||
|
t.Errorf("expected msg 'command failed', got %q", logger.errorCalls[0].msg)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
func TestCommand_Output_Good(t *testing.T) {
|
||||||
|
logger := &mockLogger{}
|
||||||
|
ctx := context.Background()
|
||||||
|
|
||||||
|
out, err := exec.Command(ctx, "echo", "test").
|
||||||
|
WithLogger(logger).
|
||||||
|
Output()
|
||||||
|
if err != nil {
|
||||||
|
t.Fatalf("unexpected error: %v", err)
|
||||||
|
}
|
||||||
|
if strings.TrimSpace(string(out)) != "test" {
|
||||||
|
t.Errorf("expected 'test', got %q", string(out))
|
||||||
|
}
|
||||||
|
if len(logger.debugCalls) != 1 {
|
||||||
|
t.Errorf("expected 1 debug call, got %d", len(logger.debugCalls))
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
func TestCommand_CombinedOutput_Good(t *testing.T) {
|
||||||
|
logger := &mockLogger{}
|
||||||
|
ctx := context.Background()
|
||||||
|
|
||||||
|
out, err := exec.Command(ctx, "echo", "combined").
|
||||||
|
WithLogger(logger).
|
||||||
|
CombinedOutput()
|
||||||
|
if err != nil {
|
||||||
|
t.Fatalf("unexpected error: %v", err)
|
||||||
|
}
|
||||||
|
if strings.TrimSpace(string(out)) != "combined" {
|
||||||
|
t.Errorf("expected 'combined', got %q", string(out))
|
||||||
|
}
|
||||||
|
if len(logger.debugCalls) != 1 {
|
||||||
|
t.Errorf("expected 1 debug call, got %d", len(logger.debugCalls))
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
func TestNopLogger(t *testing.T) {
|
||||||
|
// Verify NopLogger doesn't panic
|
||||||
|
var nop exec.NopLogger
|
||||||
|
nop.Debug("msg", "key", "val")
|
||||||
|
nop.Error("msg", "key", "val")
|
||||||
|
}
|
||||||
|
|
||||||
|
func TestSetDefaultLogger(t *testing.T) {
|
||||||
|
original := exec.DefaultLogger()
|
||||||
|
defer exec.SetDefaultLogger(original)
|
||||||
|
|
||||||
|
logger := &mockLogger{}
|
||||||
|
exec.SetDefaultLogger(logger)
|
||||||
|
|
||||||
|
if exec.DefaultLogger() != logger {
|
||||||
|
t.Error("default logger not set correctly")
|
||||||
|
}
|
||||||
|
|
||||||
|
// Test nil resets to NopLogger
|
||||||
|
exec.SetDefaultLogger(nil)
|
||||||
|
if _, ok := exec.DefaultLogger().(exec.NopLogger); !ok {
|
||||||
|
t.Error("expected NopLogger when setting nil")
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
func TestCommand_UsesDefaultLogger(t *testing.T) {
|
||||||
|
original := exec.DefaultLogger()
|
||||||
|
defer exec.SetDefaultLogger(original)
|
||||||
|
|
||||||
|
logger := &mockLogger{}
|
||||||
|
exec.SetDefaultLogger(logger)
|
||||||
|
|
||||||
|
ctx := context.Background()
|
||||||
|
_ = exec.Command(ctx, "echo", "test").Run()
|
||||||
|
|
||||||
|
if len(logger.debugCalls) != 1 {
|
||||||
|
t.Errorf("expected default logger to receive 1 debug call, got %d", len(logger.debugCalls))
|
||||||
|
}
|
||||||
|
}
|
||||||
30
pkg/process/exec/logger.go
Normal file
30
pkg/process/exec/logger.go
Normal file
|
|
@ -0,0 +1,30 @@
|
||||||
|
package exec
|
||||||
|
|
||||||
|
// Logger interface for command execution logging.
|
||||||
|
// Compatible with pkg/log.Logger and other structured loggers.
|
||||||
|
type Logger interface {
|
||||||
|
Debug(msg string, keyvals ...any)
|
||||||
|
Error(msg string, keyvals ...any)
|
||||||
|
}
|
||||||
|
|
||||||
|
// NopLogger is a no-op logger that discards all messages.
|
||||||
|
type NopLogger struct{}
|
||||||
|
|
||||||
|
func (NopLogger) Debug(string, ...any) {}
|
||||||
|
func (NopLogger) Error(string, ...any) {}
|
||||||
|
|
||||||
|
var defaultLogger Logger = NopLogger{}
|
||||||
|
|
||||||
|
// SetDefaultLogger sets the package-level default logger.
|
||||||
|
// Commands without an explicit logger will use this.
|
||||||
|
func SetDefaultLogger(l Logger) {
|
||||||
|
if l == nil {
|
||||||
|
l = NopLogger{}
|
||||||
|
}
|
||||||
|
defaultLogger = l
|
||||||
|
}
|
||||||
|
|
||||||
|
// DefaultLogger returns the current default logger.
|
||||||
|
func DefaultLogger() Logger {
|
||||||
|
return defaultLogger
|
||||||
|
}
|
||||||
Loading…
Add table
Reference in a new issue