Skip to content

Commit 4b7bdd6

Browse files
lyarwoodlyarwood
committed
refactor(kubevirt): Move tests to external packages and test public API
Refactor all kubevirt VM tool tests to use external test packages (_test suffix) and test only public behavior through the Tools() API. This ensures tests verify the public interface rather than implementation details. Assisted-By: Claude <[email protected]> Signed-off-by: Lee Yarwood <[email protected]>
1 parent c41a184 commit 4b7bdd6

File tree

4 files changed

+107
-176
lines changed

4 files changed

+107
-176
lines changed

‎pkg/toolsets/kubevirt/vm/create/tool_test.go‎

Lines changed: 71 additions & 167 deletions
Original file line numberDiff line numberDiff line change
@@ -1,197 +1,101 @@
1-
package create
1+
package create_test
22

33
import (
4-
"strings"
4+
"context"
55
"testing"
6+
7+
"github.com/containers/kubernetes-mcp-server/pkg/api"
8+
internalk8s "github.com/containers/kubernetes-mcp-server/pkg/kubernetes"
9+
"github.com/containers/kubernetes-mcp-server/pkg/toolsets/kubevirt/vm/create"
610
)
711

8-
// Test the YAML rendering directly without creating resources
9-
func TestRenderVMYaml(t *testing.T) {
12+
type mockToolCallRequest struct {
13+
arguments map[string]interface{}
14+
}
15+
16+
func (m *mockToolCallRequest) GetArguments() map[string]any {
17+
return m.arguments
18+
}
19+
20+
func TestCreateParameterValidation(t *testing.T) {
1021
tests := []struct {
11-
name string
12-
params vmParams
13-
wantErr bool
14-
checkFunc func(t *testing.T, result string)
22+
name string
23+
args map[string]interface{}
24+
wantErr bool
25+
errMsg string
1526
}{
1627
{
17-
name: "renders VM with basic settings",
18-
params: vmParams{
19-
Namespace: "test-ns",
20-
Name: "test-vm",
21-
ContainerDisk: "quay.io/containerdisks/fedora:latest",
22-
RunStrategy: "Halted",
23-
},
24-
wantErr: false,
25-
checkFunc: func(t *testing.T, result string) {
26-
if !strings.Contains(result, "apiVersion: kubevirt.io/v1") {
27-
t.Errorf("Expected apiVersion in YAML")
28-
}
29-
if !strings.Contains(result, "kind: VirtualMachine") {
30-
t.Errorf("Expected kind VirtualMachine in YAML")
31-
}
32-
if !strings.Contains(result, "name: test-vm") {
33-
t.Errorf("Expected VM name test-vm in YAML")
34-
}
35-
if !strings.Contains(result, "namespace: test-ns") {
36-
t.Errorf("Expected namespace test-ns in YAML")
37-
}
38-
if !strings.Contains(result, "quay.io/containerdisks/fedora:latest") {
39-
t.Errorf("Expected fedora container disk in result")
40-
}
41-
if !strings.Contains(result, "guest: 2Gi") {
42-
t.Errorf("Expected guest: 2Gi in YAML manifest")
43-
}
44-
},
45-
},
46-
{
47-
name: "renders VM with instancetype",
48-
params: vmParams{
49-
Namespace: "test-ns",
50-
Name: "test-vm",
51-
ContainerDisk: "quay.io/containerdisks/ubuntu:24.04",
52-
Instancetype: "u1.medium",
53-
RunStrategy: "Halted",
54-
},
55-
wantErr: false,
56-
checkFunc: func(t *testing.T, result string) {
57-
if !strings.Contains(result, "name: u1.medium") {
58-
t.Errorf("Expected instance type in YAML manifest")
59-
}
60-
if !strings.Contains(result, "kind: VirtualMachineClusterInstancetype") {
61-
t.Errorf("Expected VirtualMachineClusterInstancetype in YAML manifest")
62-
}
63-
// When instancetype is set, memory should not be in the YAML
64-
if strings.Contains(result, "guest: 2Gi") {
65-
t.Errorf("Should not have guest memory when instancetype is specified")
66-
}
28+
name: "missing namespace parameter",
29+
args: map[string]interface{}{
30+
"name": "test-vm",
6731
},
32+
wantErr: true,
33+
errMsg: "namespace parameter required",
6834
},
6935
{
70-
name: "renders VM with preference",
71-
params: vmParams{
72-
Namespace: "test-ns",
73-
Name: "test-vm",
74-
ContainerDisk: "registry.redhat.io/rhel9/rhel-guest-image:latest",
75-
Preference: "rhel.9",
76-
RunStrategy: "Halted",
77-
},
78-
wantErr: false,
79-
checkFunc: func(t *testing.T, result string) {
80-
if !strings.Contains(result, "name: rhel.9") {
81-
t.Errorf("Expected preference in YAML manifest")
82-
}
83-
if !strings.Contains(result, "kind: VirtualMachineClusterPreference") {
84-
t.Errorf("Expected VirtualMachineClusterPreference in YAML manifest")
85-
}
86-
},
87-
},
88-
{
89-
name: "renders VM with custom container disk",
90-
params: vmParams{
91-
Namespace: "test-ns",
92-
Name: "test-vm",
93-
ContainerDisk: "quay.io/myrepo/myimage:v1.0",
94-
RunStrategy: "Halted",
95-
},
96-
wantErr: false,
97-
checkFunc: func(t *testing.T, result string) {
98-
if !strings.Contains(result, "quay.io/myrepo/myimage:v1.0") {
99-
t.Errorf("Expected custom container disk in YAML")
100-
}
36+
name: "missing name parameter",
37+
args: map[string]interface{}{
38+
"namespace": "test-ns",
10139
},
40+
wantErr: true,
41+
errMsg: "name parameter required",
10242
},
10343
{
104-
name: "renders VM with DataSource",
105-
params: vmParams{
106-
Namespace: "test-ns",
107-
Name: "test-vm",
108-
UseDataSource: true,
109-
DataSourceName: "fedora",
110-
DataSourceNamespace: "openshift-virtualization-os-images",
111-
RunStrategy: "Halted",
112-
},
113-
wantErr: false,
114-
checkFunc: func(t *testing.T, result string) {
115-
if !strings.Contains(result, "dataVolumeTemplates") {
116-
t.Errorf("Expected dataVolumeTemplates in YAML")
117-
}
118-
if !strings.Contains(result, "kind: DataSource") {
119-
t.Errorf("Expected DataSource kind in YAML")
120-
}
121-
if !strings.Contains(result, "name: fedora") {
122-
t.Errorf("Expected DataSource name in YAML")
123-
}
124-
if !strings.Contains(result, "openshift-virtualization-os-images") {
125-
t.Errorf("Expected DataSource namespace in YAML")
126-
}
44+
name: "invalid namespace type",
45+
args: map[string]interface{}{
46+
"namespace": 123,
47+
"name": "test-vm",
12748
},
49+
wantErr: true,
50+
errMsg: "namespace parameter must be a string",
12851
},
12952
{
130-
name: "renders VM with autostart (runStrategy Always)",
131-
params: vmParams{
132-
Namespace: "test-ns",
133-
Name: "test-vm",
134-
ContainerDisk: "quay.io/containerdisks/fedora:latest",
135-
RunStrategy: "Always",
136-
},
137-
wantErr: false,
138-
checkFunc: func(t *testing.T, result string) {
139-
if !strings.Contains(result, "runStrategy: Always") {
140-
t.Errorf("Expected runStrategy: Always in YAML")
141-
}
53+
name: "invalid name type",
54+
args: map[string]interface{}{
55+
"namespace": "test-ns",
56+
"name": 456,
14257
},
58+
wantErr: true,
59+
errMsg: "name parameter must be a string",
14360
},
14461
}
14562

63+
// Get the tool through the public API
64+
tools := create.Tools()
65+
if len(tools) != 1 {
66+
t.Fatalf("Expected 1 tool, got %d", len(tools))
67+
}
68+
vmCreateTool := tools[0]
69+
14670
for _, tt := range tests {
14771
t.Run(tt.name, func(t *testing.T) {
148-
result, err := renderVMYaml(tt.params)
72+
params := api.ToolHandlerParams{
73+
Context: context.Background(),
74+
Kubernetes: &internalk8s.Kubernetes{},
75+
ToolCallRequest: &mockToolCallRequest{arguments: tt.args},
76+
}
14977

150-
if tt.wantErr {
151-
if err == nil {
152-
t.Error("Expected error, got nil")
153-
}
154-
} else {
155-
if err != nil {
156-
t.Errorf("Expected no error, got: %v", err)
157-
}
158-
if result == "" {
159-
t.Error("Expected non-empty result")
160-
}
161-
if tt.checkFunc != nil {
162-
tt.checkFunc(t, result)
163-
}
78+
// Call through the public Handler interface
79+
result, err := vmCreateTool.Handler(params)
80+
if err != nil {
81+
t.Errorf("Handler() unexpected Go error: %v", err)
82+
return
16483
}
165-
})
166-
}
167-
}
16884

169-
func TestResolveContainerDisk(t *testing.T) {
170-
tests := []struct {
171-
name string
172-
input string
173-
expected string
174-
}{
175-
{"fedora", "fedora", "quay.io/containerdisks/fedora:latest"},
176-
{"ubuntu", "ubuntu", "quay.io/containerdisks/ubuntu:24.04"},
177-
{"rhel8", "rhel8", "registry.redhat.io/rhel8/rhel-guest-image:latest"},
178-
{"rhel9", "rhel9", "registry.redhat.io/rhel9/rhel-guest-image:latest"},
179-
{"rhel10", "rhel10", "registry.redhat.io/rhel10/rhel-guest-image:latest"},
180-
{"centos", "centos", "quay.io/containerdisks/centos-stream:9-latest"},
181-
{"centos-stream", "centos-stream", "quay.io/containerdisks/centos-stream:9-latest"},
182-
{"debian", "debian", "quay.io/containerdisks/debian:latest"},
183-
{"case insensitive", "FEDORA", "quay.io/containerdisks/fedora:latest"},
184-
{"with whitespace", " ubuntu ", "quay.io/containerdisks/ubuntu:24.04"},
185-
{"custom image", "quay.io/myrepo/myimage:v1", "quay.io/myrepo/myimage:v1"},
186-
{"with tag", "myimage:latest", "myimage:latest"},
187-
{"unknown OS", "customos", "customos"},
188-
}
85+
if result == nil {
86+
t.Error("Expected non-nil result")
87+
return
88+
}
18989

190-
for _, tt := range tests {
191-
t.Run(tt.name, func(t *testing.T) {
192-
result := resolveContainerDisk(tt.input)
193-
if result != tt.expected {
194-
t.Errorf("resolveContainerDisk(%s) = %s, want %s", tt.input, result, tt.expected)
90+
// For parameter validation errors, check the error message
91+
if tt.wantErr && tt.errMsg != "" {
92+
if result.Error == nil {
93+
t.Error("Expected error in result.Error, got nil")
94+
return
95+
}
96+
if result.Error.Error() != tt.errMsg {
97+
t.Errorf("Expected error message %q, got %q", tt.errMsg, result.Error.Error())
98+
}
19599
}
196100
})
197101
}

‎pkg/toolsets/kubevirt/vm/start/tool_test.go‎

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,12 @@
1-
package start
1+
package start_test
22

33
import (
44
"context"
55
"testing"
66

77
"github.com/containers/kubernetes-mcp-server/pkg/api"
88
internalk8s "github.com/containers/kubernetes-mcp-server/pkg/kubernetes"
9+
"github.com/containers/kubernetes-mcp-server/pkg/toolsets/kubevirt/vm/start"
910
)
1011

1112
type mockToolCallRequest struct {
@@ -67,6 +68,13 @@ func TestStartParameterValidation(t *testing.T) {
6768
},
6869
}
6970

71+
// Get the tool through the public API
72+
tools := start.Tools()
73+
if len(tools) != 1 {
74+
t.Fatalf("Expected 1 tool, got %d", len(tools))
75+
}
76+
vmStartTool := tools[0]
77+
7078
for _, tt := range tests {
7179
t.Run(tt.name, func(t *testing.T) {
7280
params := api.ToolHandlerParams{
@@ -75,9 +83,10 @@ func TestStartParameterValidation(t *testing.T) {
7583
ToolCallRequest: &mockToolCallRequest{arguments: tt.args},
7684
}
7785

78-
result, err := start(params)
86+
// Call through the public Handler interface
87+
result, err := vmStartTool.Handler(params)
7988
if err != nil {
80-
t.Errorf("start() unexpected Go error: %v", err)
89+
t.Errorf("Handler() unexpected Go error: %v", err)
8190
return
8291
}
8392

‎pkg/toolsets/kubevirt/vm/stop/tool_test.go‎

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,12 @@
1-
package stop
1+
package stop_test
22

33
import (
44
"context"
55
"testing"
66

77
"github.com/containers/kubernetes-mcp-server/pkg/api"
88
internalk8s "github.com/containers/kubernetes-mcp-server/pkg/kubernetes"
9+
"github.com/containers/kubernetes-mcp-server/pkg/toolsets/kubevirt/vm/stop"
910
)
1011

1112
type mockToolCallRequest struct {
@@ -67,6 +68,13 @@ func TestStopParameterValidation(t *testing.T) {
6768
},
6869
}
6970

71+
// Get the tool through the public API
72+
tools := stop.Tools()
73+
if len(tools) != 1 {
74+
t.Fatalf("Expected 1 tool, got %d", len(tools))
75+
}
76+
vmStopTool := tools[0]
77+
7078
for _, tt := range tests {
7179
t.Run(tt.name, func(t *testing.T) {
7280
params := api.ToolHandlerParams{
@@ -75,9 +83,10 @@ func TestStopParameterValidation(t *testing.T) {
7583
ToolCallRequest: &mockToolCallRequest{arguments: tt.args},
7684
}
7785

78-
result, err := stop(params)
86+
// Call through the public Handler interface
87+
result, err := vmStopTool.Handler(params)
7988
if err != nil {
80-
t.Errorf("stop() unexpected Go error: %v", err)
89+
t.Errorf("Handler() unexpected Go error: %v", err)
8190
return
8291
}
8392

‎pkg/toolsets/kubevirt/vm/troubleshoot/tool_test.go‎

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
package troubleshoot
1+
package troubleshoot_test
22

33
import (
44
"context"
@@ -7,6 +7,7 @@ import (
77

88
"github.com/containers/kubernetes-mcp-server/pkg/api"
99
internalk8s "github.com/containers/kubernetes-mcp-server/pkg/kubernetes"
10+
"github.com/containers/kubernetes-mcp-server/pkg/toolsets/kubevirt/vm/troubleshoot"
1011
)
1112

1213
type mockToolCallRequest struct {
@@ -71,6 +72,13 @@ func TestTroubleshoot(t *testing.T) {
7172
},
7273
}
7374

75+
// Get the tool through the public API
76+
tools := troubleshoot.Tools()
77+
if len(tools) != 1 {
78+
t.Fatalf("Expected 1 tool, got %d", len(tools))
79+
}
80+
vmTroubleshootTool := tools[0]
81+
7482
for _, tt := range tests {
7583
t.Run(tt.name, func(t *testing.T) {
7684
params := api.ToolHandlerParams{
@@ -79,9 +87,10 @@ func TestTroubleshoot(t *testing.T) {
7987
ToolCallRequest: &mockToolCallRequest{arguments: tt.args},
8088
}
8189

82-
result, err := troubleshoot(params)
90+
// Call through the public Handler interface
91+
result, err := vmTroubleshootTool.Handler(params)
8392
if err != nil {
84-
t.Errorf("troubleshoot() unexpected Go error: %v", err)
93+
t.Errorf("Handler() unexpected Go error: %v", err)
8594
return
8695
}
8796

0 commit comments

Comments
 (0)