Improve manager tests

pkg/manager/operations_test.go

@@ -38,8 +38,7 @@ func TestCreateInstance_Success(t *testing.T) {
 	}
 }
 
-func TestCreateInstance_ValidationAndLimits(t *testing.T) {
-	// Test duplicate names
+func TestCreateInstance_DuplicateName(t *testing.T) {
 	mngr := createTestManager()
 	options := &instance.Options{
 		BackendOptions: backends.Options{
@@ -63,15 +62,13 @@ func TestCreateInstance_ValidationAndLimits(t *testing.T) {
 	if !strings.Contains(err.Error(), "already exists") {
 		t.Errorf("Expected duplicate name error, got: %v", err)
 	}
+}
 
-	// Test max instances limit
+func TestCreateInstance_MaxInstancesLimit(t *testing.T) {
 	backendConfig := config.BackendConfig{
 		LlamaCpp: config.BackendSettings{
 			Command: "llama-server",
 		},
-		MLX: config.BackendSettings{
-			Command: "mlx_lm.server",
-		},
 	}
 	cfg := config.InstancesConfig{
 		PortRange:            [2]int{8000, 9000},
@@ -80,7 +77,16 @@ func TestCreateInstance_ValidationAndLimits(t *testing.T) {
 	}
 	limitedManager := manager.New(backendConfig, cfg, map[string]config.NodeConfig{}, "main")
 
-	_, err = limitedManager.CreateInstance("instance1", options)
+	options := &instance.Options{
+		BackendOptions: backends.Options{
+			BackendType: backends.BackendTypeLlamaCpp,
+			LlamaServerOptions: &backends.LlamaServerOptions{
+				Model: "/path/to/model.gguf",
+			},
+		},
+	}
+
+	_, err := limitedManager.CreateInstance("instance1", options)
 	if err != nil {
 		t.Fatalf("CreateInstance 1 failed: %v", err)
 	}
@@ -95,78 +101,98 @@ func TestCreateInstance_ValidationAndLimits(t *testing.T) {
 	}
 }
 
-func TestPortManagement(t *testing.T) {
-	manager := createTestManager()
-
-	// Test auto port assignment
-	options1 := &instance.Options{
-		BackendOptions: backends.Options{
-			BackendType: backends.BackendTypeLlamaCpp,
-			LlamaServerOptions: &backends.LlamaServerOptions{
-				Model: "/path/to/model.gguf",
-			},
-		},
-	}
-
-	inst1, err := manager.CreateInstance("instance1", options1)
-	if err != nil {
-		t.Fatalf("CreateInstance failed: %v", err)
-	}
-
-	port1 := inst1.GetPort()
-	if port1 < 8000 || port1 > 9000 {
-		t.Errorf("Expected port in range 8000-9000, got %d", port1)
-	}
-
-	// Test port conflict detection
-	options2 := &instance.Options{
-		BackendOptions: backends.Options{
-			BackendType: backends.BackendTypeLlamaCpp,
-			LlamaServerOptions: &backends.LlamaServerOptions{
-				Model: "/path/to/model2.gguf",
-				Port:  port1, // Same port - should conflict
-			},
-		},
-	}
-
-	_, err = manager.CreateInstance("instance2", options2)
-	if err == nil {
-		t.Error("Expected error for port conflict")
-	}
-	if !strings.Contains(err.Error(), "port") && !strings.Contains(err.Error(), "in use") {
-		t.Errorf("Expected port conflict error, got: %v", err)
-	}
-
-	// Test port release on deletion
-	specificPort := 8080
-	options3 := &instance.Options{
-		BackendOptions: backends.Options{
-			BackendType: backends.BackendTypeLlamaCpp,
-			LlamaServerOptions: &backends.LlamaServerOptions{
-				Model: "/path/to/model.gguf",
-				Port:  specificPort,
-			},
-		},
-	}
-
-	_, err = manager.CreateInstance("port-test", options3)
-	if err != nil {
-		t.Fatalf("CreateInstance failed: %v", err)
-	}
-
-	err = manager.DeleteInstance("port-test")
-	if err != nil {
-		t.Fatalf("DeleteInstance failed: %v", err)
-	}
-
-	// Should be able to create new instance with same port
-	_, err = manager.CreateInstance("new-port-test", options3)
-	if err != nil {
-		t.Errorf("Expected to reuse port after deletion, got error: %v", err)
-	}
-}
-
-func TestInstanceOperations(t *testing.T) {
+func TestPort_AutoAssignment(t *testing.T) {
+	manager := createTestManager()
+
+	options := &instance.Options{
+		BackendOptions: backends.Options{
+			BackendType: backends.BackendTypeLlamaCpp,
+			LlamaServerOptions: &backends.LlamaServerOptions{
+				Model: "/path/to/model.gguf",
+			},
+		},
+	}
+
+	inst, err := manager.CreateInstance("instance1", options)
+	if err != nil {
+		t.Fatalf("CreateInstance failed: %v", err)
+	}
+
+	port := inst.GetPort()
+	if port < 8000 || port > 9000 {
+		t.Errorf("Expected port in range 8000-9000, got %d", port)
+	}
+}
+
+func TestPort_ConflictDetection(t *testing.T) {
+	manager := createTestManager()
+
+	options1 := &instance.Options{
+		BackendOptions: backends.Options{
+			BackendType: backends.BackendTypeLlamaCpp,
+			LlamaServerOptions: &backends.LlamaServerOptions{
+				Model: "/path/to/model.gguf",
+				Port:  8080,
+			},
+		},
+	}
+
+	_, err := manager.CreateInstance("instance1", options1)
+	if err != nil {
+		t.Fatalf("CreateInstance failed: %v", err)
+	}
+
+	// Try to create instance with same port
+	options2 := &instance.Options{
+		BackendOptions: backends.Options{
+			BackendType: backends.BackendTypeLlamaCpp,
+			LlamaServerOptions: &backends.LlamaServerOptions{
+				Model: "/path/to/model2.gguf",
+				Port:  8080, // Same port - should conflict
+			},
+		},
+	}
+
+	_, err = manager.CreateInstance("instance2", options2)
+	if err == nil {
+		t.Error("Expected error for port conflict")
+	}
+	if !strings.Contains(err.Error(), "port") && !strings.Contains(err.Error(), "in use") {
+		t.Errorf("Expected port conflict error, got: %v", err)
+	}
+}
+
+func TestPort_ReleaseOnDeletion(t *testing.T) {
+	manager := createTestManager()
+
+	options := &instance.Options{
+		BackendOptions: backends.Options{
+			BackendType: backends.BackendTypeLlamaCpp,
+			LlamaServerOptions: &backends.LlamaServerOptions{
+				Model: "/path/to/model.gguf",
+				Port:  8080,
+			},
+		},
+	}
+
+	_, err := manager.CreateInstance("port-test", options)
+	if err != nil {
+		t.Fatalf("CreateInstance failed: %v", err)
+	}
+
+	err = manager.DeleteInstance("port-test")
+	if err != nil {
+		t.Fatalf("DeleteInstance failed: %v", err)
+	}
+
+	// Should be able to create new instance with same port
+	_, err = manager.CreateInstance("new-port-test", options)
+	if err != nil {
+		t.Errorf("Expected to reuse port after deletion, got error: %v", err)
+	}
+}
+
+func TestGetInstance(t *testing.T) {
 	manager := createTestManager()
 
 	options := &instance.Options{
@@ -178,13 +204,11 @@ func TestInstanceOperations(t *testing.T) {
 		},
 	}
 
-	// Create instance
 	created, err := manager.CreateInstance("test-instance", options)
 	if err != nil {
 		t.Fatalf("CreateInstance failed: %v", err)
 	}
 
-	// Get instance
 	retrieved, err := manager.GetInstance("test-instance")
 	if err != nil {
 		t.Fatalf("GetInstance failed: %v", err)
@@ -192,8 +216,26 @@ func TestInstanceOperations(t *testing.T) {
 	if retrieved.Name != created.Name {
 		t.Errorf("Expected name %q, got %q", created.Name, retrieved.Name)
 	}
+}
+
+func TestUpdateInstance(t *testing.T) {
+	manager := createTestManager()
+
+	options := &instance.Options{
+		BackendOptions: backends.Options{
+			BackendType: backends.BackendTypeLlamaCpp,
+			LlamaServerOptions: &backends.LlamaServerOptions{
+				Model: "/path/to/model.gguf",
+				Port:  8080,
+			},
+		},
+	}
+
+	_, err := manager.CreateInstance("test-instance", options)
+	if err != nil {
+		t.Fatalf("CreateInstance failed: %v", err)
+	}
 
-	// Update instance
 	newOptions := &instance.Options{
 		BackendOptions: backends.Options{
 			BackendType: backends.BackendTypeLlamaCpp,
@@ -211,8 +253,25 @@ func TestInstanceOperations(t *testing.T) {
 	if updated.GetOptions().BackendOptions.LlamaServerOptions.Model != "/path/to/new-model.gguf" {
 		t.Errorf("Expected model '/path/to/new-model.gguf', got %q", updated.GetOptions().BackendOptions.LlamaServerOptions.Model)
 	}
+}
+
+func TestListInstances(t *testing.T) {
+	manager := createTestManager()
+
+	options := &instance.Options{
+		BackendOptions: backends.Options{
+			BackendType: backends.BackendTypeLlamaCpp,
+			LlamaServerOptions: &backends.LlamaServerOptions{
+				Model: "/path/to/model.gguf",
+			},
+		},
+	}
+
+	_, err := manager.CreateInstance("test-instance", options)
+	if err != nil {
+		t.Fatalf("CreateInstance failed: %v", err)
+	}
 
-	// List instances
 	instances, err := manager.ListInstances()
 	if err != nil {
 		t.Fatalf("ListInstances failed: %v", err)
@@ -220,8 +279,25 @@ func TestInstanceOperations(t *testing.T) {
 	if len(instances) != 1 {
 		t.Errorf("Expected 1 instance, got %d", len(instances))
 	}
+}
+
+func TestDeleteInstance(t *testing.T) {
+	manager := createTestManager()
+
+	options := &instance.Options{
+		BackendOptions: backends.Options{
+			BackendType: backends.BackendTypeLlamaCpp,
+			LlamaServerOptions: &backends.LlamaServerOptions{
+				Model: "/path/to/model.gguf",
+			},
+		},
+	}
+
+	_, err := manager.CreateInstance("test-instance", options)
+	if err != nil {
+		t.Fatalf("CreateInstance failed: %v", err)
+	}
 
-	// Delete instance
 	err = manager.DeleteInstance("test-instance")
 	if err != nil {
 		t.Fatalf("DeleteInstance failed: %v", err)
@@ -231,9 +307,21 @@ func TestInstanceOperations(t *testing.T) {
 	if err == nil {
 		t.Error("Instance should not exist after deletion")
 	}
+}
 
-	// Test operations on non-existent instances
-	_, err = manager.GetInstance("nonexistent")
+func TestInstanceOperations_NonExistentInstance(t *testing.T) {
+	manager := createTestManager()
+
+	options := &instance.Options{
+		BackendOptions: backends.Options{
+			BackendType: backends.BackendTypeLlamaCpp,
+			LlamaServerOptions: &backends.LlamaServerOptions{
+				Model: "/path/to/model.gguf",
+			},
+		},
+	}
+
+	_, err := manager.GetInstance("nonexistent")
 	if err == nil || !strings.Contains(err.Error(), "not found") {
 		t.Errorf("Expected 'not found' error, got: %v", err)
 	}