test: Add tests for allow, ensureContainerRootDiskEntry; remove dead code

- 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%)
This commit is contained in:
rcourtman
2025-12-01 20:15:32 +00:00
parent ccd7851426
commit f30e2ca547
3 changed files with 143 additions and 6 deletions

View File

@@ -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)

View File

@@ -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 {

View File

@@ -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{