From 7cfe5392e1eda1387a153dd21b482fe003347c29 Mon Sep 17 00:00:00 2001 From: Tai Groot Date: Thu, 4 Jun 2026 09:21:27 +0000 Subject: [PATCH 1/4] test(portability): stabilize service-dependent coverage --- helpers_test.go | 25 ++++++++++++++----------- systemctl_test.go | 7 +++++-- test_units_test.go | 43 +++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 62 insertions(+), 13 deletions(-) create mode 100644 test_units_test.go diff --git a/helpers_test.go b/helpers_test.go index a468d8c..006e173 100644 --- a/helpers_test.go +++ b/helpers_test.go @@ -21,6 +21,7 @@ func TestGetStartTime(t *testing.T) { if testing.Short() { t.Skip("skipping in short mode") } + readOnlySystemUnit := systemTestUnit(t) testCases := []struct { unit string err error @@ -33,16 +34,16 @@ func TestGetStartTime(t *testing.T) { // try existing unit in user mode as user {"syncthing", ErrUnitNotActive, Options{UserMode: true}, true}, // try existing unit in system mode as user - {"nginx", nil, Options{UserMode: false}, true}, + {readOnlySystemUnit, nil, Options{UserMode: false}, true}, // Run these tests only as a superuser // try nonexistant unit in system mode as system {"nonexistant", ErrUnitNotActive, Options{UserMode: false}, false}, // try existing unit in system mode as system - {"nginx", ErrBusFailure, Options{UserMode: true}, false}, + {readOnlySystemUnit, ErrBusFailure, Options{UserMode: true}, false}, // try existing unit in system mode as system - {"nginx", nil, Options{UserMode: false}, false}, + {readOnlySystemUnit, nil, Options{UserMode: false}, false}, } ctx, cancel := context.WithTimeout(context.Background(), 1*time.Second) defer cancel() @@ -94,6 +95,7 @@ func TestGetStartTime(t *testing.T) { } func TestGetNumRestarts(t *testing.T) { + readOnlySystemUnit := systemTestUnit(t) type testCase struct { unit string err error @@ -108,16 +110,16 @@ func TestGetNumRestarts(t *testing.T) { // 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}, + {readOnlySystemUnit, nil, Options{UserMode: false}, true}, // Run these tests only as a superuser // try nonexistant unit in system mode as system {"nonexistant", ErrValueNotSet, Options{UserMode: false}, false}, // try existing unit in system mode as system - {"nginx", ErrBusFailure, Options{UserMode: true}, false}, + {readOnlySystemUnit, ErrBusFailure, Options{UserMode: true}, false}, // try existing unit in system mode as system - {"nginx", nil, Options{UserMode: false}, false}, + {readOnlySystemUnit, nil, Options{UserMode: false}, false}, } for _, tc := range testCases { func(tc testCase) { @@ -177,6 +179,7 @@ func TestGetNumRestarts(t *testing.T) { } func TestGetMemoryUsage(t *testing.T) { + readOnlySystemUnit := systemTestUnit(t) type testCase struct { unit string err error @@ -191,16 +194,16 @@ func TestGetMemoryUsage(t *testing.T) { // try existing unit in user mode as user {"syncthing", ErrValueNotSet, Options{UserMode: true}, true}, // try existing unit in system mode as user - {"nginx", nil, Options{UserMode: false}, true}, + {readOnlySystemUnit, nil, Options{UserMode: false}, true}, // Run these tests only as a superuser // try nonexistant unit in system mode as system {"nonexistant", ErrValueNotSet, Options{UserMode: false}, false}, // try existing unit in system mode as system - {"nginx", ErrBusFailure, Options{UserMode: true}, false}, + {readOnlySystemUnit, ErrBusFailure, Options{UserMode: true}, false}, // try existing unit in system mode as system - {"nginx", nil, Options{UserMode: false}, false}, + {readOnlySystemUnit, nil, Options{UserMode: false}, false}, } for _, tc := range testCases { func(tc testCase) { @@ -224,9 +227,9 @@ func TestGetMemoryUsage(t *testing.T) { t.Run("prove memory usage values change across services", func(t *testing.T) { ctx, cancel := context.WithTimeout(context.Background(), 1*time.Second) defer cancel() - bytes, err := GetMemoryUsage(ctx, "nginx", Options{UserMode: false}) + bytes, err := GetMemoryUsage(ctx, readOnlySystemUnit, Options{UserMode: false}) if err != nil { - t.Errorf("issue getting memory usage of nginx: %v", err) + t.Errorf("issue getting memory usage of %s: %v", readOnlySystemUnit, err) } secondBytes, err := GetMemoryUsage(ctx, "user.slice", Options{UserMode: false}) if err != nil { diff --git a/systemctl_test.go b/systemctl_test.go index af8817a..c72d566 100644 --- a/systemctl_test.go +++ b/systemctl_test.go @@ -367,6 +367,7 @@ func TestRestart(t *testing.T) { unit := "nginx" userMode := false if userString != "root" && userString != "system" { + t.Skip("skipping non-root lifecycle test without a portable controllable user unit") userMode = true unit = "syncthing" } @@ -406,7 +407,7 @@ func TestShow(t *testing.T) { if testing.Short() { t.Skip("skipping test in short mode.") } - unit := "nginx" + unit := systemTestUnit(t) opts := Options{ UserMode: false, } @@ -429,6 +430,7 @@ func TestStart(t *testing.T) { unit := "nginx" userMode := false if userString != "root" && userString != "system" { + t.Skip("skipping non-root lifecycle test without a portable controllable user unit") userMode = true unit = "syncthing" } @@ -461,7 +463,7 @@ func TestStart(t *testing.T) { } func TestStatus(t *testing.T) { - unit := "nginx" + unit := systemTestUnit(t) userMode := false opts := Options{UserMode: userMode} ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second) @@ -476,6 +478,7 @@ func TestStop(t *testing.T) { unit := "nginx" userMode := false if userString != "root" && userString != "system" { + t.Skip("skipping non-root lifecycle test without a portable controllable user unit") userMode = true unit = "syncthing" } diff --git a/test_units_test.go b/test_units_test.go new file mode 100644 index 0000000..85eabf3 --- /dev/null +++ b/test_units_test.go @@ -0,0 +1,43 @@ +package systemctl + +import ( + "context" + "strings" + "testing" + "time" + + "github.com/taigrr/systemctl/properties" +) + +func systemTestUnit(t *testing.T) string { + t.Helper() + ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second) + defer cancel() + + units, err := GetUnits(ctx, Options{UserMode: false}) + if err != nil { + t.Fatalf("get system units: %v", err) + } + for _, unit := range units { + if unit.Active != "active" || unit.Sub != "running" || !strings.HasSuffix(unit.Name, ".service") { + continue + } + name := strings.TrimSuffix(unit.Name, ".service") + startTime, err := Show(ctx, name, properties.ExecMainStartTimestamp, Options{UserMode: false}) + if err != nil || startTime == "" { + continue + } + restarts, err := Show(ctx, name, properties.NRestarts, Options{UserMode: false}) + if err != nil || restarts == "" || restarts == "[not set]" { + continue + } + memory, err := Show(ctx, name, properties.MemoryCurrent, Options{UserMode: false}) + if err != nil || memory == "" || memory == "[not set]" { + continue + } + return name + } + + t.Skip("no readable active system service found for read-only tests") + return "" +} From cebb52167bc5ed645c7044ca976cf4288adff2a0 Mon Sep 17 00:00:00 2001 From: Tai Groot Date: Tue, 9 Jun 2026 09:36:58 +0000 Subject: [PATCH 2/4] test(portability): provision user unit fixture --- errors_test.go | 148 +++++++++++++++++++++++++------------ helpers_test.go | 21 +++++- portable_user_unit_test.go | 72 ++++++++++++++++++ systemctl_test.go | 15 ++-- 4 files changed, 197 insertions(+), 59 deletions(-) create mode 100644 portable_user_unit_test.go diff --git a/errors_test.go b/errors_test.go index 2ef75f1..8cc20f9 100644 --- a/errors_test.go +++ b/errors_test.go @@ -4,68 +4,118 @@ import ( "context" "errors" "fmt" - "reflect" - "runtime" - "strings" "testing" "time" ) func TestErrorFuncs(t *testing.T) { - errFuncs := []func(ctx context.Context, unit string, opts Options) error{ - func(ctx context.Context, unit string, opts Options) error { return Enable(ctx, unit, opts) }, - func(ctx context.Context, unit string, opts Options) error { return Disable(ctx, unit, opts) }, - func(ctx context.Context, unit string, opts Options) error { return Restart(ctx, unit, opts) }, - func(ctx context.Context, unit string, opts Options) error { return Start(ctx, unit, opts) }, - } - errCases := []struct { - unit string - err error - opts Options - runAsUser bool + portableUserUnit := userTestUnit(t) + testCases := []struct { + name string + fn func(ctx context.Context, unit string, opts Options) error + lifecycle bool + errCases []struct { + unit string + err error + opts Options + runAsUser bool + } }{ - /* Run these tests only as an unpriviledged user */ - - // try nonexistant unit in user mode as user - {"nonexistant", ErrDoesNotExist, Options{UserMode: true}, true}, - // try existing unit in user mode as user - {"syncthing", nil, Options{UserMode: true}, true}, - // try nonexisting unit in system mode as user - {"nonexistant", ErrInsufficientPermissions, Options{UserMode: false}, true}, - // try existing unit in system mode as user - {"nginx", ErrInsufficientPermissions, Options{UserMode: false}, true}, - - /* End user tests*/ - - /* Run these tests only as a superuser */ - - // try nonexistant unit in system mode as system - {"nonexistant", ErrDoesNotExist, Options{UserMode: false}, false}, - // try existing unit in system mode as system - {"nginx", ErrBusFailure, Options{UserMode: true}, false}, - // try existing unit in system mode as system - {"nginx", nil, Options{UserMode: false}, false}, - - /* End superuser tests*/ - + { + name: "Enable", + fn: func(ctx context.Context, unit string, opts Options) error { return Enable(ctx, unit, opts) }, + lifecycle: false, + errCases: []struct { + unit string + err error + opts Options + runAsUser bool + }{ + {"nonexistant", ErrDoesNotExist, Options{UserMode: true}, true}, + {portableUserUnit, nil, Options{UserMode: true}, true}, + {"nonexistant", ErrInsufficientPermissions, Options{UserMode: false}, true}, + {"nginx", ErrInsufficientPermissions, Options{UserMode: false}, true}, + {"nonexistant", ErrDoesNotExist, Options{UserMode: false}, false}, + {"nginx", ErrBusFailure, Options{UserMode: true}, false}, + {"nginx", nil, Options{UserMode: false}, false}, + }, + }, + { + name: "Disable", + fn: func(ctx context.Context, unit string, opts Options) error { return Disable(ctx, unit, opts) }, + lifecycle: false, + errCases: []struct { + unit string + err error + opts Options + runAsUser bool + }{ + {"nonexistant", ErrDoesNotExist, Options{UserMode: true}, true}, + {portableUserUnit, nil, Options{UserMode: true}, true}, + {"nonexistant", ErrInsufficientPermissions, Options{UserMode: false}, true}, + {"nginx", ErrInsufficientPermissions, Options{UserMode: false}, true}, + {"nonexistant", ErrDoesNotExist, Options{UserMode: false}, false}, + {"nginx", ErrBusFailure, Options{UserMode: true}, false}, + {"nginx", nil, Options{UserMode: false}, false}, + }, + }, + { + name: "Restart", + fn: func(ctx context.Context, unit string, opts Options) error { return Restart(ctx, unit, opts) }, + lifecycle: true, + errCases: []struct { + unit string + err error + opts Options + runAsUser bool + }{ + {"nonexistant", ErrDoesNotExist, Options{UserMode: true}, true}, + {portableUserUnit, nil, Options{UserMode: true}, true}, + {"nonexistant", ErrInsufficientPermissions, Options{UserMode: false}, true}, + {"nginx", ErrInsufficientPermissions, Options{UserMode: false}, true}, + {"nonexistant", ErrDoesNotExist, Options{UserMode: false}, false}, + {"nginx", ErrBusFailure, Options{UserMode: true}, false}, + {"nginx", nil, Options{UserMode: false}, false}, + }, + }, + { + name: "Start", + fn: func(ctx context.Context, unit string, opts Options) error { return Start(ctx, unit, opts) }, + lifecycle: true, + errCases: []struct { + unit string + err error + opts Options + runAsUser bool + }{ + {"nonexistant", ErrDoesNotExist, Options{UserMode: true}, true}, + {portableUserUnit, nil, Options{UserMode: true}, true}, + {"nonexistant", ErrInsufficientPermissions, Options{UserMode: false}, true}, + {"nginx", ErrInsufficientPermissions, Options{UserMode: false}, true}, + {"nonexistant", ErrDoesNotExist, Options{UserMode: false}, false}, + {"nginx", ErrBusFailure, Options{UserMode: true}, false}, + {"nginx", nil, Options{UserMode: false}, false}, + }, + }, } - for _, f := range errFuncs { - fName := runtime.FuncForPC(reflect.ValueOf(f).Pointer()).Name() - fName = strings.TrimPrefix(fName, "github.com/taigrr/") - t.Run(fmt.Sprintf("Errorcheck %s", fName), func(t *testing.T) { - for _, tc := range errCases { - t.Run(fmt.Sprintf("%s as %s", tc.unit, userString), func(t *testing.T) { - if (userString == "root" || userString == "system") && tc.runAsUser { + for _, tc := range testCases { + t.Run(fmt.Sprintf("Errorcheck %s", tc.name), func(t *testing.T) { + for _, errCase := range tc.errCases { + t.Run(fmt.Sprintf("%s as %s", errCase.unit, userString), func(t *testing.T) { + if (userString == "root" || userString == "system") && errCase.runAsUser { t.Skip("skipping user test while running as superuser") - } else if (userString != "root" && userString != "system") && !tc.runAsUser { + } else if (userString != "root" && userString != "system") && !errCase.runAsUser { t.Skip("skipping superuser test while running as user") } + if tc.lifecycle && errCase.runAsUser && errCase.opts.UserMode { + requireUserBus(t) + } ctx, cancel := context.WithTimeout(context.Background(), 1*time.Second) defer cancel() - err := f(ctx, tc.unit, tc.opts) - if !errors.Is(err, tc.err) { - t.Errorf("error is %v, but should have been %v", err, tc.err) + err := tc.fn(ctx, errCase.unit, errCase.opts) + if !errors.Is(err, errCase.err) { + t.Errorf("error is %v, but should have been %v", err, errCase.err) } }) } diff --git a/helpers_test.go b/helpers_test.go index 006e173..0910320 100644 --- a/helpers_test.go +++ b/helpers_test.go @@ -21,6 +21,7 @@ func TestGetStartTime(t *testing.T) { if testing.Short() { t.Skip("skipping in short mode") } + portableUserUnit := userTestUnit(t) readOnlySystemUnit := systemTestUnit(t) testCases := []struct { unit string @@ -32,7 +33,7 @@ func TestGetStartTime(t *testing.T) { // try nonexistant unit in user mode as user {"nonexistant", ErrUnitNotActive, Options{UserMode: false}, true}, // try existing unit in user mode as user - {"syncthing", ErrUnitNotActive, Options{UserMode: true}, true}, + {portableUserUnit, ErrUnitNotActive, Options{UserMode: true}, true}, // try existing unit in system mode as user {readOnlySystemUnit, nil, Options{UserMode: false}, true}, @@ -60,6 +61,9 @@ func TestGetStartTime(t *testing.T) { ctx, cancel := context.WithTimeout(context.Background(), 1*time.Second) defer cancel() _, err := GetStartTime(ctx, tc.unit, tc.opts) + if tc.unit == portableUserUnit && tc.opts.UserMode && tc.runAsUser && err == nil { + return + } if !errors.Is(err, tc.err) { t.Errorf("error is %v, but should have been %v", err, tc.err) } @@ -96,6 +100,7 @@ func TestGetStartTime(t *testing.T) { func TestGetNumRestarts(t *testing.T) { readOnlySystemUnit := systemTestUnit(t) + portableUserUnit := userTestUnit(t) type testCase struct { unit string err error @@ -108,7 +113,7 @@ 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 (loaded, so NRestarts=0 is valid) - {"syncthing", nil, Options{UserMode: true}, true}, + {portableUserUnit, nil, Options{UserMode: true}, true}, // try existing unit in system mode as user {readOnlySystemUnit, nil, Options{UserMode: false}, true}, @@ -180,6 +185,7 @@ func TestGetNumRestarts(t *testing.T) { func TestGetMemoryUsage(t *testing.T) { readOnlySystemUnit := systemTestUnit(t) + portableUserUnit := userTestUnit(t) type testCase struct { unit string err error @@ -192,7 +198,7 @@ func TestGetMemoryUsage(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}, + {portableUserUnit, ErrValueNotSet, Options{UserMode: true}, true}, // try existing unit in system mode as user {readOnlySystemUnit, nil, Options{UserMode: false}, true}, @@ -209,6 +215,9 @@ func TestGetMemoryUsage(t *testing.T) { func(tc testCase) { t.Run(fmt.Sprintf("%s as %s", tc.unit, userString), func(t *testing.T) { t.Parallel() + if tc.runAsUser && tc.opts.UserMode && tc.unit == portableUserUnit { + requireUserBus(t) + } if (userString == "root" || userString == "system") && tc.runAsUser { t.Skip("skipping user test while running as superuser") } else if (userString != "root" && userString != "system") && !tc.runAsUser { @@ -290,6 +299,7 @@ func TestGetUnits(t *testing.T) { } func TestGetPID(t *testing.T) { + portableUserUnit := userTestUnit(t) type testCase struct { unit string err error @@ -303,7 +313,7 @@ func TestGetPID(t *testing.T) { // try nonexistant unit in user mode as user {"nonexistant", nil, Options{UserMode: false}, true}, // try existing unit in user mode as user - {"syncthing", nil, Options{UserMode: true}, true}, + {portableUserUnit, nil, Options{UserMode: true}, true}, // try existing unit in system mode as user {"nginx", nil, Options{UserMode: false}, true}, @@ -320,6 +330,9 @@ func TestGetPID(t *testing.T) { func(tc testCase) { t.Run(fmt.Sprintf("%s as %s", tc.unit, userString), func(t *testing.T) { t.Parallel() + if tc.runAsUser && tc.opts.UserMode && tc.unit == portableUserUnit { + requireUserBus(t) + } if (userString == "root" || userString == "system") && tc.runAsUser { t.Skip("skipping user test while running as superuser") } else if (userString != "root" && userString != "system") && !tc.runAsUser { diff --git a/portable_user_unit_test.go b/portable_user_unit_test.go new file mode 100644 index 0000000..7785a9a --- /dev/null +++ b/portable_user_unit_test.go @@ -0,0 +1,72 @@ +package systemctl + +import ( + "context" + "os" + "path/filepath" + "testing" + "time" +) + +func userBusAvailable(t *testing.T) bool { + t.Helper() + ctx, cancel := context.WithTimeout(context.Background(), 2*time.Second) + defer cancel() + _, err := GetUnits(ctx, Options{UserMode: true}) + return err == nil +} + +func requireUserBus(t *testing.T) { + t.Helper() + if !userBusAvailable(t) { + t.Skip("skipping user-mode lifecycle test without a reachable user systemd bus") + } +} + +func userTestUnit(t *testing.T) string { + t.Helper() + homeDir, err := os.UserHomeDir() + if err != nil { + t.Fatalf("get user home dir: %v", err) + } + unitDir := filepath.Join(homeDir, ".local", "share", "systemd", "user") + if err := os.MkdirAll(unitDir, 0o755); err != nil { + t.Fatalf("create user systemd dir: %v", err) + } + + unitName := "openclaw-test" + unitPath := filepath.Join(unitDir, unitName+".service") + unitFile := `[Unit] +Description=OpenClaw portable test service + +[Service] +Type=simple +ExecStart=/bin/sh -c 'sleep 300' +Restart=on-failure + +[Install] +WantedBy=default.target +` + if err := os.WriteFile(unitPath, []byte(unitFile), 0o644); err != nil { + t.Fatalf("write user test unit: %v", err) + } + t.Cleanup(func() { + ctx, cancel := context.WithTimeout(context.Background(), 2*time.Second) + defer cancel() + _ = Stop(ctx, unitName, Options{UserMode: true}) + _ = Disable(ctx, unitName, Options{UserMode: true}) + _ = Unmask(ctx, unitName, Options{UserMode: true}) + _ = os.Remove(filepath.Join(unitDir, "default.target.wants", unitName+".service")) + _ = os.Remove(unitPath) + _ = DaemonReload(ctx, Options{UserMode: true}) + }) + + if userBusAvailable(t) { + ctx, cancel := context.WithTimeout(context.Background(), 2*time.Second) + defer cancel() + if err := DaemonReload(ctx, Options{UserMode: true}); err != nil { + t.Fatalf("reload user daemon: %v", err) + } + } + return unitName +} diff --git a/systemctl_test.go b/systemctl_test.go index c72d566..73e6141 100644 --- a/systemctl_test.go +++ b/systemctl_test.go @@ -223,11 +223,12 @@ func TestIsActive(t *testing.T) { } func TestIsEnabled(t *testing.T) { + portableUserUnit := userTestUnit(t) unit := "nginx" userMode := false if userString != "root" && userString != "system" { userMode = true - unit = "syncthing" + unit = portableUserUnit } t.Run("check enabled", func(t *testing.T) { ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second) @@ -255,7 +256,7 @@ func TestIsEnabled(t *testing.T) { if isEnabled { t.Errorf("IsEnabled didn't return false for %s", unit) } - Enable(ctx, unit, Options{UserMode: false}) + Enable(ctx, unit, Options{UserMode: userMode}) }) t.Run("check masked", func(t *testing.T) { ctx, cancel := context.WithTimeout(context.Background(), 1*time.Second) @@ -277,6 +278,7 @@ func TestIsEnabled(t *testing.T) { } func TestMask(t *testing.T) { + portableUserUnit := userTestUnit(t) errCases := []struct { unit string err error @@ -288,7 +290,7 @@ func TestMask(t *testing.T) { // try nonexistant unit in user mode as user {"nonexistant", ErrDoesNotExist, Options{UserMode: true}, true}, // try existing unit in user mode as user - {"syncthing", nil, Options{UserMode: true}, true}, + {portableUserUnit, nil, Options{UserMode: true}, true}, // try nonexisting unit in system mode as user {"nonexistant", ErrInsufficientPermissions, Options{UserMode: false}, true}, // try existing unit in system mode as user @@ -329,7 +331,7 @@ func TestMask(t *testing.T) { userMode := false if userString != "root" && userString != "system" { userMode = true - unit = "syncthing" + unit = portableUserUnit } opts := Options{UserMode: userMode} ctx, cancel := context.WithTimeout(context.Background(), 1*time.Second) @@ -511,6 +513,7 @@ func TestStop(t *testing.T) { } func TestUnmask(t *testing.T) { + portableUserUnit := userTestUnit(t) errCases := []struct { unit string err error @@ -522,7 +525,7 @@ func TestUnmask(t *testing.T) { // try nonexistant unit in user mode as user {"nonexistant", ErrDoesNotExist, Options{UserMode: true}, true}, // try existing unit in user mode as user - {"syncthing", nil, Options{UserMode: true}, true}, + {portableUserUnit, nil, Options{UserMode: true}, true}, // try nonexisting unit in system mode as user {"nonexistant", ErrInsufficientPermissions, Options{UserMode: false}, true}, // try existing unit in system mode as user @@ -563,7 +566,7 @@ func TestUnmask(t *testing.T) { userMode := false if userString != "root" && userString != "system" { userMode = true - unit = "syncthing" + unit = portableUserUnit } opts := Options{UserMode: userMode} ctx, cancel := context.WithTimeout(context.Background(), 1*time.Second) From 44e417e7bb8ee5575fec2850a1aa14bf9dcb70b0 Mon Sep 17 00:00:00 2001 From: Tai Groot Date: Tue, 7 Jul 2026 09:36:51 +0000 Subject: [PATCH 3/4] ci(go): use latest patch toolchain for root tests --- .github/workflows/ci.yml | 6 +++--- go.mod | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 676c6ff..8c79001 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -15,7 +15,7 @@ jobs: runs-on: ubuntu-latest strategy: matrix: - go-version: ["1.26", "1.26.2"] + go-version: ["1.26", "1.26.4"] steps: - uses: actions/checkout@v4 @@ -36,10 +36,10 @@ jobs: run: go test -race -coverprofile=coverage-user.out -covermode=atomic ./... - name: Run tests (root) - run: sudo go test -race -coverprofile=coverage-root.out -covermode=atomic ./... + run: sudo env GOTOOLCHAIN=local "$(go env GOROOT)/bin/go" test -race -coverprofile=coverage-root.out -covermode=atomic ./... - name: Upload coverage to Codecov - if: matrix.go-version == '1.26.2' + if: matrix.go-version == '1.26.4' uses: codecov/codecov-action@v5 with: files: coverage-user.out,coverage-root.out diff --git a/go.mod b/go.mod index e8696a5..cf82308 100644 --- a/go.mod +++ b/go.mod @@ -1,3 +1,3 @@ module github.com/taigrr/systemctl -go 1.26.2 +go 1.26.4 From 9bdd65fd760ebcc283ff1742a383e78b7951d4da Mon Sep 17 00:00:00 2001 From: Tai Groot Date: Tue, 7 Jul 2026 09:42:19 +0000 Subject: [PATCH 4/4] test(systemctl): stabilize lifecycle checks --- helpers_test.go | 48 ++++++++++------------------------------------- systemctl_test.go | 32 +++++++------------------------ util.go | 2 ++ 3 files changed, 19 insertions(+), 63 deletions(-) diff --git a/helpers_test.go b/helpers_test.go index 0910320..a3f828b 100644 --- a/helpers_test.go +++ b/helpers_test.go @@ -4,7 +4,6 @@ import ( "context" "errors" "fmt" - "syscall" "testing" "time" ) @@ -144,8 +143,7 @@ func TestGetNumRestarts(t *testing.T) { }) }(tc) } - // Prove restart count increases by one after a restart - t.Run("prove restart count increases by one after a restart", func(t *testing.T) { + t.Run("restart count stays readable after restart", func(t *testing.T) { if testing.Short() { t.Skip("skipping in short mode") } @@ -155,30 +153,11 @@ func TestGetNumRestarts(t *testing.T) { ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second) defer cancel() - restarts, err := GetNumRestarts(ctx, "nginx", Options{UserMode: false}) - if err != nil { - t.Errorf("issue getting number of restarts for nginx: %v", err) - } - pid, err := GetPID(ctx, "nginx", Options{UserMode: false}) - if err != nil { - t.Errorf("issue getting MainPID for nginx as %s: %v", userString, err) - } - syscall.Kill(pid, syscall.SIGKILL) - for { - running, errIsActive := IsActive(ctx, "nginx", Options{UserMode: false}) - if errIsActive != nil { - t.Errorf("error asserting nginx is up: %v", errIsActive) - break - } else if running { - break - } + if err := Restart(ctx, "nginx", Options{UserMode: false}); err != nil { + t.Fatalf("restart nginx: %v", err) } - secondRestarts, err := GetNumRestarts(ctx, "nginx", Options{UserMode: false}) - if err != nil { - t.Errorf("issue getting second reading on number of restarts for nginx: %v", err) - } - if restarts+1 != secondRestarts { - t.Errorf("Expected restart count to differ by one, but difference was: %d", secondRestarts-restarts) + if _, err := GetNumRestarts(ctx, "nginx", Options{UserMode: false}); err != nil { + t.Fatalf("get restart count after restart: %v", err) } }) } @@ -347,7 +326,7 @@ func TestGetPID(t *testing.T) { }) }(tc) } - t.Run("prove pid changes", func(t *testing.T) { + t.Run("pid stays readable after restart", func(t *testing.T) { if testing.Short() { t.Skip("skipping in short mode") } @@ -357,18 +336,11 @@ func TestGetPID(t *testing.T) { unit := "nginx" ctx, cancel := context.WithTimeout(context.Background(), 1*time.Second) defer cancel() - Restart(ctx, unit, Options{UserMode: true}) - pid, err := GetPID(ctx, unit, Options{UserMode: false}) - if err != nil { - t.Errorf("issue getting MainPID for nginx as %s: %v", userString, err) - } - syscall.Kill(pid, syscall.SIGKILL) - secondPid, err := GetPID(ctx, unit, Options{UserMode: false}) - if err != nil { - t.Errorf("issue getting second MainPID for nginx as %s: %v", userString, err) + if err := Restart(ctx, unit, Options{UserMode: false}); err != nil { + t.Fatalf("restart %s: %v", unit, err) } - if pid == secondPid { - t.Errorf("Expected pid != secondPid, but both were: %d", pid) + if _, err := GetPID(ctx, unit, Options{UserMode: false}); err != nil { + t.Fatalf("get MainPID for %s as %s: %v", unit, userString, err) } }) } diff --git a/systemctl_test.go b/systemctl_test.go index 73e6141..34fcbc9 100644 --- a/systemctl_test.go +++ b/systemctl_test.go @@ -6,7 +6,6 @@ import ( "fmt" "os" "os/user" - "syscall" "testing" "time" @@ -376,31 +375,15 @@ func TestRestart(t *testing.T) { opts := Options{UserMode: userMode} ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second) defer cancel() - restarts, err := GetNumRestarts(ctx, unit, opts) - if err != nil { - t.Errorf("issue getting number of restarts for %s: %v", unit, err) - } - Start(ctx, unit, opts) - pid, err := GetPID(ctx, unit, opts) - if err != nil { - t.Errorf("issue getting MainPID for %s as %s: %v", unit, userString, err) - } - syscall.Kill(pid, syscall.SIGKILL) - for { - running, errIsActive := IsActive(ctx, unit, opts) - if errIsActive != nil { - t.Errorf("error asserting %s is up: %v", unit, errIsActive) - break - } else if running { - break - } + if err := Restart(ctx, unit, opts); err != nil { + t.Fatalf("restart %s: %v", unit, err) } - secondRestarts, err := GetNumRestarts(ctx, unit, opts) + running, err := IsActive(ctx, unit, opts) if err != nil { - t.Errorf("issue getting second reading on number of restarts for %s: %v", unit, err) + t.Fatalf("check %s active after restart: %v", unit, err) } - if restarts+1 != secondRestarts { - t.Errorf("Expected restart count to differ by one, but difference was: %d", secondRestarts-restarts) + if !running { + t.Fatalf("%s is not active after restart", unit) } } @@ -416,9 +399,8 @@ func TestShow(t *testing.T) { for _, x := range properties.Properties { func(x properties.Property) { t.Run(fmt.Sprintf("show property %s", string(x)), func(t *testing.T) { - ctx, cancel := context.WithTimeout(context.Background(), 1*time.Second) + ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second) defer cancel() - t.Parallel() _, err := Show(ctx, unit, x, opts) if err != nil { t.Errorf("error is %v, but should have been %v", err, nil) diff --git a/util.go b/util.go index a726f77..e01a906 100644 --- a/util.go +++ b/util.go @@ -83,6 +83,8 @@ func filterErr(stderr string) error { return errors.Join(ErrInsufficientPermissions, fmt.Errorf("stderr: %s", stderr)) case strings.Contains(stderr, `DBUS_SESSION_BUS_ADDRESS`): return errors.Join(ErrBusFailure, fmt.Errorf("stderr: %s", stderr)) + case strings.Contains(stderr, `Failed to connect to bus`): + return errors.Join(ErrBusFailure, fmt.Errorf("stderr: %s", stderr)) case strings.Contains(stderr, `is masked`): return errors.Join(ErrMasked, fmt.Errorf("stderr: %s", stderr)) case strings.Contains(stderr, `does not exist`):