test(pkg/systemd): add more unit tests to increasing coverage (#355)

Signed-off-by: Gyuho Lee <[email protected]> --------- Signed-off-by: Gyuho Lee <[email protected]>
leptonai · Feb 8, 2025 · 9fe57e6 · 9fe57e6
1 parent 4922eb9
commit 9fe57e6
Show file tree

Hide file tree

Showing 2 changed files with 282 additions and 6 deletions.
diff --git a/pkg/systemd/dbus.go b/pkg/systemd/dbus.go
@@ -9,8 +9,14 @@ import (
 	"github.com/coreos/go-systemd/v22/dbus"
 )
 
+type dbusConn interface {
+	Close()
+	Connected() bool
+	GetUnitPropertiesContext(ctx context.Context, unit string) (map[string]interface{}, error)
+}
+
 type DbusConn struct {
-	conn *dbus.Conn
+	conn dbusConn
 }
 
 // Caller should explicitly close the connection by calling Close() on returned connection object
@@ -37,13 +43,26 @@ func (c *DbusConn) IsActive(ctx context.Context, unitName string) (bool, error)
 	if !c.conn.Connected() {
 		return false, fmt.Errorf("connection disconnected")
 	}
-	if !strings.HasSuffix(unitName, ".target") && !strings.HasSuffix(unitName, ".service") {
-		unitName = fmt.Sprintf("%s.service", unitName)
-	}
-	props, err := c.conn.GetUnitPropertiesContext(ctx, unitName)
+
+	formattedUnitName := normalizeServiceUnitName(unitName)
+	props, err := c.conn.GetUnitPropertiesContext(ctx, formattedUnitName)
 	if err != nil {
-		return false, fmt.Errorf("unable to get unit properties for %s: %w", unitName, err)
+		return false, fmt.Errorf("unable to get unit properties for %s: %w", formattedUnitName, err)
+	}
+
+	return checkActiveState(props, formattedUnitName)
+}
+
+// normalizeServiceUnitName ensures the unit name has the correct suffix
+func normalizeServiceUnitName(unitName string) string {
+	if !strings.HasSuffix(unitName, ".target") && !strings.HasSuffix(unitName, ".service") {
+		return fmt.Sprintf("%s.service", unitName)
 	}
+	return unitName
+}
+
+// checkActiveState checks if a unit is active based on its properties
+func checkActiveState(props map[string]interface{}, unitName string) (bool, error) {
 	activeState, ok := props["ActiveState"]
 	if !ok {
 		return false, fmt.Errorf("ActiveState property not found for unit %s", unitName)

diff --git a/pkg/systemd/dbus_test.go b/pkg/systemd/dbus_test.go
@@ -0,0 +1,257 @@
+package systemd
+
+import (
+	"context"
+	"fmt"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+)
+
+// mockDbusConn implements dbusConn interface for testing
+type mockDbusConn struct {
+	connected bool
+	props     map[string]interface{}
+	err       error
+}
+
+func (m *mockDbusConn) Close() {}
+
+func (m *mockDbusConn) Connected() bool {
+	return m.connected
+}
+
+func (m *mockDbusConn) GetUnitPropertiesContext(_ context.Context, _ string) (map[string]interface{}, error) {
+	if m.err != nil {
+		return nil, m.err
+	}
+	return m.props, nil
+}
+
+func TestFormatUnitName(t *testing.T) {
+	tests := []struct {
+		name     string
+		input    string
+		expected string
+	}{
+		{
+			name:     "bare service name",
+			input:    "nginx",
+			expected: "nginx.service",
+		},
+		{
+			name:     "already has .service suffix",
+			input:    "nginx.service",
+			expected: "nginx.service",
+		},
+		{
+			name:     "has .target suffix",
+			input:    "network.target",
+			expected: "network.target",
+		},
+		{
+			name:     "empty string",
+			input:    "",
+			expected: ".service",
+		},
+		{
+			name:     "with dots in name",
+			input:    "my.custom.service.name",
+			expected: "my.custom.service.name.service",
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			result := normalizeServiceUnitName(tt.input)
+			assert.Equal(t, tt.expected, result)
+		})
+	}
+}
+
+func TestCheckActiveState(t *testing.T) {
+	tests := []struct {
+		name        string
+		props       map[string]interface{}
+		unitName    string
+		expected    bool
+		expectError bool
+		errorMsg    string
+	}{
+		{
+			name: "active service",
+			props: map[string]interface{}{
+				"ActiveState": "active",
+			},
+			unitName:    "test.service",
+			expected:    true,
+			expectError: false,
+		},
+		{
+			name: "inactive service",
+			props: map[string]interface{}{
+				"ActiveState": "inactive",
+			},
+			unitName:    "test.service",
+			expected:    false,
+			expectError: false,
+		},
+		{
+			name: "failed service",
+			props: map[string]interface{}{
+				"ActiveState": "failed",
+			},
+			unitName:    "test.service",
+			expected:    false,
+			expectError: false,
+		},
+		{
+			name:        "missing ActiveState",
+			props:       map[string]interface{}{},
+			unitName:    "test.service",
+			expected:    false,
+			expectError: true,
+			errorMsg:    "ActiveState property not found for unit test.service",
+		},
+		{
+			name: "wrong type for ActiveState",
+			props: map[string]interface{}{
+				"ActiveState": 123,
+			},
+			unitName:    "test.service",
+			expected:    false,
+			expectError: true,
+			errorMsg:    "ActiveState property is not a string for unit test.service",
+		},
+		{
+			name: "wrong type for ActiveState (bool)",
+			props: map[string]interface{}{
+				"ActiveState": true,
+			},
+			unitName:    "test.service",
+			expected:    false,
+			expectError: true,
+			errorMsg:    "ActiveState property is not a string for unit test.service",
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			result, err := checkActiveState(tt.props, tt.unitName)
+			if tt.expectError {
+				assert.Error(t, err)
+				assert.Equal(t, tt.errorMsg, err.Error())
+			} else {
+				assert.NoError(t, err)
+				assert.Equal(t, tt.expected, result)
+			}
+		})
+	}
+}
+
+func TestIsActive(t *testing.T) {
+	tests := []struct {
+		name        string
+		conn        *mockDbusConn
+		unitName    string
+		expected    bool
+		expectError bool
+		errorMsg    string
+	}{
+		{
+			name:        "nil connection",
+			conn:        nil,
+			unitName:    "test.service",
+			expectError: true,
+			errorMsg:    "connection not initialized",
+		},
+		{
+			name: "disconnected connection",
+			conn: &mockDbusConn{
+				connected: false,
+			},
+			unitName:    "test.service",
+			expectError: true,
+			errorMsg:    "connection disconnected",
+		},
+		{
+			name: "error getting properties",
+			conn: &mockDbusConn{
+				connected: true,
+				err:       fmt.Errorf("dbus error"),
+			},
+			unitName:    "test.service",
+			expectError: true,
+			errorMsg:    "unable to get unit properties for test.service: dbus error",
+		},
+		{
+			name: "active service",
+			conn: &mockDbusConn{
+				connected: true,
+				props: map[string]interface{}{
+					"ActiveState": "active",
+				},
+			},
+			unitName:    "test.service",
+			expected:    true,
+			expectError: false,
+		},
+		{
+			name: "inactive service",
+			conn: &mockDbusConn{
+				connected: true,
+				props: map[string]interface{}{
+					"ActiveState": "inactive",
+				},
+			},
+			unitName:    "test.service",
+			expected:    false,
+			expectError: false,
+		},
+		{
+			name: "service with target suffix",
+			conn: &mockDbusConn{
+				connected: true,
+				props: map[string]interface{}{
+					"ActiveState": "active",
+				},
+			},
+			unitName:    "test.target",
+			expected:    true,
+			expectError: false,
+		},
+		{
+			name: "service without suffix",
+			conn: &mockDbusConn{
+				connected: true,
+				props: map[string]interface{}{
+					"ActiveState": "active",
+				},
+			},
+			unitName:    "test",
+			expected:    true,
+			expectError: false,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			var conn *DbusConn
+			if tt.conn != nil {
+				conn = &DbusConn{conn: tt.conn}
+			} else {
+				conn = &DbusConn{conn: nil}
+			}
+
+			result, err := conn.IsActive(context.Background(), tt.unitName)
+			if tt.expectError {
+				assert.Error(t, err)
+				assert.Equal(t, tt.errorMsg, err.Error())
+				assert.False(t, result)
+			} else {
+				assert.NoError(t, err)
+				assert.Equal(t, tt.expected, result)
+			}
+		})
+	}
+}