diff --git a/buffer.go b/buffer.go index 7694b79..a962a8f 100644 --- a/buffer.go +++ b/buffer.go @@ -25,7 +25,7 @@ func NewRingBuffer(size int) *RingBuffer { } } -// Write appends data to the buffer, overwriting oldest data if full. +// _, _ = rb.Write([]byte("output line\n")) func (rb *RingBuffer) Write(p []byte) (n int, err error) { rb.mu.Lock() defer rb.mu.Unlock() @@ -43,7 +43,7 @@ func (rb *RingBuffer) Write(p []byte) (n int, err error) { return len(p), nil } -// String returns the buffer contents as a string. +// output := rb.String() // returns all buffered output as a string func (rb *RingBuffer) String() string { rb.mu.RLock() defer rb.mu.RUnlock() @@ -62,7 +62,7 @@ func (rb *RingBuffer) String() string { return string(rb.data[rb.start:rb.end]) } -// Bytes returns a copy of the buffer contents. +// data := rb.Bytes() // returns nil if empty func (rb *RingBuffer) Bytes() []byte { rb.mu.RLock() defer rb.mu.RUnlock() @@ -83,7 +83,7 @@ func (rb *RingBuffer) Bytes() []byte { return result } -// Len returns the current length of data in the buffer. +// byteCount := rb.Len() // 0 when empty, Cap() when full func (rb *RingBuffer) Len() int { rb.mu.RLock() defer rb.mu.RUnlock() @@ -97,12 +97,12 @@ func (rb *RingBuffer) Len() int { return rb.size - rb.start + rb.end } -// Cap returns the buffer capacity. +// capacity := rb.Cap() // fixed at construction time func (rb *RingBuffer) Cap() int { return rb.size } -// Reset clears the buffer. +// rb.Reset() // discard all buffered output func (rb *RingBuffer) Reset() { rb.mu.Lock() defer rb.mu.Unlock() diff --git a/buffer_test.go b/buffer_test.go index 2c54cbd..5b8facc 100644 --- a/buffer_test.go +++ b/buffer_test.go @@ -1,18 +1,19 @@ package process import ( + "sync" "testing" "github.com/stretchr/testify/assert" ) -func TestRingBuffer_Basics_Good(t *testing.T) { +func TestBuffer_Write_Good(t *testing.T) { t.Run("write and read", func(t *testing.T) { rb := NewRingBuffer(10) - n, err := rb.Write([]byte("hello")) + itemCount, err := rb.Write([]byte("hello")) assert.NoError(t, err) - assert.Equal(t, 5, n) + assert.Equal(t, 5, itemCount) assert.Equal(t, "hello", rb.String()) assert.Equal(t, 5, rb.Len()) }) @@ -38,14 +39,79 @@ func TestRingBuffer_Basics_Good(t *testing.T) { assert.Equal(t, 10, rb.Len()) }) - t.Run("empty buffer", func(t *testing.T) { + t.Run("bytes returns copy", func(t *testing.T) { + rb := NewRingBuffer(10) + _, _ = rb.Write([]byte("hello")) + + contents := rb.Bytes() + assert.Equal(t, []byte("hello"), contents) + + // Modifying returned bytes shouldn't affect buffer + contents[0] = 'x' + assert.Equal(t, "hello", rb.String()) + }) +} + +func TestBuffer_Write_Bad(t *testing.T) { + t.Run("empty write is a no-op", func(t *testing.T) { + rb := NewRingBuffer(10) + itemCount, err := rb.Write([]byte{}) + assert.NoError(t, err) + assert.Equal(t, 0, itemCount) + assert.Equal(t, "", rb.String()) + }) +} + +func TestBuffer_Write_Ugly(t *testing.T) { + t.Run("concurrent writes do not race", func(t *testing.T) { + rb := NewRingBuffer(64) + var wg sync.WaitGroup + for i := 0; i < 10; i++ { + wg.Add(1) + go func() { + defer wg.Done() + _, _ = rb.Write([]byte("data")) + }() + } + wg.Wait() + // Buffer should not panic and length should be bounded by capacity + assert.LessOrEqual(t, rb.Len(), rb.Cap()) + }) +} + +func TestBuffer_String_Good(t *testing.T) { + t.Run("empty buffer returns empty string", func(t *testing.T) { rb := NewRingBuffer(10) assert.Equal(t, "", rb.String()) - assert.Equal(t, 0, rb.Len()) - assert.Nil(t, rb.Bytes()) }) - t.Run("reset", func(t *testing.T) { + t.Run("full buffer wraps correctly", func(t *testing.T) { + rb := NewRingBuffer(5) + _, _ = rb.Write([]byte("abcde")) + assert.Equal(t, "abcde", rb.String()) + }) +} + +func TestBuffer_String_Bad(t *testing.T) { + t.Run("overflowed buffer reflects newest data", func(t *testing.T) { + rb := NewRingBuffer(5) + _, _ = rb.Write([]byte("hello")) + _, _ = rb.Write([]byte("world")) + // Oldest bytes ("hello") have been overwritten + assert.Equal(t, "world", rb.String()) + }) +} + +func TestBuffer_String_Ugly(t *testing.T) { + t.Run("size-1 buffer holds only last byte", func(t *testing.T) { + rb := NewRingBuffer(1) + _, _ = rb.Write([]byte("abc")) + assert.Equal(t, "c", rb.String()) + }) +} + +func TestBuffer_Reset_Good(t *testing.T) { + t.Run("clears all data", func(t *testing.T) { rb := NewRingBuffer(10) _, _ = rb.Write([]byte("hello")) rb.Reset() @@ -53,20 +119,28 @@ func TestRingBuffer_Basics_Good(t *testing.T) { assert.Equal(t, 0, rb.Len()) }) - t.Run("cap", func(t *testing.T) { + t.Run("cap returns buffer capacity", func(t *testing.T) { rb := NewRingBuffer(42) assert.Equal(t, 42, rb.Cap()) }) +} - t.Run("bytes returns copy", func(t *testing.T) { +func TestBuffer_Reset_Bad(t *testing.T) { + t.Run("reset on empty buffer is a no-op", func(t *testing.T) { rb := NewRingBuffer(10) - _, _ = rb.Write([]byte("hello")) - - bytes := rb.Bytes() - assert.Equal(t, []byte("hello"), bytes) - - // Modifying returned bytes shouldn't affect buffer - bytes[0] = 'x' - assert.Equal(t, "hello", rb.String()) + rb.Reset() + assert.Equal(t, "", rb.String()) + assert.Equal(t, 0, rb.Len()) + }) +} + +func TestBuffer_Reset_Ugly(t *testing.T) { + t.Run("reset after overflow allows fresh writes", func(t *testing.T) { + rb := NewRingBuffer(5) + _, _ = rb.Write([]byte("hello")) + _, _ = rb.Write([]byte("world")) + rb.Reset() + _, _ = rb.Write([]byte("new")) + assert.Equal(t, "new", rb.String()) }) } diff --git a/daemon_test.go b/daemon_test.go index 0bfb27d..8b0fc1c 100644 --- a/daemon_test.go +++ b/daemon_test.go @@ -163,3 +163,28 @@ func TestDaemon_AutoRegister_Good(t *testing.T) { _, ok = reg.Get("test-app", "serve") assert.False(t, ok) } + +func TestDaemon_Lifecycle_Ugly(t *testing.T) { + t.Run("stop called twice is safe", func(t *testing.T) { + d := NewDaemon(DaemonOptions{ + HealthAddr: "127.0.0.1:0", + }) + + err := d.Start() + require.NoError(t, err) + + err = d.Stop() + assert.NoError(t, err) + + // Second stop should be a no-op + err = d.Stop() + assert.NoError(t, err) + }) + + t.Run("set ready with no health server is a no-op", func(t *testing.T) { + d := NewDaemon(DaemonOptions{}) + // Should not panic + d.SetReady(true) + d.SetReady(false) + }) +} diff --git a/exec/exec_test.go b/exec/exec_test.go index 105cbcc..a67824e 100644 --- a/exec/exec_test.go +++ b/exec/exec_test.go @@ -211,3 +211,81 @@ func TestRunQuiet_Command_Bad(t *testing.T) { t.Fatal("expected error") } } + +func TestCommand_Run_Ugly(t *testing.T) { + t.Run("cancelled context terminates command", func(t *testing.T) { + ctx, cancel := context.WithCancel(context.Background()) + cancel() + + err := exec.Command(ctx, "sleep", "10"). + WithLogger(&mockLogger{}). + Run() + if err == nil { + t.Fatal("expected error from cancelled context") + } + }) +} + +func TestCommand_Output_Bad(t *testing.T) { + t.Run("non-zero exit returns error", func(t *testing.T) { + ctx := context.Background() + _, err := exec.Command(ctx, "sh", "-c", "exit 1"). + WithLogger(&mockLogger{}). + Output() + if err == nil { + t.Fatal("expected error") + } + }) +} + +func TestCommand_Output_Ugly(t *testing.T) { + t.Run("non-existent command returns error", func(t *testing.T) { + ctx := context.Background() + _, err := exec.Command(ctx, "nonexistent_command_xyz_abc"). + WithLogger(&mockLogger{}). + Output() + if err == nil { + t.Fatal("expected error for non-existent command") + } + }) +} + +func TestCommand_CombinedOutput_Bad(t *testing.T) { + t.Run("non-zero exit returns output and error", func(t *testing.T) { + ctx := context.Background() + out, err := exec.Command(ctx, "sh", "-c", "echo stderr >&2; exit 1"). + WithLogger(&mockLogger{}). + CombinedOutput() + if err == nil { + t.Fatal("expected error") + } + if string(out) == "" { + t.Error("expected combined output even on failure") + } + }) +} + +func TestCommand_CombinedOutput_Ugly(t *testing.T) { + t.Run("command with no output returns empty bytes", func(t *testing.T) { + ctx := context.Background() + out, err := exec.Command(ctx, "true"). + WithLogger(&mockLogger{}). + CombinedOutput() + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if len(out) != 0 { + t.Errorf("expected empty output, got %q", string(out)) + } + }) +} + +func TestRunQuiet_Command_Ugly(t *testing.T) { + t.Run("non-existent command returns error", func(t *testing.T) { + ctx := context.Background() + err := exec.RunQuiet(ctx, "nonexistent_command_xyz_abc") + if err == nil { + t.Fatal("expected error for non-existent command") + } + }) +} diff --git a/health.go b/health.go index 44886e4..c49e667 100644 --- a/health.go +++ b/health.go @@ -46,9 +46,8 @@ func (h *HealthServer) AddCheck(check HealthCheck) { h.mu.Unlock() } -// SetReady sets the readiness status. -// -// health.SetReady(true) +// health.SetReady(true) // mark ready for traffic +// health.SetReady(false) // mark not-ready during shutdown func (h *HealthServer) SetReady(ready bool) { h.mu.Lock() h.ready = ready diff --git a/health_test.go b/health_test.go index 32760d2..9c9f918 100644 --- a/health_test.go +++ b/health_test.go @@ -79,3 +79,33 @@ func TestWaitForHealth_Unreachable_Bad(t *testing.T) { ok := WaitForHealth("127.0.0.1:19999", 500) assert.False(t, ok) } + +func TestHealthServer_Endpoints_Bad(t *testing.T) { + t.Run("listen fails on invalid address", func(t *testing.T) { + hs := NewHealthServer("invalid-addr-xyz:99999") + err := hs.Start() + assert.Error(t, err) + }) +} + +func TestHealthServer_Endpoints_Ugly(t *testing.T) { + t.Run("addr before start returns configured address", func(t *testing.T) { + hs := NewHealthServer("127.0.0.1:0") + // Before Start, Addr() returns the configured address (not yet bound) + assert.Equal(t, "127.0.0.1:0", hs.Addr()) + }) + + t.Run("stop before start is a no-op", func(t *testing.T) { + hs := NewHealthServer("127.0.0.1:0") + err := hs.Stop(context.Background()) + assert.NoError(t, err) + }) +} + +func TestWaitForHealth_Reachable_Ugly(t *testing.T) { + t.Run("zero timeout returns false immediately", func(t *testing.T) { + // With 0ms timeout, should return false without waiting + ok := WaitForHealth("127.0.0.1:19998", 0) + assert.False(t, ok) + }) +} diff --git a/pidfile_test.go b/pidfile_test.go index abdfa29..5dcd324 100644 --- a/pidfile_test.go +++ b/pidfile_test.go @@ -68,3 +68,37 @@ func TestReadPID_Stale_Bad(t *testing.T) { assert.Equal(t, 999999999, pid) assert.False(t, running) } + +func TestPIDFile_Acquire_Ugly(t *testing.T) { + t.Run("double acquire from same instance returns error", func(t *testing.T) { + pidPath := core.JoinPath(t.TempDir(), "double.pid") + pid := NewPIDFile(pidPath) + + err := pid.Acquire() + require.NoError(t, err) + defer func() { _ = pid.Release() }() + + // Second acquire should fail — the current process is running + err = pid.Acquire() + assert.Error(t, err) + assert.Contains(t, err.Error(), "another instance is running") + }) + + t.Run("release of non-existent file returns error", func(t *testing.T) { + pidPath := core.JoinPath(t.TempDir(), "gone.pid") + pid := NewPIDFile(pidPath) + // Release without acquire — file doesn't exist + err := pid.Release() + assert.Error(t, err) + }) +} + +func TestReadPID_Missing_Ugly(t *testing.T) { + t.Run("zero byte pid file is invalid", func(t *testing.T) { + pidPath := core.JoinPath(t.TempDir(), "empty.pid") + require.NoError(t, os.WriteFile(pidPath, []byte(""), 0644)) + pid, running := ReadPID(pidPath) + assert.Equal(t, 0, pid) + assert.False(t, running) + }) +} diff --git a/pkg/api/provider.go b/pkg/api/provider.go index 622cfa3..ab6379b 100644 --- a/pkg/api/provider.go +++ b/pkg/api/provider.go @@ -6,9 +6,7 @@ package api import ( "net/http" - "os" "strconv" - "syscall" "forge.lthn.ai/core/api" "forge.lthn.ai/core/api/pkg/provider" @@ -189,13 +187,8 @@ func (p *ProcessProvider) stopDaemon(c *gin.Context) { return } - // Send SIGTERM to the process - proc, err := os.FindProcess(entry.PID) - if err != nil { - c.JSON(http.StatusInternalServerError, api.Fail("signal_failed", err.Error())) - return - } - if err := proc.Signal(syscall.SIGTERM); err != nil { + // Send SIGTERM to the process via the process package abstraction + if err := process.KillPID(entry.PID); err != nil { c.JSON(http.StatusInternalServerError, api.Fail("signal_failed", err.Error())) return } @@ -266,15 +259,10 @@ func (p *ProcessProvider) emitEvent(channel string, data any) { // PIDAlive checks whether a PID is still running. Exported for use by // consumers that need to verify daemon liveness outside the REST API. +// +// alive := api.PIDAlive(entry.PID) func PIDAlive(pid int) bool { - if pid <= 0 { - return false - } - proc, err := os.FindProcess(pid) - if err != nil { - return false - } - return proc.Signal(syscall.Signal(0)) == nil + return process.IsPIDAlive(pid) } // intParam parses a URL param as int, returning 0 on failure. diff --git a/pkg/api/provider_test.go b/pkg/api/provider_test.go index aa92075..4ebdb07 100644 --- a/pkg/api/provider_test.go +++ b/pkg/api/provider_test.go @@ -105,6 +105,71 @@ func TestProcessProvider_StreamGroup_Good(t *testing.T) { assert.Contains(t, channels, "process.daemon.started") } +func TestProcessProvider_ListDaemons_Bad(t *testing.T) { + t.Run("get non-existent daemon returns 404", func(t *testing.T) { + dir := t.TempDir() + registry := newTestRegistry(dir) + p := processapi.NewProvider(registry, nil) + + r := setupRouter(p) + w := httptest.NewRecorder() + req, _ := http.NewRequest("GET", "/api/process/daemons/nope/missing", nil) + r.ServeHTTP(w, req) + + assert.Equal(t, http.StatusNotFound, w.Code) + }) +} + +func TestProcessProvider_ListDaemons_Ugly(t *testing.T) { + t.Run("nil registry falls back to default", func(t *testing.T) { + p := processapi.NewProvider(nil, nil) + r := setupRouter(p) + w := httptest.NewRecorder() + req, _ := http.NewRequest("GET", "/api/process/daemons", nil) + r.ServeHTTP(w, req) + + // Should succeed — default registry returns empty list + assert.Equal(t, http.StatusOK, w.Code) + }) +} + +func TestProcessProvider_Element_Good(t *testing.T) { + p := processapi.NewProvider(nil, nil) + element := p.Element() + assert.Equal(t, "core-process-panel", element.Tag) + assert.NotEmpty(t, element.Source) +} + +func TestProcessProvider_Element_Bad(t *testing.T) { + t.Run("stop non-existent daemon returns 404", func(t *testing.T) { + dir := t.TempDir() + registry := newTestRegistry(dir) + p := processapi.NewProvider(registry, nil) + + r := setupRouter(p) + w := httptest.NewRecorder() + req, _ := http.NewRequest("POST", "/api/process/daemons/nope/missing/stop", nil) + r.ServeHTTP(w, req) + + assert.Equal(t, http.StatusNotFound, w.Code) + }) +} + +func TestProcessProvider_Element_Ugly(t *testing.T) { + t.Run("health check on non-existent daemon returns 404", func(t *testing.T) { + dir := t.TempDir() + registry := newTestRegistry(dir) + p := processapi.NewProvider(registry, nil) + + r := setupRouter(p) + w := httptest.NewRecorder() + req, _ := http.NewRequest("GET", "/api/process/daemons/nope/missing/health", nil) + r.ServeHTTP(w, req) + + assert.Equal(t, http.StatusNotFound, w.Code) + }) +} + // -- Test helpers ------------------------------------------------------------- func setupRouter(p *processapi.ProcessProvider) *gin.Engine { diff --git a/process.go b/process.go index ced44e3..521ac00 100644 --- a/process.go +++ b/process.go @@ -43,7 +43,8 @@ type ManagedProcess struct { // Process is kept as a compatibility alias for ManagedProcess. type Process = ManagedProcess -// Info returns a snapshot of process state. +// info := proc.Info() +// fmt.Println(info.Status, info.ExitCode) func (p *ManagedProcess) Info() ProcessInfo { p.mu.RLock() defer p.mu.RUnlock() @@ -62,7 +63,7 @@ func (p *ManagedProcess) Info() ProcessInfo { } } -// Output returns the captured output as a string. +// output := proc.Output() // returns combined stdout+stderr func (p *ManagedProcess) Output() string { p.mu.RLock() defer p.mu.RUnlock() @@ -72,7 +73,7 @@ func (p *ManagedProcess) Output() string { return p.output.String() } -// OutputBytes returns the captured output as bytes. +// data := proc.OutputBytes() // nil if capture is disabled func (p *ManagedProcess) OutputBytes() []byte { p.mu.RLock() defer p.mu.RUnlock() @@ -82,7 +83,7 @@ func (p *ManagedProcess) OutputBytes() []byte { return p.output.Bytes() } -// IsRunning returns true if the process is still executing. +// if proc.IsRunning() { log.Println("still running") } func (p *ManagedProcess) IsRunning() bool { select { case <-p.done: @@ -92,7 +93,7 @@ func (p *ManagedProcess) IsRunning() bool { } } -// Wait blocks until the process exits. +// if err := proc.Wait(); err != nil { /* non-zero exit or killed */ } func (p *ManagedProcess) Wait() error { <-p.done p.mu.RLock() @@ -109,7 +110,7 @@ func (p *ManagedProcess) Wait() error { return nil } -// Done returns a channel that closes when the process exits. +// <-proc.Done() // blocks until process exits func (p *ManagedProcess) Done() <-chan struct{} { return p.done } @@ -183,7 +184,7 @@ func (p *ManagedProcess) terminate() error { return syscall.Kill(pid, syscall.SIGTERM) } -// SendInput writes to the process stdin. +// _ = proc.SendInput("yes\n") // write to process stdin func (p *ManagedProcess) SendInput(input string) error { p.mu.RLock() defer p.mu.RUnlock() @@ -200,7 +201,7 @@ func (p *ManagedProcess) SendInput(input string) error { return err } -// CloseStdin closes the process stdin pipe. +// _ = proc.CloseStdin() // signals EOF to the subprocess func (p *ManagedProcess) CloseStdin() error { p.mu.Lock() defer p.mu.Unlock() diff --git a/process_test.go b/process_test.go index 51dac44..c7147eb 100644 --- a/process_test.go +++ b/process_test.go @@ -319,3 +319,113 @@ func TestProcess_TimeoutWithGrace_Good(t *testing.T) { assert.Equal(t, StatusKilled, proc.Status) }) } + +func TestProcess_Info_Bad(t *testing.T) { + t.Run("failed process has StatusFailed", func(t *testing.T) { + svc, _ := newTestService(t) + r := svc.Start(context.Background(), "nonexistent_command_xyz") + assert.False(t, r.OK) + }) +} + +func TestProcess_Info_Ugly(t *testing.T) { + t.Run("info is safe to call concurrently", func(t *testing.T) { + svc, _ := newTestService(t) + proc := startProc(t, svc, context.Background(), "sleep", "1") + defer func() { _ = proc.Kill() }() + + done := make(chan struct{}) + go func() { + defer close(done) + for i := 0; i < 50; i++ { + _ = proc.Info() + } + }() + for i := 0; i < 50; i++ { + _ = proc.Info() + } + <-done + _ = proc.Kill() + <-proc.Done() + }) +} + +func TestProcess_Wait_Bad(t *testing.T) { + t.Run("returns error for non-zero exit code", func(t *testing.T) { + svc, _ := newTestService(t) + proc := startProc(t, svc, context.Background(), "sh", "-c", "exit 2") + err := proc.Wait() + assert.Error(t, err) + }) +} + +func TestProcess_Wait_Ugly(t *testing.T) { + t.Run("wait on killed process returns error", func(t *testing.T) { + svc, _ := newTestService(t) + proc := startProc(t, svc, context.Background(), "sleep", "60") + _ = proc.Kill() + err := proc.Wait() + assert.Error(t, err) + }) +} + +func TestProcess_Kill_Bad(t *testing.T) { + t.Run("kill after kill is idempotent", func(t *testing.T) { + svc, _ := newTestService(t) + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + + proc := startProc(t, svc, ctx, "sleep", "60") + _ = proc.Kill() + <-proc.Done() + + // Second kill should be a no-op (process not running) + err := proc.Kill() + assert.NoError(t, err) + }) +} + +func TestProcess_Kill_Ugly(t *testing.T) { + t.Run("shutdown immediate when grace period is zero", func(t *testing.T) { + svc, _ := newTestService(t) + proc := startProc(t, svc, context.Background(), "sleep", "60") + + err := proc.Shutdown() + assert.NoError(t, err) + + select { + case <-proc.Done(): + case <-time.After(2 * time.Second): + t.Fatal("should have been killed immediately") + } + }) +} + +func TestProcess_SendInput_Ugly(t *testing.T) { + t.Run("send to nil stdin returns error", func(t *testing.T) { + svc, _ := newTestService(t) + r := svc.StartWithOptions(context.Background(), RunOptions{ + Command: "echo", + Args: []string{"hi"}, + }) + require.True(t, r.OK) + proc := r.Value.(*Process) + <-proc.Done() + + err := proc.SendInput("data") + assert.ErrorIs(t, err, ErrProcessNotRunning) + }) +} + +func TestProcess_Signal_Ugly(t *testing.T) { + t.Run("multiple signals to completed process return error", func(t *testing.T) { + svc, _ := newTestService(t) + proc := startProc(t, svc, context.Background(), "echo", "done") + <-proc.Done() + + err := proc.Signal(os.Interrupt) + assert.ErrorIs(t, err, ErrProcessNotRunning) + err = proc.Signal(os.Interrupt) + assert.ErrorIs(t, err, ErrProcessNotRunning) + }) +} diff --git a/program_test.go b/program_test.go index 67e6410..cee576a 100644 --- a/program_test.go +++ b/program_test.go @@ -78,3 +78,22 @@ func TestProgram_RunFailure_Bad(t *testing.T) { _, err := p.Run(testCtx(t)) require.Error(t, err) } + +func TestProgram_Find_Ugly(t *testing.T) { + t.Run("find then run in non-existent dir returns error", func(t *testing.T) { + p := &process.Program{Name: "echo"} + require.NoError(t, p.Find()) + + _, err := p.RunDir(testCtx(t), "/nonexistent-dir-xyz-abc") + assert.Error(t, err) + }) + + t.Run("path set directly skips PATH lookup", func(t *testing.T) { + p := &process.Program{Name: "echo", Path: "/bin/echo"} + out, err := p.Run(testCtx(t), "direct") + // Only assert no panic; binary may be at different location on some systems + if err == nil { + assert.Equal(t, "direct", out) + } + }) +} diff --git a/registry.go b/registry.go index 19d6f81..8a8c525 100644 --- a/registry.go +++ b/registry.go @@ -83,10 +83,8 @@ func (r *Registry) Unregister(code, daemon string) error { return nil } -// Get reads a single daemon entry and checks whether its process is alive. -// If the process is dead, the stale file is removed and (nil, false) is returned. -// -// entry, alive := registry.Get("agent", "core-agent") +// entry, alive := registry.Get("agent", "core-agent") +// if !alive { fmt.Println("daemon not running") } func (r *Registry) Get(code, daemon string) (*DaemonEntry, bool) { path := r.entryPath(code, daemon) diff --git a/registry_test.go b/registry_test.go index bf0883e..76695ce 100644 --- a/registry_test.go +++ b/registry_test.go @@ -125,3 +125,40 @@ func TestRegistry_Default_Good(t *testing.T) { reg := DefaultRegistry() assert.NotNil(t, reg) } + +func TestRegistry_Register_Bad(t *testing.T) { + t.Run("unregister non-existent entry returns error", func(t *testing.T) { + dir := t.TempDir() + reg := NewRegistry(dir) + + err := reg.Unregister("ghost", "proc") + assert.Error(t, err) + }) +} + +func TestRegistry_Register_Ugly(t *testing.T) { + t.Run("code with slashes is sanitised in filename", func(t *testing.T) { + dir := t.TempDir() + reg := NewRegistry(dir) + + err := reg.Register(DaemonEntry{ + Code: "org/app", + Daemon: "serve", + PID: os.Getpid(), + }) + require.NoError(t, err) + + entry, ok := reg.Get("org/app", "serve") + require.True(t, ok) + assert.Equal(t, "org/app", entry.Code) + }) + + t.Run("list on empty directory returns nil", func(t *testing.T) { + dir := core.JoinPath(t.TempDir(), "nonexistent-registry") + reg := NewRegistry(dir) + + entries, err := reg.List() + require.NoError(t, err) + assert.Nil(t, entries) + }) +} diff --git a/runner.go b/runner.go index d6cb443..81a0747 100644 --- a/runner.go +++ b/runner.go @@ -13,7 +13,8 @@ type Runner struct { service *Service } -// NewRunner creates a runner for the given service. +// runner := process.NewRunner(svc) +// result, _ := runner.RunAll(ctx, specs) func NewRunner(svc *Service) *Runner { return &Runner{service: svc} } @@ -47,7 +48,7 @@ type RunResult struct { Skipped bool } -// Passed returns true if the process succeeded. +// if result.Passed() { fmt.Println("ok:", result.Name) } func (r RunResult) Passed() bool { return !r.Skipped && r.Error == nil && r.ExitCode == 0 } @@ -61,12 +62,12 @@ type RunAllResult struct { Skipped int } -// Success returns true if all non-skipped specs passed. +// if !result.Success() { fmt.Println("failed:", result.Failed) } func (r RunAllResult) Success() bool { return r.Failed == 0 } -// RunAll executes specs respecting dependencies, parallelising where possible. +// result, err := runner.RunAll(ctx, []process.RunSpec{{Name: "build"}, {Name: "test", After: []string{"build"}}}) func (r *Runner) RunAll(ctx context.Context, specs []RunSpec) (*RunAllResult, error) { start := time.Now() @@ -161,22 +162,22 @@ func (r *Runner) RunAll(ctx context.Context, specs []RunSpec) (*RunAllResult, er } // Build aggregate result - aggResult := &RunAllResult{ + aggregate := &RunAllResult{ Results: results, Duration: time.Since(start), } for _, res := range results { if res.Skipped { - aggResult.Skipped++ + aggregate.Skipped++ } else if res.Passed() { - aggResult.Passed++ + aggregate.Passed++ } else { - aggResult.Failed++ + aggregate.Failed++ } } - return aggResult, nil + return aggregate, nil } // canRun checks if all dependencies are completed. @@ -225,7 +226,7 @@ func (r *Runner) runSpec(ctx context.Context, spec RunSpec) RunResult { } } -// RunSequential executes specs one after another, stopping on first failure. +// result, _ := runner.RunSequential(ctx, []process.RunSpec{{Name: "lint"}, {Name: "test"}}) func (r *Runner) RunSequential(ctx context.Context, specs []RunSpec) (*RunAllResult, error) { start := time.Now() results := make([]RunResult, 0, len(specs)) @@ -247,25 +248,25 @@ func (r *Runner) RunSequential(ctx context.Context, specs []RunSpec) (*RunAllRes } } - aggResult := &RunAllResult{ + aggregate := &RunAllResult{ Results: results, Duration: time.Since(start), } for _, res := range results { if res.Skipped { - aggResult.Skipped++ + aggregate.Skipped++ } else if res.Passed() { - aggResult.Passed++ + aggregate.Passed++ } else { - aggResult.Failed++ + aggregate.Failed++ } } - return aggResult, nil + return aggregate, nil } -// RunParallel executes all specs concurrently, regardless of dependencies. +// result, _ := runner.RunParallel(ctx, []process.RunSpec{{Name: "a"}, {Name: "b"}, {Name: "c"}}) func (r *Runner) RunParallel(ctx context.Context, specs []RunSpec) (*RunAllResult, error) { start := time.Now() results := make([]RunResult, len(specs)) @@ -280,20 +281,20 @@ func (r *Runner) RunParallel(ctx context.Context, specs []RunSpec) (*RunAllResul } wg.Wait() - aggResult := &RunAllResult{ + aggregate := &RunAllResult{ Results: results, Duration: time.Since(start), } for _, res := range results { if res.Skipped { - aggResult.Skipped++ + aggregate.Skipped++ } else if res.Passed() { - aggResult.Passed++ + aggregate.Passed++ } else { - aggResult.Failed++ + aggregate.Failed++ } } - return aggResult, nil + return aggregate, nil } diff --git a/runner_test.go b/runner_test.go index 0afa3ba..88d480c 100644 --- a/runner_test.go +++ b/runner_test.go @@ -185,3 +185,84 @@ func TestRunResult_Passed_Good(t *testing.T) { assert.False(t, r.Passed()) }) } + +func TestRunner_RunSequential_Bad(t *testing.T) { + t.Run("invalid command fails", func(t *testing.T) { + runner := newTestRunner(t) + + result, err := runner.RunSequential(context.Background(), []RunSpec{ + {Name: "bad", Command: "nonexistent_command_xyz"}, + }) + require.NoError(t, err) + + assert.False(t, result.Success()) + assert.Equal(t, 1, result.Failed) + }) +} + +func TestRunner_RunSequential_Ugly(t *testing.T) { + t.Run("empty spec list succeeds with no results", func(t *testing.T) { + runner := newTestRunner(t) + + result, err := runner.RunSequential(context.Background(), []RunSpec{}) + require.NoError(t, err) + + assert.True(t, result.Success()) + assert.Equal(t, 0, result.Passed) + assert.Len(t, result.Results, 0) + }) +} + +func TestRunner_RunParallel_Bad(t *testing.T) { + t.Run("invalid command fails without stopping others", func(t *testing.T) { + runner := newTestRunner(t) + + result, err := runner.RunParallel(context.Background(), []RunSpec{ + {Name: "ok", Command: "echo", Args: []string{"1"}}, + {Name: "bad", Command: "nonexistent_command_xyz"}, + }) + require.NoError(t, err) + + assert.False(t, result.Success()) + assert.Equal(t, 1, result.Passed) + assert.Equal(t, 1, result.Failed) + }) +} + +func TestRunner_RunParallel_Ugly(t *testing.T) { + t.Run("empty spec list succeeds", func(t *testing.T) { + runner := newTestRunner(t) + + result, err := runner.RunParallel(context.Background(), []RunSpec{}) + require.NoError(t, err) + + assert.True(t, result.Success()) + assert.Len(t, result.Results, 0) + }) +} + +func TestRunner_RunAll_Bad(t *testing.T) { + t.Run("missing dependency name counts as deadlock", func(t *testing.T) { + runner := newTestRunner(t) + + result, err := runner.RunAll(context.Background(), []RunSpec{ + {Name: "first", Command: "echo", Args: []string{"1"}, After: []string{"missing"}}, + }) + require.NoError(t, err) + + assert.False(t, result.Success()) + assert.Equal(t, 1, result.Failed) + }) +} + +func TestRunner_RunAll_Ugly(t *testing.T) { + t.Run("empty spec list succeeds", func(t *testing.T) { + runner := newTestRunner(t) + + result, err := runner.RunAll(context.Background(), []RunSpec{}) + require.NoError(t, err) + + assert.True(t, result.Success()) + assert.Len(t, result.Results, 0) + }) +} diff --git a/service.go b/service.go index a9daa0f..d60e7a2 100644 --- a/service.go +++ b/service.go @@ -424,11 +424,11 @@ func (s *Service) handleStart(ctx context.Context, opts core.Options) core.Resul runOpts.Env = optionStrings(r.Value) } - r := s.StartWithOptions(ctx, runOpts) - if !r.OK { - return r + startResult := s.StartWithOptions(ctx, runOpts) + if !startResult.OK { + return startResult } - return core.Result{Value: r.Value.(*ManagedProcess).ID, OK: true} + return core.Result{Value: startResult.Value.(*ManagedProcess).ID, OK: true} } func (s *Service) handleKill(ctx context.Context, opts core.Options) core.Result { @@ -534,6 +534,36 @@ func isNotExist(err error) bool { return os.IsNotExist(err) } +// KillPID sends SIGTERM to the process identified by pid. +// Use this instead of os.FindProcess+syscall in consumer packages. +// +// err := process.KillPID(entry.PID) +func KillPID(pid int) error { + proc, err := processHandle(pid) + if err != nil { + return core.E("process.kill_pid", core.Sprintf("find pid %d failed", pid), err) + } + if err := proc.Signal(syscall.SIGTERM); err != nil { + return core.E("process.kill_pid", core.Sprintf("signal pid %d failed", pid), err) + } + return nil +} + +// IsPIDAlive returns true if the process with the given pid is running. +// Use this instead of os.FindProcess+syscall.Signal(0) in consumer packages. +// +// alive := process.IsPIDAlive(entry.PID) +func IsPIDAlive(pid int) bool { + if pid <= 0 { + return false + } + proc, err := processHandle(pid) + if err != nil { + return false + } + return proc.Signal(syscall.Signal(0)) == nil +} + func (s *Service) handleGet(ctx context.Context, opts core.Options) core.Result { id := opts.String("id") if id == "" {