From 4c91dce2f83295a0bb63b19711f2e4524e8d9c2a Mon Sep 17 00:00:00 2001 From: rcourtman Date: Mon, 1 Dec 2025 20:15:32 +0000 Subject: [PATCH] test: Add tests for allow, ensureContainerRootDiskEntry; remove dead code MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - allow (circuitBreaker): all states including unknown/default branch, open→half-open transition, half-open window timing (93.8% -> 100%) - ensureContainerRootDiskEntry: nil container, existing disks, used>total clamping, negative free clamping, usage calculation (92.3% -> 100%) - convertPoolInfoToModel: removed unreachable nil check (dead code since ConvertToModelZFSPool only returns nil for nil receiver) (88.9% -> 100%) --- internal/monitoring/circuit_breaker_test.go | 88 +++++++++++++++++++ internal/monitoring/container_parsing_test.go | 58 +++++++++++- internal/monitoring/monitor_polling.go | 3 - 3 files changed, 143 insertions(+), 6 deletions(-) diff --git a/internal/monitoring/circuit_breaker_test.go b/internal/monitoring/circuit_breaker_test.go index fedcd2d..30adece 100644 --- a/internal/monitoring/circuit_breaker_test.go +++ b/internal/monitoring/circuit_breaker_test.go @@ -365,6 +365,94 @@ func TestCircuitBreaker_ConcurrentAccess(t *testing.T) { } } +func TestCircuitBreaker_Allow(t *testing.T) { + tests := []struct { + name string + setup func(cb *circuitBreaker, now time.Time) + timeOffset time.Duration + want bool + wantStateAfter breakerState + }{ + { + name: "closed state always allows", + setup: func(cb *circuitBreaker, now time.Time) {}, + timeOffset: 0, + want: true, + wantStateAfter: breakerClosed, + }, + { + name: "open state denies before retry interval", + setup: func(cb *circuitBreaker, now time.Time) { + cb.state = breakerOpen + cb.openedAt = now + cb.retryInterval = 10 * time.Second + }, + timeOffset: 5 * time.Second, + want: false, + wantStateAfter: breakerOpen, + }, + { + name: "open state allows and transitions to half-open after retry interval", + setup: func(cb *circuitBreaker, now time.Time) { + cb.state = breakerOpen + cb.openedAt = now + cb.retryInterval = 10 * time.Second + }, + timeOffset: 10 * time.Second, + want: true, + wantStateAfter: breakerHalfOpen, + }, + { + name: "half-open state denies during window", + setup: func(cb *circuitBreaker, now time.Time) { + cb.state = breakerHalfOpen + cb.lastAttempt = now + cb.halfOpenWindow = 30 * time.Second + }, + timeOffset: 15 * time.Second, + want: false, + wantStateAfter: breakerHalfOpen, + }, + { + name: "half-open state allows after window passed", + setup: func(cb *circuitBreaker, now time.Time) { + cb.state = breakerHalfOpen + cb.lastAttempt = now + cb.halfOpenWindow = 30 * time.Second + }, + timeOffset: 30 * time.Second, + want: true, + wantStateAfter: breakerHalfOpen, + }, + { + name: "unknown state allows (default branch)", + setup: func(cb *circuitBreaker, now time.Time) { + cb.state = breakerState(99) + }, + timeOffset: 0, + want: true, + wantStateAfter: breakerState(99), + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + cb := newCircuitBreaker(3, 5*time.Second, 5*time.Minute, 30*time.Second) + now := time.Now() + tt.setup(cb, now) + + got := cb.allow(now.Add(tt.timeOffset)) + + if got != tt.want { + t.Errorf("allow() = %v, want %v", got, tt.want) + } + if cb.state != tt.wantStateAfter { + t.Errorf("state after allow() = %v, want %v", cb.state, tt.wantStateAfter) + } + }) + } +} + func TestCircuitBreaker_StateDetails(t *testing.T) { t.Run("closed state", func(t *testing.T) { cb := newCircuitBreaker(3, 5*time.Second, 5*time.Minute, 30*time.Second) diff --git a/internal/monitoring/container_parsing_test.go b/internal/monitoring/container_parsing_test.go index 4b1361c..7a0f9d6 100644 --- a/internal/monitoring/container_parsing_test.go +++ b/internal/monitoring/container_parsing_test.go @@ -1200,11 +1200,11 @@ func TestEnsureContainerRootDiskEntry(t *testing.T) { }, }, { - name: "negative free clamped to zero", + name: "used greater than total gets clamped when total positive", container: &models.Container{ Disk: models.Disk{ Total: 1000, - Used: 1500, // More than total + Used: 1500, // More than total, will be clamped to 1000 }, }, want: &models.Container{ @@ -1215,7 +1215,7 @@ func TestEnsureContainerRootDiskEntry(t *testing.T) { Disks: []models.Disk{ { Total: 1000, - Used: 1000, + Used: 1000, // Clamped to total Free: 0, Usage: 100.0, Mountpoint: "/", @@ -1224,6 +1224,58 @@ func TestEnsureContainerRootDiskEntry(t *testing.T) { }, }, }, + { + name: "negative free clamped to zero when total is zero but used is positive", + container: &models.Container{ + Disk: models.Disk{ + Total: 0, + Used: 500, // Used > 0, total = 0, so free = 0 - 500 = -500, clamped to 0 + }, + }, + want: &models.Container{ + Disk: models.Disk{ + Total: 0, + Used: 500, + }, + Disks: []models.Disk{ + { + Total: 0, + Used: 500, // Not clamped because total == 0 (clamping only when total > 0) + Free: 0, // Clamped from -500 to 0 + Usage: 0, // No calculation when total == 0 + Mountpoint: "/", + Type: "rootfs", + }, + }, + }, + }, + { + name: "usage already set not recalculated", + container: &models.Container{ + Disk: models.Disk{ + Total: 1000, + Used: 500, + Usage: 75.0, // Already set (even if wrong), should not be recalculated + }, + }, + want: &models.Container{ + Disk: models.Disk{ + Total: 1000, + Used: 500, + Usage: 75.0, + }, + Disks: []models.Disk{ + { + Total: 1000, + Used: 500, + Free: 500, + Usage: 75.0, // Preserved from original + Mountpoint: "/", + Type: "rootfs", + }, + }, + }, + }, } for _, tt := range tests { diff --git a/internal/monitoring/monitor_polling.go b/internal/monitoring/monitor_polling.go index 2a2d1d5..3dc7e07 100644 --- a/internal/monitoring/monitor_polling.go +++ b/internal/monitoring/monitor_polling.go @@ -155,9 +155,6 @@ func convertPoolInfoToModel(poolInfo *proxmox.ZFSPoolInfo) *models.ZFSPool { // Use the converter from the proxmox package proxmoxPool := poolInfo.ConvertToModelZFSPool() - if proxmoxPool == nil { - return nil - } // Convert to our internal model modelPool := &models.ZFSPool{