diff --git a/pkg/instance/instance_test.go b/pkg/instance/instance_test.go index d89bad3..3ed6c08 100644 --- a/pkg/instance/instance_test.go +++ b/pkg/instance/instance_test.go @@ -91,38 +91,6 @@ func TestNewInstance_WithRestartOptions(t *testing.T) { } } -func TestNewInstance_ValidationAndDefaults(t *testing.T) { - globalSettings := &config.InstancesConfig{ - LogsDir: "/tmp/test", - DefaultAutoRestart: true, - DefaultMaxRestarts: 3, - DefaultRestartDelay: 5, - } - - // Test with invalid negative values - invalidMaxRestarts := -5 - invalidRestartDelay := -10 - - options := &instance.CreateInstanceOptions{ - MaxRestarts: &invalidMaxRestarts, - RestartDelay: &invalidRestartDelay, - LlamaServerOptions: llamacpp.LlamaServerOptions{ - Model: "/path/to/model.gguf", - }, - } - - instance := instance.NewInstance("test-instance", globalSettings, options) - opts := instance.GetOptions() - - // Check that negative values were corrected to 0 - if opts.MaxRestarts == nil || *opts.MaxRestarts != 0 { - t.Errorf("Expected MaxRestarts to be corrected to 0, got %v", opts.MaxRestarts) - } - if opts.RestartDelay == nil || *opts.RestartDelay != 0 { - t.Errorf("Expected RestartDelay to be corrected to 0, got %v", opts.RestartDelay) - } -} - func TestSetOptions(t *testing.T) { globalSettings := &config.InstancesConfig{ LogsDir: "/tmp/test", @@ -164,33 +132,6 @@ func TestSetOptions(t *testing.T) { } } -func TestSetOptions_NilOptions(t *testing.T) { - globalSettings := &config.InstancesConfig{ - LogsDir: "/tmp/test", - DefaultAutoRestart: true, - DefaultMaxRestarts: 3, - DefaultRestartDelay: 5, - } - - options := &instance.CreateInstanceOptions{ - LlamaServerOptions: llamacpp.LlamaServerOptions{ - Model: "/path/to/model.gguf", - }, - } - - instance := instance.NewInstance("test-instance", globalSettings, options) - originalOptions := instance.GetOptions() - - // Try to set nil options - instance.SetOptions(nil) - - // Options should remain unchanged - currentOptions := instance.GetOptions() - if currentOptions.Model != originalOptions.Model { - t.Error("Options should not change when setting nil options") - } -} - func TestGetProxy(t *testing.T) { globalSettings := &config.InstancesConfig{ LogsDir: "/tmp/test", @@ -317,58 +258,6 @@ func TestUnmarshalJSON(t *testing.T) { } } -func TestUnmarshalJSON_PartialOptions(t *testing.T) { - jsonData := `{ - "name": "test-instance", - "running": false, - "options": { - "model": "/path/to/model.gguf" - } - }` - - var inst instance.Process - err := json.Unmarshal([]byte(jsonData), &inst) - if err != nil { - t.Fatalf("JSON unmarshal failed: %v", err) - } - - opts := inst.GetOptions() - if opts.Model != "/path/to/model.gguf" { - t.Errorf("Expected model '/path/to/model.gguf', got %q", opts.Model) - } - - // Note: Defaults are NOT applied during unmarshaling - // They should only be applied by NewInstance or SetOptions - if opts.AutoRestart != nil { - t.Error("Expected AutoRestart to be nil (no defaults applied during unmarshal)") - } -} - -func TestUnmarshalJSON_NoOptions(t *testing.T) { - jsonData := `{ - "name": "test-instance", - "running": false - }` - - var inst instance.Process - err := json.Unmarshal([]byte(jsonData), &inst) - if err != nil { - t.Fatalf("JSON unmarshal failed: %v", err) - } - - if inst.Name != "test-instance" { - t.Errorf("Expected name 'test-instance', got %q", inst.Name) - } - if inst.Running { - t.Error("Expected running to be false") - } - - opts := inst.GetOptions() - if opts != nil { - t.Error("Expected options to be nil when not provided in JSON") - } -} - func TestCreateInstanceOptionsValidation(t *testing.T) { tests := []struct { name string @@ -377,13 +266,6 @@ func TestCreateInstanceOptionsValidation(t *testing.T) { expectedMax int expectedDelay int }{ - { - name: "nil values", - maxRestarts: nil, - restartDelay: nil, - expectedMax: 0, // Should remain nil, but we can't easily test nil in this structure - expectedDelay: 0, - }, { name: "valid positive values", maxRestarts: testutil.IntPtr(10), @@ -424,20 +306,16 @@ func TestCreateInstanceOptionsValidation(t *testing.T) { instance := instance.NewInstance("test", globalSettings, options) opts := instance.GetOptions() - if tt.maxRestarts != nil { - if opts.MaxRestarts == nil { - t.Error("Expected MaxRestarts to be set") - } else if *opts.MaxRestarts != tt.expectedMax { - t.Errorf("Expected MaxRestarts %d, got %d", tt.expectedMax, *opts.MaxRestarts) - } + if opts.MaxRestarts == nil { + t.Error("Expected MaxRestarts to be set") + } else if *opts.MaxRestarts != tt.expectedMax { + t.Errorf("Expected MaxRestarts %d, got %d", tt.expectedMax, *opts.MaxRestarts) } - if tt.restartDelay != nil { - if opts.RestartDelay == nil { - t.Error("Expected RestartDelay to be set") - } else if *opts.RestartDelay != tt.expectedDelay { - t.Errorf("Expected RestartDelay %d, got %d", tt.expectedDelay, *opts.RestartDelay) - } + if opts.RestartDelay == nil { + t.Error("Expected RestartDelay to be set") + } else if *opts.RestartDelay != tt.expectedDelay { + t.Errorf("Expected RestartDelay %d, got %d", tt.expectedDelay, *opts.RestartDelay) } }) } diff --git a/pkg/instance/timeout_test.go b/pkg/instance/timeout_test.go index 1a9579f..5cd67f2 100644 --- a/pkg/instance/timeout_test.go +++ b/pkg/instance/timeout_test.go @@ -46,20 +46,6 @@ func TestUpdateLastRequestTime(t *testing.T) { // Test that UpdateLastRequestTime doesn't panic inst.UpdateLastRequestTime() - - // Test concurrent calls to ensure thread safety - done := make(chan bool, 10) - for i := 0; i < 10; i++ { - go func() { - inst.UpdateLastRequestTime() - done <- true - }() - } - - // Wait for all goroutines to complete - for i := 0; i < 10; i++ { - <-done - } } func TestShouldTimeout_NotRunning(t *testing.T) { @@ -173,109 +159,6 @@ func TestShouldTimeout_ExceedsTimeLimit(t *testing.T) { } } -func TestShouldTimeout_TimeUnitConversion(t *testing.T) { - globalSettings := &config.InstancesConfig{ - LogsDir: "/tmp/test", - } - - tests := []struct { - name string - idleTimeout int // in minutes - lastRequestAge int // seconds ago - shouldTimeout bool - }{ - {"exactly at timeout boundary", 2, 120, false}, // 2 minutes = 120 seconds - {"just over timeout", 2, 121, true}, // 121 seconds > 120 seconds - {"well under timeout", 5, 60, false}, // 1 minute < 5 minutes - {"well over timeout", 1, 300, true}, // 5 minutes > 1 minute - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - options := &instance.CreateInstanceOptions{ - IdleTimeout: &tt.idleTimeout, - LlamaServerOptions: llamacpp.LlamaServerOptions{ - Model: "/path/to/model.gguf", - }, - } - - inst := instance.NewInstance("test-instance", globalSettings, options) - inst.Running = true - - // Use MockTimeProvider to control time - mockTime := NewMockTimeProvider(time.Now()) - inst.SetTimeProvider(mockTime) - - // Set last request time to now - inst.UpdateLastRequestTime() - - // Advance time by the specified amount - mockTime.SetTime(time.Now().Add(time.Duration(tt.lastRequestAge) * time.Second)) - - result := inst.ShouldTimeout() - if result != tt.shouldTimeout { - t.Errorf("Expected timeout=%v for %s, got %v", tt.shouldTimeout, tt.name, result) - } - }) - } -} - -func TestInstanceTimeoutInitialization(t *testing.T) { - globalSettings := &config.InstancesConfig{ - LogsDir: "/tmp/test", - } - - idleTimeout := 5 - options := &instance.CreateInstanceOptions{ - IdleTimeout: &idleTimeout, - LlamaServerOptions: llamacpp.LlamaServerOptions{ - Model: "/path/to/model.gguf", - }, - } - - inst := instance.NewInstance("test-instance", globalSettings, options) - inst.Running = true - - // Use MockTimeProvider to control time and verify initialization - mockTime := NewMockTimeProvider(time.Now()) - inst.SetTimeProvider(mockTime) - - // Update last request time (simulates what Start() does) - inst.UpdateLastRequestTime() - - // Should not timeout immediately after initialization with a 5-minute timeout - if inst.ShouldTimeout() { - t.Error("Fresh instance should not timeout immediately") - } - - // Now advance time by 6 minutes and verify it would timeout - mockTime.SetTime(time.Now().Add(6 * time.Minute)) - if !inst.ShouldTimeout() { - t.Error("Instance should timeout after exceeding idle timeout period") - } -} - -func TestTimeoutConfiguration_DefaultValue(t *testing.T) { - globalSettings := &config.InstancesConfig{ - LogsDir: "/tmp/test", - } - - // Create instance without specifying idle timeout - options := &instance.CreateInstanceOptions{ - LlamaServerOptions: llamacpp.LlamaServerOptions{ - Model: "/path/to/model.gguf", - }, - } - - inst := instance.NewInstance("test-instance", globalSettings, options) - opts := inst.GetOptions() - - // Default should be 0 (disabled) - if opts.IdleTimeout == nil || *opts.IdleTimeout != 0 { - t.Errorf("Expected default IdleTimeout to be 0, got %v", opts.IdleTimeout) - } -} - func TestTimeoutConfiguration_Validation(t *testing.T) { globalSettings := &config.InstancesConfig{ LogsDir: "/tmp/test", @@ -286,6 +169,7 @@ func TestTimeoutConfiguration_Validation(t *testing.T) { inputTimeout *int expectedTimeout int }{ + {"default value when nil", nil, 0}, {"positive value", testutil.IntPtr(10), 10}, {"zero value", testutil.IntPtr(0), 0}, {"negative value gets corrected", testutil.IntPtr(-5), 0},