fix(helpers): return ErrValueNotSet for nonexistent units in GetNumRestarts (#9)

systemd returns NRestarts=0 for units with LoadState=not-found, making
it indistinguishable from a genuinely loaded unit with zero restarts.
GetNumRestarts now checks LoadState when NRestarts is 0 and returns
ErrValueNotSet for units that don't exist, matching GetMemoryUsage
behavior.

Also adds unit tests for filterErr (all stderr error mapping cases)
and HasValidUnitSuffix (all valid unit types + negative cases).

Updates syncthing test expectation from ErrValueNotSet to nil since
loaded-but-inactive units legitimately have NRestarts=0.
This commit is contained in:
2026-03-05 12:26:12 -05:00
committed by GitHub
parent 22132919e5
commit adf3c36632
3 changed files with 138 additions and 3 deletions

119
filtererr_test.go Normal file
View File

@@ -0,0 +1,119 @@
package systemctl
import (
"errors"
"testing"
)
func TestFilterErr(t *testing.T) {
tests := []struct {
name string
stderr string
want error
}{
{
name: "empty stderr",
stderr: "",
want: nil,
},
{
name: "unit does not exist",
stderr: "Unit foo.service does not exist, proceeding anyway.",
want: ErrDoesNotExist,
},
{
name: "unit not found",
stderr: "Unit foo.service not found.",
want: ErrDoesNotExist,
},
{
name: "unit not loaded",
stderr: "Unit foo.service not loaded.",
want: ErrUnitNotLoaded,
},
{
name: "no such file or directory",
stderr: "No such file or directory",
want: ErrDoesNotExist,
},
{
name: "interactive authentication required",
stderr: "Interactive authentication required.",
want: ErrInsufficientPermissions,
},
{
name: "access denied",
stderr: "Access denied",
want: ErrInsufficientPermissions,
},
{
name: "dbus session bus address",
stderr: "Failed to connect to bus: $DBUS_SESSION_BUS_ADDRESS not set",
want: ErrBusFailure,
},
{
name: "unit is masked",
stderr: "Unit foo.service is masked.",
want: ErrMasked,
},
{
name: "generic failed",
stderr: "Failed to do something unknown",
want: ErrUnspecified,
},
{
name: "unrecognized warning",
stderr: "Warning: something benign happened",
want: nil,
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
got := filterErr(tt.stderr)
if tt.want == nil {
if got != nil {
t.Errorf("filterErr(%q) = %v, want nil", tt.stderr, got)
}
return
}
if !errors.Is(got, tt.want) {
t.Errorf("filterErr(%q) = %v, want error wrapping %v", tt.stderr, got, tt.want)
}
})
}
}
func TestHasValidUnitSuffix(t *testing.T) {
tests := []struct {
unit string
want bool
}{
{"nginx.service", true},
{"sshd.socket", true},
{"backup.timer", true},
{"dev-sda1.device", true},
{"home.mount", true},
{"dev-sda1.swap", true},
{"user.slice", true},
{"multi-user.target", true},
{"session-1.scope", true},
{"foo.automount", true},
{"backup.path", true},
{"foo.snapshot", true},
{"nginx", false},
{"", false},
{"foo.bar", false},
{"foo.services", false},
{".service", true},
}
for _, tt := range tests {
t.Run(tt.unit, func(t *testing.T) {
got := HasValidUnitSuffix(tt.unit)
if got != tt.want {
t.Errorf("HasValidUnitSuffix(%q) = %v, want %v", tt.unit, got, tt.want)
}
})
}
}

View File

@@ -32,7 +32,23 @@ func GetNumRestarts(ctx context.Context, unit string, opts Options) (int, error)
if err != nil {
return -1, err
}
return strconv.Atoi(value)
if value == "[not set]" {
return -1, ErrValueNotSet
}
restarts, err := strconv.Atoi(value)
if err != nil {
return -1, err
}
// systemd returns NRestarts=0 for both genuinely zero-restart units and
// nonexistent/unloaded units. Disambiguate by checking LoadState: if the
// unit isn't loaded, the value is meaningless.
if restarts == 0 {
loadState, loadErr := Show(ctx, unit, properties.LoadState, opts)
if loadErr == nil && loadState == "not-found" {
return -1, ErrValueNotSet
}
}
return restarts, nil
}
// Get current memory in bytes (`systemctl show [unit] --property MemoryCurrent`) as an int

View File

@@ -105,8 +105,8 @@ func TestGetNumRestarts(t *testing.T) {
// try nonexistant unit in user mode as user
{"nonexistant", ErrValueNotSet, Options{UserMode: false}, true},
// try existing unit in user mode as user
{"syncthing", ErrValueNotSet, Options{UserMode: true}, true},
// try existing unit in user mode as user (loaded, so NRestarts=0 is valid)
{"syncthing", nil, Options{UserMode: true}, true},
// try existing unit in system mode as user
{"nginx", nil, Options{UserMode: false}, true},