From aaa5f28ba7f8874431ba6e0052149df9c7b90a53 Mon Sep 17 00:00:00 2001 From: Tai Groot Date: Sat, 15 May 2021 17:39:41 -0700 Subject: [PATCH] adds table-driven tests --- README.md | 27 ++++++----- errors.go | 2 + systemctl_test.go | 118 ++++++++++++++++++++++++++++------------------ util.go | 3 ++ 4 files changed, 92 insertions(+), 58 deletions(-) diff --git a/README.md b/README.md index 0b49623..3530304 100644 --- a/README.md +++ b/README.md @@ -4,6 +4,7 @@ This library aims at providing idiomatic `systemctl` bindings for go developers, This tool tries to take guesswork out of arbitrarily shelling out to `systemctl` by providing a structured, throuroughly-tested wrapper for the `systemctl` functions most-likely to be used in a system program. If your system isn't running (or targeting another system running) `systemctl`, this library will be of little use to you. +In fact, if `systemctl` isn't found in the `PATH`, this library will panic. ## What is systemctl @@ -13,25 +14,25 @@ If your system isn't running (or targeting another system running) `systemctl`, ## Supported systemctl functions -- [ ] `systemctl is-failed` -- [ ] `systemctl is-active` -- [ ] `systemctl status` -- [ ] `systemctl restart` -- [ ] `systemctl start` -- [ ] `systemctl stop` -- [ ] `systemctl enable` -- [ ] `systemctl disable` -- [ ] `systemctl is-enabled` - [ ] `systemctl daemon-reload` -- [ ] `systemctl show` +- [ ] `systemctl disable` +- [ ] `systemctl enable` +- [ ] `systemctl is-active` +- [ ] `systemctl is-enabled` +- [ ] `systemctl is-failed` - [ ] `systemctl mask` +- [ ] `systemctl restart` +- [ ] `systemctl show` +- [ ] `systemctl start` +- [ ] `systemctl status` +- [ ] `systemctl stop` - [ ] `systemctl unmask` ## Helper functionality -- [ ] Get start time of a service (`ExecMainStartTimestamp`) as a `Time` type -- [ ] Get current memory in bytes (`MemoryCurrent`) an an int -- [ ] Get the PID of the main process (`MainPID`) as an int +- [x] Get start time of a service (`ExecMainStartTimestamp`) as a `Time` type +- [x] Get current memory in bytes (`MemoryCurrent`) an an int +- [x] Get the PID of the main process (`MainPID`) as an int ## Useful errors diff --git a/errors.go b/errors.go index e8242a5..d300169 100644 --- a/errors.go +++ b/errors.go @@ -21,4 +21,6 @@ var ( ErrUnspecified = errors.New("Unknown error") // ErrUnitNotRunning means a function which requires a unit to be run (such as GetStartTime) was run against an inactive unit ErrUnitNotRunning = errors.New("Unit isn't running") + // ErrBusFailure means $DBUS_SESSION_BUS_ADDRESS and $XDG_RUNTIME_DIR were not defined + ErrBusFailure = errors.New("Failure to connect to bus, did you run in usermode as root?") ) diff --git a/systemctl_test.go b/systemctl_test.go index 409b0e6..7c7f710 100644 --- a/systemctl_test.go +++ b/systemctl_test.go @@ -2,58 +2,84 @@ package systemctl import ( "context" + "fmt" + "os" + "os/user" "testing" "time" "github.com/taigrr/systemctl/properties" ) -func TestEnableNonexistant(t *testing.T) { - unit := "nonexistant" - ctx, cancel := context.WithTimeout(context.Background(), 1*time.Second) - defer cancel() - opts := Options{ - usermode: true, - } - err := Enable(ctx, unit, opts) - if err != ErrDoesNotExist { - t.Errorf("error is %v, but should have been %v", err, ErrDoesNotExist) - } +var userString string -} +// Testing assumptions +// - there's no unit installed named `nonexistant` +// - the syncthing unit to be available on the tester's system. +// this is just what was available on mine, should you want to change it, +// either to something in this repo or more common, feel free to submit a PR. +// - your 'user' isn't root +// - your user doesn't have a PolKit rule allowing access to configure nginx -// Note: test assumes your user isn't root and doesn't have a PolKit rule allowing access -// to configure nginx. Whether it's installed should be irrelevant. -func TestEnableNoPermissions(t *testing.T) { - unit := "nginx" - ctx, cancel := context.WithTimeout(context.Background(), 1*time.Second) - defer cancel() - opts := Options{ - usermode: false, - } - err := Enable(ctx, unit, opts) - if err != ErrInsufficientPermissions { - t.Errorf("error is %v, but should have been %v", err, ErrInsufficientPermissions) - } +func TestMain(m *testing.M) { + curUser, err := user.Current() -} - -// Note: requires the syncthing unit to be available on the tester's system. -// this is just what was available on mine, should you want to change it, -// either to something in this repo or more common, feel free to submit a PR. -func TestEnableSuccess(t *testing.T) { - unit := "syncthing" - ctx, cancel := context.WithTimeout(context.Background(), 1*time.Second) - defer cancel() - opts := Options{ - usermode: true, - } - err := Enable(ctx, unit, opts) if err != nil { - t.Errorf("error is %v, but should have been %v", err, nil) + fmt.Println("Could not determine running user") } + userString = curUser.Username + fmt.Printf("currently running tests as: %s \n", userString) + fmt.Println("Don't forget to run both root and user tests.") + os.Exit(m.Run()) } -func TestAllProperties(t *testing.T) { + +func TestEnable(t *testing.T) { + testCases := []struct { + unit string + err error + opts Options + runAsUser bool + }{ + // Run these tests only as a 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}, + + // 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}, + } + for _, tc := range testCases { + t.Run(fmt.Sprintf("%s as %s", tc.unit, userString), func(t *testing.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 { + t.Skip("skipping superuser test while running as user") + } + ctx, cancel := context.WithTimeout(context.Background(), 1*time.Second) + defer cancel() + err := Enable(ctx, tc.unit, tc.opts) + if err != tc.err { + t.Errorf("error is %v, but should have been %v", err, tc.err) + } + }) + } + +} + +// Runs through all defined properties and checks for error cases +func TestShow(t *testing.T) { if testing.Short() { t.Skip("skipping test in short mode.") } @@ -61,12 +87,14 @@ func TestAllProperties(t *testing.T) { ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second) defer cancel() opts := Options{ - usermode: true, + usermode: false, } for _, x := range properties.Properties { - _, err := Show(ctx, unit, x, opts) - if err != nil { - t.Errorf("error is %v, but should have been %v", err, nil) - } + t.Run(fmt.Sprintf("show property %s", string(x)), func(t *testing.T) { + _, 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 7a78964..ee38e62 100644 --- a/util.go +++ b/util.go @@ -62,6 +62,9 @@ func filterErr(stderr string) error { if matched, _ := regexp.MatchString(`Access denied`, stderr); matched { return ErrInsufficientPermissions } + if matched, _ := regexp.MatchString(`DBUS_SESSION_BUS_ADDRESS`, stderr); matched { + return ErrBusFailure + } if matched, _ := regexp.MatchString(`Failed`, stderr); matched { return ErrUnspecified }