Skip to content

Commit 1699231

Browse files
authored
[test-improver] Improve tests for guard Registry.HasNonNoopGuard (#3158)
# Test Improvements: `internal/guard/guard_test.go` ## File Analyzed - **Test File**: `internal/guard/guard_test.go` - **Package**: `internal/guard` - **Lines of Code**: 802 → 847 (+45 lines) ## Improvements Made ### 1. Increased Coverage — Direct Tests for `Registry.HasNonNoopGuard` **Problem**: The `HasNonNoopGuard` method in `internal/guard/registry.go` was called from `internal/server/unified.go` to determine whether DIFC should be auto-enabled: ```go // server/unified.go line 170 if !us.enableDIFC && (us.guardRegistry.HasNonNoopGuard() || ...) { ``` Despite being a meaningful predicate used in server startup logic, `HasNonNoopGuard` had **no dedicated unit tests** in `guard_test.go`. All other `Registry` methods (`Register`, `Get`, `Has`, `List`, `Remove`, `GetGuardInfo`) had explicit subtests — `HasNonNoopGuard` was the only gap. ### 2. New Test: `TestGuardRegistry_HasNonNoopGuard` Added 6 subtests covering all branches of the `HasNonNoopGuard` method: - ✅ **empty registry returns false** — verifies base case - ✅ **only noop guards returns false** — verifies multiple noop guards don't trigger true - ✅ **one non-noop guard returns true** — verifies detection of a single non-noop guard - ✅ **mix of noop and non-noop returns true** — verifies any non-noop triggers true - ✅ **removing non-noop guard makes it return false** — verifies dynamic removal - ✅ **replacing non-noop with noop returns false** — verifies guard replacement The tests reuse the existing `mockGuard` helper and follow the established subtest patterns throughout the file (consistent with `TestGuardRegistry`). ### 3. Cleaner & More Stable Tests - ✅ Each subtest uses its own fresh `NewRegistry()` — fully isolated, no shared state - ✅ Uses `assert.True`/`assert.False` for clear, readable assertions - ✅ Tests the state _before_ and _after_ mutations (remove/replace) to validate dynamic behaviour - ✅ Follows the existing file style exactly — no new imports needed ## Why These Changes? `HasNonNoopGuard` is the sentinel used by the server to auto-activate DIFC when a non-trivial guard is registered. A regression in this predicate (e.g., always returning `false`) would silently disable DIFC enforcement at startup — a security-relevant failure. Despite the rest of the Registry API being well-tested, this method was the only gap. --- *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/23977968757/agentic_workflow) · [◷](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: 23977968757, workflow_id: test-improver, run: https://github.com/github/gh-aw-mcpg/actions/runs/23977968757 --> <!-- gh-aw-workflow-id: test-improver -->
2 parents e8c466f + 57f313a commit 1699231

1 file changed

Lines changed: 45 additions & 0 deletions

File tree

internal/guard/guard_test.go

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -405,6 +405,51 @@ func TestGuardRegistryConcurrency(t *testing.T) {
405405
})
406406
}
407407

408+
func TestGuardRegistry_HasNonNoopGuard(t *testing.T) {
409+
t.Run("empty registry returns false", func(t *testing.T) {
410+
registry := NewRegistry()
411+
assert.False(t, registry.HasNonNoopGuard())
412+
})
413+
414+
t.Run("only noop guards returns false", func(t *testing.T) {
415+
registry := NewRegistry()
416+
registry.Register("server1", NewNoopGuard())
417+
registry.Register("server2", NewNoopGuard())
418+
assert.False(t, registry.HasNonNoopGuard())
419+
})
420+
421+
t.Run("one non-noop guard returns true", func(t *testing.T) {
422+
registry := NewRegistry()
423+
registry.Register("server1", &mockGuard{id: "wasm"})
424+
assert.True(t, registry.HasNonNoopGuard())
425+
})
426+
427+
t.Run("mix of noop and non-noop returns true", func(t *testing.T) {
428+
registry := NewRegistry()
429+
registry.Register("server1", NewNoopGuard())
430+
registry.Register("server2", &mockGuard{id: "github"})
431+
assert.True(t, registry.HasNonNoopGuard())
432+
})
433+
434+
t.Run("removing non-noop guard makes it return false", func(t *testing.T) {
435+
registry := NewRegistry()
436+
registry.Register("server1", &mockGuard{id: "wasm"})
437+
assert.True(t, registry.HasNonNoopGuard())
438+
439+
registry.Remove("server1")
440+
assert.False(t, registry.HasNonNoopGuard())
441+
})
442+
443+
t.Run("replacing non-noop with noop returns false", func(t *testing.T) {
444+
registry := NewRegistry()
445+
registry.Register("server1", &mockGuard{id: "wasm"})
446+
assert.True(t, registry.HasNonNoopGuard())
447+
448+
registry.Register("server1", NewNoopGuard())
449+
assert.False(t, registry.HasNonNoopGuard())
450+
})
451+
}
452+
408453
func TestCreateGuard(t *testing.T) {
409454
tests := []struct {
410455
name string

0 commit comments

Comments
 (0)