From c145d64891d57ed039f23e1faf5d45c86bb5debe Mon Sep 17 00:00:00 2001 From: Snider Date: Fri, 30 Jan 2026 19:55:37 +0000 Subject: [PATCH] test(process): add concurrency tests and global function wrappers Address code review findings: - Add global wrappers: StartWithOptions, RunWithOptions, Running - Add global_test.go with concurrent access tests for Default(), SetDefault(), and concurrent operations - Add process_test.go with dedicated Process struct method tests - All tests pass with race detector Co-Authored-By: Claude Opus 4.5 --- pkg/process/global_test.go | 298 ++++++++++++++++++++++++++++++++++ pkg/process/process_global.go | 27 +++ pkg/process/process_test.go | 227 ++++++++++++++++++++++++++ pkg/process/service_test.go | 13 -- 4 files changed, 552 insertions(+), 13 deletions(-) create mode 100644 pkg/process/global_test.go create mode 100644 pkg/process/process_test.go diff --git a/pkg/process/global_test.go b/pkg/process/global_test.go new file mode 100644 index 0000000..c1965f7 --- /dev/null +++ b/pkg/process/global_test.go @@ -0,0 +1,298 @@ +package process + +import ( + "context" + "sync" + "testing" + + "github.com/host-uk/core/pkg/framework" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestGlobal_DefaultNotInitialized(t *testing.T) { + // Reset global state for this test + old := defaultService.Swap(nil) + defer func() { + if old != nil { + defaultService.Store(old) + } + }() + + assert.Nil(t, Default()) + + _, err := Start(context.Background(), "echo", "test") + assert.ErrorIs(t, err, ErrServiceNotInitialized) + + _, err = Run(context.Background(), "echo", "test") + assert.ErrorIs(t, err, ErrServiceNotInitialized) + + _, err = Get("proc-1") + assert.ErrorIs(t, err, ErrServiceNotInitialized) + + assert.Nil(t, List()) + assert.Nil(t, Running()) + + err = Kill("proc-1") + assert.ErrorIs(t, err, ErrServiceNotInitialized) + + _, err = StartWithOptions(context.Background(), RunOptions{Command: "echo"}) + assert.ErrorIs(t, err, ErrServiceNotInitialized) + + _, err = RunWithOptions(context.Background(), RunOptions{Command: "echo"}) + assert.ErrorIs(t, err, ErrServiceNotInitialized) +} + +func TestGlobal_SetDefault(t *testing.T) { + t.Run("sets and retrieves service", func(t *testing.T) { + // Reset global state + old := defaultService.Swap(nil) + defer func() { + if old != nil { + defaultService.Store(old) + } + }() + + core, err := framework.New( + framework.WithName("process", NewService(Options{})), + ) + require.NoError(t, err) + + svc, err := framework.ServiceFor[*Service](core, "process") + require.NoError(t, err) + + SetDefault(svc) + assert.Equal(t, svc, Default()) + }) + + t.Run("panics on nil", func(t *testing.T) { + assert.Panics(t, func() { + SetDefault(nil) + }) + }) +} + +func TestGlobal_ConcurrentDefault(t *testing.T) { + // Reset global state + old := defaultService.Swap(nil) + defer func() { + if old != nil { + defaultService.Store(old) + } + }() + + core, err := framework.New( + framework.WithName("process", NewService(Options{})), + ) + require.NoError(t, err) + + svc, err := framework.ServiceFor[*Service](core, "process") + require.NoError(t, err) + + SetDefault(svc) + + // Concurrent reads of Default() + var wg sync.WaitGroup + for i := 0; i < 100; i++ { + wg.Add(1) + go func() { + defer wg.Done() + s := Default() + assert.NotNil(t, s) + assert.Equal(t, svc, s) + }() + } + wg.Wait() +} + +func TestGlobal_ConcurrentSetDefault(t *testing.T) { + // Reset global state + old := defaultService.Swap(nil) + defer func() { + if old != nil { + defaultService.Store(old) + } + }() + + // Create multiple services + var services []*Service + for i := 0; i < 10; i++ { + core, err := framework.New( + framework.WithName("process", NewService(Options{})), + ) + require.NoError(t, err) + + svc, err := framework.ServiceFor[*Service](core, "process") + require.NoError(t, err) + services = append(services, svc) + } + + // Concurrent SetDefault calls - should not panic or race + var wg sync.WaitGroup + for _, svc := range services { + wg.Add(1) + go func(s *Service) { + defer wg.Done() + SetDefault(s) + }(svc) + } + wg.Wait() + + // Final state should be one of the services + final := Default() + assert.NotNil(t, final) + + found := false + for _, svc := range services { + if svc == final { + found = true + break + } + } + assert.True(t, found, "Default should be one of the set services") +} + +func TestGlobal_ConcurrentOperations(t *testing.T) { + // Reset global state + old := defaultService.Swap(nil) + defer func() { + if old != nil { + defaultService.Store(old) + } + }() + + core, err := framework.New( + framework.WithName("process", NewService(Options{})), + ) + require.NoError(t, err) + + svc, err := framework.ServiceFor[*Service](core, "process") + require.NoError(t, err) + + SetDefault(svc) + + // Concurrent Start, List, Get operations + var wg sync.WaitGroup + var processes []*Process + var procMu sync.Mutex + + // Start 20 processes concurrently + for i := 0; i < 20; i++ { + wg.Add(1) + go func() { + defer wg.Done() + proc, err := Start(context.Background(), "echo", "concurrent") + if err == nil { + procMu.Lock() + processes = append(processes, proc) + procMu.Unlock() + } + }() + } + + // Concurrent List calls while starting + for i := 0; i < 10; i++ { + wg.Add(1) + go func() { + defer wg.Done() + _ = List() + _ = Running() + }() + } + + wg.Wait() + + // Wait for all processes to complete + procMu.Lock() + for _, p := range processes { + <-p.Done() + } + procMu.Unlock() + + // All should have succeeded + assert.Len(t, processes, 20) + + // Concurrent Get calls + var wg2 sync.WaitGroup + for _, p := range processes { + wg2.Add(1) + go func(id string) { + defer wg2.Done() + got, err := Get(id) + assert.NoError(t, err) + assert.NotNil(t, got) + }(p.ID) + } + wg2.Wait() +} + +func TestGlobal_StartWithOptions(t *testing.T) { + svc, _ := newTestService(t) + + // Set as default + old := defaultService.Swap(svc) + defer func() { + if old != nil { + defaultService.Store(old) + } + }() + + proc, err := StartWithOptions(context.Background(), RunOptions{ + Command: "echo", + Args: []string{"with", "options"}, + }) + require.NoError(t, err) + + <-proc.Done() + + assert.Equal(t, 0, proc.ExitCode) + assert.Contains(t, proc.Output(), "with options") +} + +func TestGlobal_RunWithOptions(t *testing.T) { + svc, _ := newTestService(t) + + // Set as default + old := defaultService.Swap(svc) + defer func() { + if old != nil { + defaultService.Store(old) + } + }() + + output, err := RunWithOptions(context.Background(), RunOptions{ + Command: "echo", + Args: []string{"run", "options"}, + }) + require.NoError(t, err) + assert.Contains(t, output, "run options") +} + +func TestGlobal_Running(t *testing.T) { + svc, _ := newTestService(t) + + // Set as default + old := defaultService.Swap(svc) + defer func() { + if old != nil { + defaultService.Store(old) + } + }() + + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + + // Start a long-running process + proc, err := Start(ctx, "sleep", "60") + require.NoError(t, err) + + running := Running() + assert.Len(t, running, 1) + assert.Equal(t, proc.ID, running[0].ID) + + cancel() + <-proc.Done() + + running = Running() + assert.Len(t, running, 0) +} diff --git a/pkg/process/process_global.go b/pkg/process/process_global.go index b8d2bc3..9a0ffc8 100644 --- a/pkg/process/process_global.go +++ b/pkg/process/process_global.go @@ -92,6 +92,33 @@ func Kill(id string) error { return svc.Kill(id) } +// StartWithOptions spawns a process with full configuration using the default service. +func StartWithOptions(ctx context.Context, opts RunOptions) (*Process, error) { + svc := Default() + if svc == nil { + return nil, ErrServiceNotInitialized + } + return svc.StartWithOptions(ctx, opts) +} + +// RunWithOptions executes a command with options and waits using the default service. +func RunWithOptions(ctx context.Context, opts RunOptions) (string, error) { + svc := Default() + if svc == nil { + return "", ErrServiceNotInitialized + } + return svc.RunWithOptions(ctx, opts) +} + +// Running returns all currently running processes from the default service. +func Running() []*Process { + svc := Default() + if svc == nil { + return nil + } + return svc.Running() +} + // ErrServiceNotInitialized is returned when the service is not initialized. var ErrServiceNotInitialized = &ServiceError{msg: "process: service not initialized"} diff --git a/pkg/process/process_test.go b/pkg/process/process_test.go new file mode 100644 index 0000000..8bf7bf7 --- /dev/null +++ b/pkg/process/process_test.go @@ -0,0 +1,227 @@ +package process + +import ( + "context" + "testing" + "time" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestProcess_Info(t *testing.T) { + svc, _ := newTestService(t) + + proc, err := svc.Start(context.Background(), "echo", "hello") + require.NoError(t, err) + + <-proc.Done() + + info := proc.Info() + assert.Equal(t, proc.ID, info.ID) + assert.Equal(t, "echo", info.Command) + assert.Equal(t, []string{"hello"}, info.Args) + assert.Equal(t, StatusExited, info.Status) + assert.Equal(t, 0, info.ExitCode) + assert.Greater(t, info.Duration, time.Duration(0)) +} + +func TestProcess_Output(t *testing.T) { + t.Run("captures stdout", func(t *testing.T) { + svc, _ := newTestService(t) + + proc, err := svc.Start(context.Background(), "echo", "hello world") + require.NoError(t, err) + + <-proc.Done() + + output := proc.Output() + assert.Contains(t, output, "hello world") + }) + + t.Run("OutputBytes returns copy", func(t *testing.T) { + svc, _ := newTestService(t) + + proc, err := svc.Start(context.Background(), "echo", "test") + require.NoError(t, err) + + <-proc.Done() + + bytes := proc.OutputBytes() + assert.NotNil(t, bytes) + assert.Contains(t, string(bytes), "test") + }) +} + +func TestProcess_IsRunning(t *testing.T) { + t.Run("true while running", func(t *testing.T) { + svc, _ := newTestService(t) + + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + + proc, err := svc.Start(ctx, "sleep", "10") + require.NoError(t, err) + + assert.True(t, proc.IsRunning()) + + cancel() + <-proc.Done() + + assert.False(t, proc.IsRunning()) + }) + + t.Run("false after completion", func(t *testing.T) { + svc, _ := newTestService(t) + + proc, err := svc.Start(context.Background(), "echo", "done") + require.NoError(t, err) + + <-proc.Done() + + assert.False(t, proc.IsRunning()) + }) +} + +func TestProcess_Wait(t *testing.T) { + t.Run("returns nil on success", func(t *testing.T) { + svc, _ := newTestService(t) + + proc, err := svc.Start(context.Background(), "echo", "ok") + require.NoError(t, err) + + err = proc.Wait() + assert.NoError(t, err) + }) + + t.Run("returns error on failure", func(t *testing.T) { + svc, _ := newTestService(t) + + proc, err := svc.Start(context.Background(), "sh", "-c", "exit 1") + require.NoError(t, err) + + err = proc.Wait() + assert.Error(t, err) + }) +} + +func TestProcess_Done(t *testing.T) { + t.Run("channel closes on completion", func(t *testing.T) { + svc, _ := newTestService(t) + + proc, err := svc.Start(context.Background(), "echo", "test") + require.NoError(t, err) + + select { + case <-proc.Done(): + // Success - channel closed + case <-time.After(5 * time.Second): + t.Fatal("Done channel should have closed") + } + }) +} + +func TestProcess_Kill(t *testing.T) { + t.Run("terminates running process", func(t *testing.T) { + svc, _ := newTestService(t) + + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + + proc, err := svc.Start(ctx, "sleep", "60") + require.NoError(t, err) + + assert.True(t, proc.IsRunning()) + + err = proc.Kill() + assert.NoError(t, err) + + select { + case <-proc.Done(): + // Good - process terminated + case <-time.After(2 * time.Second): + t.Fatal("process should have been killed") + } + }) + + t.Run("noop on completed process", func(t *testing.T) { + svc, _ := newTestService(t) + + proc, err := svc.Start(context.Background(), "echo", "done") + require.NoError(t, err) + + <-proc.Done() + + err = proc.Kill() + assert.NoError(t, err) + }) +} + +func TestProcess_SendInput(t *testing.T) { + t.Run("writes to stdin", func(t *testing.T) { + svc, _ := newTestService(t) + + // Use cat to echo back stdin + proc, err := svc.Start(context.Background(), "cat") + require.NoError(t, err) + + err = proc.SendInput("hello\n") + assert.NoError(t, err) + + err = proc.CloseStdin() + assert.NoError(t, err) + + <-proc.Done() + + assert.Contains(t, proc.Output(), "hello") + }) + + t.Run("error on completed process", func(t *testing.T) { + svc, _ := newTestService(t) + + proc, err := svc.Start(context.Background(), "echo", "done") + require.NoError(t, err) + + <-proc.Done() + + err = proc.SendInput("test") + assert.ErrorIs(t, err, ErrProcessNotRunning) + }) +} + +func TestProcess_CloseStdin(t *testing.T) { + t.Run("closes stdin pipe", func(t *testing.T) { + svc, _ := newTestService(t) + + proc, err := svc.Start(context.Background(), "cat") + require.NoError(t, err) + + err = proc.CloseStdin() + assert.NoError(t, err) + + // Process should exit now that stdin is closed + select { + case <-proc.Done(): + // Good + case <-time.After(2 * time.Second): + t.Fatal("cat should exit when stdin is closed") + } + }) + + t.Run("double close is safe", func(t *testing.T) { + svc, _ := newTestService(t) + + proc, err := svc.Start(context.Background(), "cat") + require.NoError(t, err) + + // First close + err = proc.CloseStdin() + assert.NoError(t, err) + + <-proc.Done() + + // Second close should be safe (stdin already nil) + err = proc.CloseStdin() + assert.NoError(t, err) + }) +} diff --git a/pkg/process/service_test.go b/pkg/process/service_test.go index a4b8cdf..dba9d82 100644 --- a/pkg/process/service_test.go +++ b/pkg/process/service_test.go @@ -256,16 +256,3 @@ func TestService_Clear(t *testing.T) { }) } -func TestProcess_Info(t *testing.T) { - svc, _ := newTestService(t) - - proc, _ := svc.Start(context.Background(), "echo", "hello") - <-proc.Done() - - info := proc.Info() - assert.Equal(t, proc.ID, info.ID) - assert.Equal(t, "echo", info.Command) - assert.Equal(t, []string{"hello"}, info.Args) - assert.Equal(t, StatusExited, info.Status) - assert.Equal(t, 0, info.ExitCode) -}