agent: small refactoring and tests for battery package (#1872)

agent/battery/battery.go

@@ -1,4 +1,4 @@
-// Package battery provides functions to check if the system has a battery and to get the battery stats.
+// Package battery provides functions to check if the system has a battery and return the charge state and percentage.
 package battery
 
 const (

agent/battery/battery_darwin.go

@@ -39,15 +39,13 @@ func readMacBatteries() ([]macBattery, error) {
 var HasReadableBattery = sync.OnceValue(func() bool {
 	systemHasBattery := false
 	batteries, err := readMacBatteries()
+	slog.Debug("Batteries", "batteries", batteries, "err", err)
 	for _, bat := range batteries {
 		if bat.MaxCapacity > 0 {
 			systemHasBattery = true
 			break
 		}
 	}
-	if !systemHasBattery {
-		slog.Debug("No battery found", "err", err)
-	}
 	return systemHasBattery
 })
 

agent/battery/battery_linux.go

@@ -14,22 +14,48 @@ import (
 	"github.com/henrygd/beszel/agent/utils"
 )
 
-const sysfsPowerSupply = "/sys/class/power_supply"
+// getBatteryPaths returns the paths of all batteries in /sys/class/power_supply
+var getBatteryPaths func() ([]string, error)
 
-var getBatteryPaths = sync.OnceValues(func() ([]string, error) {
-	entries, err := os.ReadDir(sysfsPowerSupply)
-	if err != nil {
-		return nil, err
-	}
-	var paths []string
-	for _, e := range entries {
-		path := filepath.Join(sysfsPowerSupply, e.Name())
-		if utils.ReadStringFile(filepath.Join(path, "type")) == "Battery" {
-			paths = append(paths, path)
+// HasReadableBattery checks if the system has a battery and returns true if it does.
+var HasReadableBattery func() bool
+
+func init() {
+	resetBatteryState("/sys/class/power_supply")
+}
+
+// resetBatteryState resets the sync.Once functions to a fresh state.
+// Tests call this after swapping sysfsPowerSupply so the new path is picked up.
+func resetBatteryState(sysfsPowerSupplyPath string) {
+	getBatteryPaths = sync.OnceValues(func() ([]string, error) {
+		entries, err := os.ReadDir(sysfsPowerSupplyPath)
+		if err != nil {
+			return nil, err
 		}
-	}
-	return paths, nil
-})
+		var paths []string
+		for _, e := range entries {
+			path := filepath.Join(sysfsPowerSupplyPath, e.Name())
+			if utils.ReadStringFile(filepath.Join(path, "type")) == "Battery" {
+				paths = append(paths, path)
+			}
+		}
+		return paths, nil
+	})
+	HasReadableBattery = sync.OnceValue(func() bool {
+		systemHasBattery := false
+		paths, err := getBatteryPaths()
+		for _, path := range paths {
+			if _, ok := utils.ReadStringFileOK(filepath.Join(path, "capacity")); ok {
+				systemHasBattery = true
+				break
+			}
+		}
+		if !systemHasBattery {
+			slog.Debug("No battery found", "err", err)
+		}
+		return systemHasBattery
+	})
+}
 
 func parseSysfsState(status string) uint8 {
 	switch status {
@@ -48,22 +74,6 @@ func parseSysfsState(status string) uint8 {
 	}
 }
 
-// HasReadableBattery checks if the system has a battery and returns true if it does.
-var HasReadableBattery = sync.OnceValue(func() bool {
-	systemHasBattery := false
-	paths, err := getBatteryPaths()
-	for _, path := range paths {
-		if _, ok := utils.ReadStringFileOK(filepath.Join(path, "capacity")); ok {
-			systemHasBattery = true
-			break
-		}
-	}
-	if !systemHasBattery {
-		slog.Debug("No battery found", "err", err)
-	}
-	return systemHasBattery
-})
-
 // GetBatteryStats returns the current battery percent and charge state.
 // Reads /sys/class/power_supply/*/capacity directly so the kernel-reported
 // value is used, which is always 0-100 and matches what the OS displays.

agent/battery/battery_linux_test.go

@@ -0,0 +1,201 @@
+//go:build testing && linux
+
+package battery
+
+import (
+	"os"
+	"path/filepath"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+)
+
+// setupFakeSysfs creates a temporary sysfs-like tree under t.TempDir(),
+// swaps sysfsPowerSupply, resets the sync.Once caches, and restores
+// everything on cleanup. Returns a helper to create battery directories.
+func setupFakeSysfs(t *testing.T) (tmpDir string, addBattery func(name, capacity, status string)) {
+	t.Helper()
+
+	tmp := t.TempDir()
+	resetBatteryState(tmp)
+
+	write := func(path, content string) {
+		t.Helper()
+		dir := filepath.Dir(path)
+		if err := os.MkdirAll(dir, 0o755); err != nil {
+			t.Fatal(err)
+		}
+		if err := os.WriteFile(path, []byte(content), 0o644); err != nil {
+			t.Fatal(err)
+		}
+	}
+
+	addBattery = func(name, capacity, status string) {
+		t.Helper()
+		batDir := filepath.Join(tmp, name)
+		write(filepath.Join(batDir, "type"), "Battery")
+		write(filepath.Join(batDir, "capacity"), capacity)
+		write(filepath.Join(batDir, "status"), status)
+	}
+
+	return tmp, addBattery
+}
+
+func TestParseSysfsState(t *testing.T) {
+	tests := []struct {
+		input string
+		want  uint8
+	}{
+		{"Empty", stateEmpty},
+		{"Full", stateFull},
+		{"Charging", stateCharging},
+		{"Discharging", stateDischarging},
+		{"Not charging", stateIdle},
+		{"", stateUnknown},
+		{"SomethingElse", stateUnknown},
+	}
+	for _, tt := range tests {
+		assert.Equal(t, tt.want, parseSysfsState(tt.input), "parseSysfsState(%q)", tt.input)
+	}
+}
+
+func TestGetBatteryStats_SingleBattery(t *testing.T) {
+	_, addBattery := setupFakeSysfs(t)
+	addBattery("BAT0", "72", "Discharging")
+
+	pct, state, err := GetBatteryStats()
+	assert.NoError(t, err)
+	assert.Equal(t, uint8(72), pct)
+	assert.Equal(t, stateDischarging, state)
+}
+
+func TestGetBatteryStats_MultipleBatteries(t *testing.T) {
+	_, addBattery := setupFakeSysfs(t)
+	addBattery("BAT0", "80", "Charging")
+	addBattery("BAT1", "40", "Charging")
+
+	pct, state, err := GetBatteryStats()
+	assert.NoError(t, err)
+	// average of 80 and 40 = 60
+	assert.EqualValues(t, 60, pct)
+	assert.Equal(t, stateCharging, state)
+}
+
+func TestGetBatteryStats_FullBattery(t *testing.T) {
+	_, addBattery := setupFakeSysfs(t)
+	addBattery("BAT0", "100", "Full")
+
+	pct, state, err := GetBatteryStats()
+	assert.NoError(t, err)
+	assert.Equal(t, uint8(100), pct)
+	assert.Equal(t, stateFull, state)
+}
+
+func TestGetBatteryStats_EmptyBattery(t *testing.T) {
+	_, addBattery := setupFakeSysfs(t)
+	addBattery("BAT0", "0", "Empty")
+
+	pct, state, err := GetBatteryStats()
+	assert.NoError(t, err)
+	assert.Equal(t, uint8(0), pct)
+	assert.Equal(t, stateEmpty, state)
+}
+
+func TestGetBatteryStats_NotCharging(t *testing.T) {
+	_, addBattery := setupFakeSysfs(t)
+	addBattery("BAT0", "80", "Not charging")
+
+	pct, state, err := GetBatteryStats()
+	assert.NoError(t, err)
+	assert.Equal(t, uint8(80), pct)
+	assert.Equal(t, stateIdle, state)
+}
+
+func TestGetBatteryStats_NoBatteries(t *testing.T) {
+	setupFakeSysfs(t) // empty directory, no batteries
+
+	_, _, err := GetBatteryStats()
+	assert.Error(t, err)
+}
+
+func TestGetBatteryStats_NonBatterySupplyIgnored(t *testing.T) {
+	tmp, addBattery := setupFakeSysfs(t)
+
+	// Add a real battery
+	addBattery("BAT0", "55", "Charging")
+
+	// Add an AC adapter (type != Battery) - should be ignored
+	acDir := filepath.Join(tmp, "AC0")
+	if err := os.MkdirAll(acDir, 0o755); err != nil {
+		t.Fatal(err)
+	}
+	if err := os.WriteFile(filepath.Join(acDir, "type"), []byte("Mains"), 0o644); err != nil {
+		t.Fatal(err)
+	}
+
+	pct, state, err := GetBatteryStats()
+	assert.NoError(t, err)
+	assert.Equal(t, uint8(55), pct)
+	assert.Equal(t, stateCharging, state)
+}
+
+func TestGetBatteryStats_InvalidCapacitySkipped(t *testing.T) {
+	tmp, addBattery := setupFakeSysfs(t)
+
+	// One battery with valid capacity
+	addBattery("BAT0", "90", "Discharging")
+
+	// Another with invalid capacity text
+	badDir := filepath.Join(tmp, "BAT1")
+	if err := os.MkdirAll(badDir, 0o755); err != nil {
+		t.Fatal(err)
+	}
+	if err := os.WriteFile(filepath.Join(badDir, "type"), []byte("Battery"), 0o644); err != nil {
+		t.Fatal(err)
+	}
+	if err := os.WriteFile(filepath.Join(badDir, "capacity"), []byte("not-a-number"), 0o644); err != nil {
+		t.Fatal(err)
+	}
+	if err := os.WriteFile(filepath.Join(badDir, "status"), []byte("Discharging"), 0o644); err != nil {
+		t.Fatal(err)
+	}
+
+	pct, _, err := GetBatteryStats()
+	assert.NoError(t, err)
+	// Only BAT0 counted
+	assert.Equal(t, uint8(90), pct)
+}
+
+func TestGetBatteryStats_UnknownStatusOnly(t *testing.T) {
+	_, addBattery := setupFakeSysfs(t)
+	addBattery("BAT0", "50", "SomethingWeird")
+
+	_, _, err := GetBatteryStats()
+	assert.Error(t, err)
+}
+
+func TestHasReadableBattery_True(t *testing.T) {
+	_, addBattery := setupFakeSysfs(t)
+	addBattery("BAT0", "50", "Charging")
+
+	assert.True(t, HasReadableBattery())
+}
+
+func TestHasReadableBattery_False(t *testing.T) {
+	setupFakeSysfs(t) // no batteries
+
+	assert.False(t, HasReadableBattery())
+}
+
+func TestHasReadableBattery_NoCapacityFile(t *testing.T) {
+	tmp, _ := setupFakeSysfs(t)
+
+	// Battery dir with type file but no capacity file
+	batDir := filepath.Join(tmp, "BAT0")
+	err := os.MkdirAll(batDir, 0o755)
+	assert.NoError(t, err)
+	err = os.WriteFile(filepath.Join(batDir, "type"), []byte("Battery"), 0o644)
+	assert.NoError(t, err)
+
+	assert.False(t, HasReadableBattery())
+}

agent/battery/battery_windows.go

@@ -1,5 +1,8 @@
 //go:build windows
 
+// Most of the Windows battery code is based on
+// distatus/battery by Karol 'Kenji Takahashi' Woźniak
+
 package battery
 
 import (
@@ -76,6 +79,10 @@ var (
 	setupDiDestroyDeviceInfoList     = setupapi.NewProc("SetupDiDestroyDeviceInfoList")
 )
 
+// winBatteryGet reads one battery by index. Returns (fullCapacity, currentCapacity, state, error).
+// Returns error == errNotFound when there are no more batteries.
+var errNotFound = errors.New("no more batteries")
+
 func setupDiSetup(proc *windows.LazyProc, nargs, a1, a2, a3, a4, a5, a6 uintptr) (uintptr, error) {
 	_ = nargs
 	r1, _, errno := syscall.SyscallN(proc.Addr(), a1, a2, a3, a4, a5, a6)
@@ -115,10 +122,6 @@ func readWinBatteryState(powerState uint32) uint8 {
 	}
 }
 
-// winBatteryGet reads one battery by index. Returns (fullCapacity, currentCapacity, state, error).
-// Returns error == errNotFound when there are no more batteries.
-var errNotFound = errors.New("no more batteries")
-
 func winBatteryGet(idx int) (full, current uint32, state uint8, err error) {
 	hdev, err := setupDiSetup(
 		setupDiGetClassDevsW,