test: improve unit-testing

Author: cedi

cmd/bladectl/cmd_status.go

@@ -5,7 +5,6 @@ import (
 
 	bladeapiv1alpha1 "github.com/compute-blade-community/compute-blade-agent/api/bladeapi/v1alpha1"
 	"github.com/compute-blade-community/compute-blade-agent/pkg/hal"
-	"github.com/compute-blade-community/compute-blade-agent/pkg/util"
 	"github.com/olekukonko/tablewriter"
 	"github.com/olekukonko/tablewriter/tw"
 	"github.com/spf13/cobra"
@@ -68,7 +67,7 @@ func printStatusTable(bladeStatus []*bladeapiv1alpha1.StatusResponse) {
 			activeStyle(status.StealthMode).Render(activeLabel(status.StealthMode)),
 			activeStyle(status.IdentifyActive).Render(activeLabel(status.IdentifyActive)),
 			activeStyle(status.CriticalActive).Render(activeLabel(status.CriticalActive)),
-			util.OkStyle().Render(hal.PowerStatus(status.PowerStatus).String()),
+			okStyle().Render(hal.PowerStatus(status.PowerStatus).String()),
 		}
 
 		_ = tbl.Append(row)

cmd/bladectl/util.go

@@ -4,7 +4,12 @@ import (
 	"fmt"
 
 	"github.com/charmbracelet/lipgloss"
-	"github.com/compute-blade-community/compute-blade-agent/pkg/util"
+)
+
+const (
+	ColorCritical = lipgloss.Color("#cc0000")
+	ColorWarning  = lipgloss.Color("#e69138")
+	ColorOk       = lipgloss.Color("#04B575")
 )
 
 func fanSpeedOverrideLabel(automatic bool, percent uint32) string {
@@ -35,40 +40,44 @@ func activeLabel(b bool) string {
 
 func speedOverrideStyle(automaticMode bool) lipgloss.Style {
 	if automaticMode {
-		return lipgloss.NewStyle().Foreground(util.ColorOk)
+		return lipgloss.NewStyle().Foreground(ColorOk)
 	}
 
-	return lipgloss.NewStyle().Foreground(util.ColorCritical)
+	return lipgloss.NewStyle().Foreground(ColorCritical)
 }
 
 func activeStyle(active bool) lipgloss.Style {
 	if active {
-		return lipgloss.NewStyle().Foreground(util.ColorCritical)
+		return lipgloss.NewStyle().Foreground(ColorCritical)
 	}
 
-	return lipgloss.NewStyle().Foreground(util.ColorOk)
+	return lipgloss.NewStyle().Foreground(ColorOk)
 }
 
 func tempStyle(temp int64, criticalTemp int64) lipgloss.Style {
-	color := util.ColorOk
+	color := ColorOk
 
 	if temp >= criticalTemp {
-		color = util.ColorCritical
+		color = ColorCritical
 	} else if temp >= criticalTemp-10 {
-		color = util.ColorWarning
+		color = ColorWarning
 	}
 
 	return lipgloss.NewStyle().Foreground(color)
 }
 
 func rpmStyle(rpm int64) lipgloss.Style {
-	color := util.ColorOk
+	color := ColorOk
 
 	if rpm > 6000 {
-		color = util.ColorCritical
+		color = ColorCritical
 	} else if rpm > 5250 {
-		color = util.ColorWarning
+		color = ColorWarning
 	}
 
 	return lipgloss.NewStyle().Foreground(color)
 }
+
+func okStyle() lipgloss.Style {
+	return lipgloss.NewStyle().Foreground(ColorOk)
+}

internal/agent/api.go

@@ -123,7 +123,7 @@ func (a *computeBladeAgent) GetStatus(_ context.Context, _ *emptypb.Empty) (*bla
 		return nil, err
 	}
 
-	steps := a.fanController.Config().Steps
+	steps := a.fanController.Steps()
 	fanCurveSteps := make([]*bladeapiv1alpha1.FanCurveStep, len(steps))
 	for idx, step := range steps {
 		fanCurveSteps[idx] = &bladeapiv1alpha1.FanCurveStep{

pkg/fancontroller/fancontroller.go

@@ -15,8 +15,9 @@ type FanController interface {
 	// IsAutomaticSpeed returns true if the FanSpeed is determined by the fan controller logic, or false if determined
 	// by an FanOverrideOpts
 	IsAutomaticSpeed() bool
-	// Config returns a copy of the FanController Config
-	Config() Config
+
+	// Steps returns the list of temperature and fan speed steps configured for the fan controller.
+	Steps() []Step
 }
 
 // FanController is a simple fan controller that reacts to temperature changes with a linear function
@@ -66,8 +67,8 @@ func NewLinearFanController(config Config) (FanController, humane.Error) {
 	}, nil
 }
 
-func (f *fanControllerLinear) Config() Config {
-	return f.config
+func (f *fanControllerLinear) Steps() []Step {
+	return f.config.Steps
 }
 
 func (f *fanControllerLinear) Override(opts *FanOverrideOpts) {

pkg/fancontroller/fancontroller_test.go

@@ -4,6 +4,7 @@ import (
 	"testing"
 
 	"github.com/compute-blade-community/compute-blade-agent/pkg/fancontroller"
+	"github.com/stretchr/testify/assert"
 )
 
 func TestFanControllerLinear_GetFanSpeed(t *testing.T) {
@@ -30,15 +31,16 @@ func TestFanControllerLinear_GetFanSpeed(t *testing.T) {
 		{35, 60}, // Should use the maximum speed
 	}
 
+	assert.Equal(t, controller.Steps(), config.Steps)
+
 	for _, tc := range testCases {
 		expected := tc.expected
 		temperature := tc.temperature
 		t.Run("", func(t *testing.T) {
 			t.Parallel()
 			speed := controller.GetFanSpeedPercent(temperature)
-			if speed != expected {
-				t.Errorf("For temperature %.2f, expected speed %d but got %d", temperature, expected, speed)
-			}
+			assert.Equal(t, expected, speed)
+			assert.True(t, controller.IsAutomaticSpeed(), "Expected fan speed to be automatic, but it was not")
 		})
 	}
 }
@@ -76,9 +78,8 @@ func TestFanControllerLinear_GetFanSpeedWithOverride(t *testing.T) {
 		t.Run("", func(t *testing.T) {
 			t.Parallel()
 			speed := controller.GetFanSpeedPercent(temperature)
-			if speed != expected {
-				t.Errorf("For temperature %.2f, expected speed %d but got %d", temperature, expected, speed)
-			}
+			assert.Equal(t, expected, speed)
+			assert.False(t, controller.IsAutomaticSpeed(), "Expected fan speed to be overridden, but it was not")
 		})
 	}
 }
@@ -127,11 +128,9 @@ func TestFanControllerLinear_ConstructionErrors(t *testing.T) {
 		t.Run(tc.name, func(t *testing.T) {
 			t.Parallel()
 			_, err := fancontroller.NewLinearFanController(config)
-			if err == nil {
-				t.Errorf("Expected error with message '%s', but got no error", expectedErrMsg)
-			} else if err.Error() != expectedErrMsg {
-				t.Errorf("Expected error message '%s', but got '%s'", expectedErrMsg, err.Error())
-			}
+
+			assert.NotNil(t, err, "Expected error with message '%s', but got no error", expectedErrMsg)
+			assert.EqualError(t, err, expectedErrMsg)
 		})
 	}
 }

pkg/ledengine/ledengine_test.go

@@ -123,6 +123,17 @@ func TestNewLedEngine(t *testing.T) {
 	assert.NotNil(t, engine)
 }
 
+func TestLedEngine_NewLedEngineWithoutClock(t *testing.T) {
+	opts := ledengine.Options{
+		Clock:  nil,
+		LedIdx: 0,
+		Hal:    &hal.ComputeBladeHalMock{},
+	}
+
+	engine := ledengine.NewLedEngine(opts)
+	assert.NotNil(t, engine)
+}
+
 func Test_LedEngine_SetPattern_WhileRunning(t *testing.T) {
 	t.Parallel()
 
@@ -254,3 +265,29 @@ func Test_LedEngine_SetPattern_SetLedFailureInPattern(t *testing.T) {
 	clk.AssertExpectations(t)
 	cbMock.AssertExpectations(t)
 }
+
+func Test_LedEngine_SetPattern_NoDelay(t *testing.T) {
+	t.Parallel()
+
+	clk := util.MockClock{}
+	clkAfterChan := make(chan time.Time)
+	clk.On("After", time.Hour).Once().Return(clkAfterChan)
+
+	cbMock := hal.ComputeBladeHalMock{}
+	cbMock.On("SetLed", hal.LedTop, led.Color{Green: 0, Blue: 0, Red: 255}).Once().Return(nil)
+
+	opts := ledengine.Options{
+		Hal:    &cbMock,
+		Clock:  &clk,
+		LedIdx: 0,
+	}
+
+	engine := ledengine.NewLedEngine(opts)
+	invalidPattern := ledengine.NewStaticPattern(led.Color{Red: 255})
+	invalidPattern.Delays = []time.Duration{}
+	// We want to change the pattern BEFORE the engine is started
+	t.Log("Setting pattern")
+	err := engine.SetPattern(invalidPattern)
+	assert.Error(t, err)
+	assert.ErrorContains(t, err, "pattern must have at least one delay")
+}

pkg/util/clock_test.go

@@ -0,0 +1,72 @@
+package util_test
+
+import (
+	"testing"
+	"time"
+
+	"github.com/compute-blade-community/compute-blade-agent/pkg/util"
+
+	"github.com/stretchr/testify/assert"
+)
+
+// TestRealClock_Now ensures that RealClock.Now() returns a time close to the actual time.
+func TestRealClock_Now(t *testing.T) {
+	rc := util.RealClock{}
+	before := time.Now()
+	got := rc.Now()
+	after := time.Now()
+
+	if got.Before(before) || got.After(after) {
+		t.Errorf("RealClock.Now() = %v, want between %v and %v", got, before, after)
+	}
+}
+
+// TestRealClock_After ensures that RealClock.After() returns a channel that sends after the given duration.
+func TestRealClock_After(t *testing.T) {
+	rc := util.RealClock{}
+	delay := 50 * time.Millisecond
+
+	start := time.Now()
+	ch := rc.After(delay)
+	<-ch
+	elapsed := time.Since(start)
+
+	if elapsed < delay {
+		t.Errorf("RealClock.After(%v) triggered too early after %v", delay, elapsed)
+	}
+}
+
+// TestMockClock_Now tests that MockClock.Now() returns the expected time and records the call.
+func TestMockClock_Now(t *testing.T) {
+	mockClock := new(util.MockClock)
+	expectedTime := time.Date(2025, time.June, 6, 12, 0, 0, 0, time.UTC)
+
+	mockClock.On("Now").Return(expectedTime)
+
+	actualTime := mockClock.Now()
+	assert.Equal(t, expectedTime, actualTime)
+	mockClock.AssertCalled(t, "Now")
+	mockClock.AssertExpectations(t)
+}
+
+// TestMockClock_After tests that MockClock.After() returns the expected channel and records the call.
+func TestMockClock_After(t *testing.T) {
+	mockClock := new(util.MockClock)
+	duration := 100 * time.Millisecond
+	expectedChan := make(chan time.Time, 1)
+	expectedTime := time.Now().Add(duration)
+	expectedChan <- expectedTime
+
+	mockClock.On("After", duration).Return(expectedChan)
+
+	resultChan := mockClock.After(duration)
+	select {
+	case result := <-resultChan:
+		assert.WithinDuration(t, expectedTime, result, time.Second)
+	case <-time.After(time.Second):
+		t.Fatal("timeout waiting for result from MockClock.After")
+	}
+
+	mockClock.AssertCalled(t, "After", duration)
+	mockClock.AssertExpectations(t)
+}

pkg/util/file_exist_test.go

@@ -0,0 +1,25 @@
+package util_test
+
+import (
+	"os"
+	"testing"
+
+	"github.com/compute-blade-community/compute-blade-agent/pkg/util"
+	"github.com/stretchr/testify/assert"
+)
+
+func TestFileExists(t *testing.T) {
+	// Create a temporary file
+	tmpFile, err := os.CreateTemp("", "fileexists-test")
+	assert.NoError(t, err)
+
+	// It should exist
+	assert.True(t, util.FileExists(tmpFile.Name()), "Expected file to exist")
+
+	// Close and remove the file
+	assert.NoError(t, tmpFile.Close())
+	assert.NoError(t, os.Remove(tmpFile.Name()))
+
+	// It should not exist anymore
+	assert.False(t, util.FileExists(tmpFile.Name()), "Expected file not to exist")
+}

pkg/util/host_ips_test.go

@@ -0,0 +1,18 @@
+package util_test
+
+import (
+	"testing"
+
+	"github.com/compute-blade-community/compute-blade-agent/pkg/util"
+	"github.com/stretchr/testify/assert"
+)
+
+func TestGetHostIPs_ReturnsNonLoopbackIPs(t *testing.T) {
+	ips, err := util.GetHostIPs()
+	assert.NoError(t, err)
+
+	for _, ip := range ips {
+		assert.False(t, ip.IsLoopback(), "Should not return loopback IPs")
+		assert.False(t, ip.IsUnspecified(), "Should not return unspecified IPs")
+	}
+}