Skip to content

Commit 1d39bf8

Browse files
authored
[test-improver] Improve tests for server hasServerGuardPolicies (#3273)
## Test Improvements: `has_server_guard_policies_test.go` ## File Analyzed - **Test File**: `internal/server/has_server_guard_policies_test.go` - **Package**: `internal/server` - **Lines of Code**: 65 → 126 (added 5 new test cases) ## Improvements Made ### 1. Increased Coverage The function `hasServerGuardPolicies` iterates `cfg.Servers` and returns `true` on the first server that has a non-empty `GuardPolicies` map, or `false` if none do. The original 3 tests only verified the single-server happy path. New test cases exercise the multi-server iteration logic and boundary conditions: - ✅ Added: **Empty Servers map** (no servers configured → `false`) - ✅ Added: **Multiple servers all without guard policies** (full iteration, all misses → `false`) - ✅ Added: **Multiple servers where only one has guard policies** (early return on match → `true`) - ✅ Added: **Multiple servers all with guard policies** (immediate early return → `true`) - ✅ Added: **Mix of servers with/without empty guard-policies maps** (confirms `{}` counts as absent → `false`) ### 2. Cleaner Test Structure - ✅ Extracted shared `allowOnlyPolicy` fixture to reduce duplication across test cases - ✅ Renamed test cases to follow the `<scenario> returns <result>` naming convention, making failures self-documenting - ✅ Removed redundant custom failure message from `assert.Equal` (testify already provides clear diffs) ## Why These Changes? `hasServerGuardPolicies` is used during DIFC auto-detection to decide whether guard enforcement should be enabled. The original 3 tests only covered single-server scenarios, leaving the multi-server iteration and early-return paths entirely untested. Since map iteration order in Go is non-deterministic, the multi-server tests confirm the function is correct regardless of which server is visited first. --- *Generated by Test Improver Workflow* *Focuses on better patterns, increased coverage, and more stable tests* > Generated by [Test Improver](https://github.com/github/gh-aw-mcpg/actions/runs/24030278829/agentic_workflow) · ● 4.4M · [◷](https://github.com/search?q=repo%3Agithub%2Fgh-aw-mcpg+%22gh-aw-workflow-id%3A+test-improver%22&type=pullrequests) <!-- gh-aw-agentic-workflow: Test Improver, engine: copilot, model: auto, id: 24030278829, workflow_id: test-improver, run: https://github.com/github/gh-aw-mcpg/actions/runs/24030278829 --> <!-- gh-aw-workflow-id: test-improver -->
2 parents 9b9bf77 + f95a1ba commit 1d39bf8

File tree

1 file changed

+73
-11
lines changed

1 file changed

+73
-11
lines changed

internal/server/has_server_guard_policies_test.go

Lines changed: 73 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -8,30 +8,32 @@ import (
88
)
99

1010
func TestHasServerGuardPolicies(t *testing.T) {
11+
allowOnlyPolicy := map[string]interface{}{
12+
"allow-only": map[string]interface{}{
13+
"min-integrity": "approved",
14+
"repos": []interface{}{"github/gh-aw*"},
15+
},
16+
}
17+
1118
tests := []struct {
1219
name string
1320
cfg *config.Config
1421
expected bool
1522
}{
1623
{
17-
name: "server with guard-policies should return true",
24+
name: "single server with guard-policies returns true",
1825
cfg: &config.Config{
1926
Servers: map[string]*config.ServerConfig{
2027
"github": {
21-
Type: "stdio",
22-
GuardPolicies: map[string]interface{}{
23-
"allow-only": map[string]interface{}{
24-
"min-integrity": "approved",
25-
"repos": []interface{}{"github/gh-aw*"},
26-
},
27-
},
28+
Type: "stdio",
29+
GuardPolicies: allowOnlyPolicy,
2830
},
2931
},
3032
},
3133
expected: true,
3234
},
3335
{
34-
name: "server without guard-policies should return false",
36+
name: "single server without guard-policies returns false",
3537
cfg: &config.Config{
3638
Servers: map[string]*config.ServerConfig{
3739
"github": {
@@ -42,10 +44,70 @@ func TestHasServerGuardPolicies(t *testing.T) {
4244
expected: false,
4345
},
4446
{
45-
name: "server with empty guard-policies should return false",
47+
name: "single server with empty guard-policies map returns false",
48+
cfg: &config.Config{
49+
Servers: map[string]*config.ServerConfig{
50+
"github": {
51+
Type: "stdio",
52+
GuardPolicies: map[string]interface{}{},
53+
},
54+
},
55+
},
56+
expected: false,
57+
},
58+
{
59+
name: "no servers returns false",
60+
cfg: &config.Config{
61+
Servers: map[string]*config.ServerConfig{},
62+
},
63+
expected: false,
64+
},
65+
{
66+
name: "multiple servers all without guard-policies returns false",
67+
cfg: &config.Config{
68+
Servers: map[string]*config.ServerConfig{
69+
"github": {Type: "stdio"},
70+
"slack": {Type: "stdio"},
71+
"jira": {Type: "stdio"},
72+
},
73+
},
74+
expected: false,
75+
},
76+
{
77+
name: "multiple servers where one has guard-policies returns true",
78+
cfg: &config.Config{
79+
Servers: map[string]*config.ServerConfig{
80+
"github": {Type: "stdio"},
81+
"slack": {
82+
Type: "stdio",
83+
GuardPolicies: allowOnlyPolicy,
84+
},
85+
},
86+
},
87+
expected: true,
88+
},
89+
{
90+
name: "multiple servers all with guard-policies returns true",
4691
cfg: &config.Config{
4792
Servers: map[string]*config.ServerConfig{
4893
"github": {
94+
Type: "stdio",
95+
GuardPolicies: allowOnlyPolicy,
96+
},
97+
"slack": {
98+
Type: "stdio",
99+
GuardPolicies: allowOnlyPolicy,
100+
},
101+
},
102+
},
103+
expected: true,
104+
},
105+
{
106+
name: "mix of servers with and without empty guard-policies returns false",
107+
cfg: &config.Config{
108+
Servers: map[string]*config.ServerConfig{
109+
"github": {Type: "stdio"},
110+
"slack": {
49111
Type: "stdio",
50112
GuardPolicies: map[string]interface{}{},
51113
},
@@ -58,7 +120,7 @@ func TestHasServerGuardPolicies(t *testing.T) {
58120
for _, tt := range tests {
59121
t.Run(tt.name, func(t *testing.T) {
60122
result := hasServerGuardPolicies(tt.cfg)
61-
assert.Equal(t, tt.expected, result, "hasServerGuardPolicies should return %v", tt.expected)
123+
assert.Equal(t, tt.expected, result)
62124
})
63125
}
64126
}

0 commit comments

Comments
 (0)