From e941c90f0df29b2346b84a208da2cfb468bdf985 Mon Sep 17 00:00:00 2001 From: Tai Groot Date: Sun, 16 May 2021 22:00:50 -0700 Subject: [PATCH] Migrates many tests to errors_test --- errors.go | 5 +- errors_test.go | 92 ++++++++++++++++++++++++++++ helpers_test.go | 11 +++- systemctl.go | 8 +++ systemctl_test.go | 151 +++++++++++++++++++++------------------------- util.go | 6 ++ 6 files changed, 188 insertions(+), 85 deletions(-) create mode 100644 errors_test.go diff --git a/errors.go b/errors.go index 18a016e..4183ea0 100644 --- a/errors.go +++ b/errors.go @@ -24,9 +24,12 @@ var ( // If this error occurs, the library isn't entirely useful, as it causes a panic // Make sure systemctl is in the PATH before calling again ErrNotInstalled = errors.New("systemctl not in $PATH") - // A unit was expected to be running but was found inactive + // A unit was expected to be running but was found inactive // This can happen when calling GetStartTime on a dead unit, for example ErrUnitNotActive = errors.New("unit not active") + // A unit was expected to be loaded, but was not. + // This can happen when trying to Stop a unit which does not exist, for example + ErrUnitNotLoaded = errors.New("unit not loaded") // An expected value is unavailable, but the unit may be running // This can happen when calling GetMemoryUsage on systemd itself, for example ErrValueNotSet = errors.New("value not set") diff --git a/errors_test.go b/errors_test.go new file mode 100644 index 0000000..aaa0743 --- /dev/null +++ b/errors_test.go @@ -0,0 +1,92 @@ +package systemctl + +import ( + "context" + "fmt" + "reflect" + "runtime" + "strings" + "testing" + "time" +) + +func TestErrorFuncs(t *testing.T) { + errFuncs := []func(ctx context.Context, unit string, opts Options) error{ + Enable, + Disable, + Restart, + Start, + } + 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*/ + + } + + 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 { + 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 := f(ctx, tc.unit, tc.opts) + if err != tc.err { + t.Errorf("error is %v, but should have been %v", err, tc.err) + } + }) + } + }) + } +} + +// /* Run these tests only as a user */ +// +// // fail to reload system daemon as user +// {"", ErrInsufficientPermissions, Options{UserMode: false}, true}, +// // reload user's scope daemon +// {"", nil, Options{UserMode: true}, true}, +// /* End user tests*/ +// +// /* Run these tests only as a superuser */ +// +// // succeed to reload daemon +// {"", nil, Options{UserMode: false}, false}, +// // fail to connect to user bus as system +// {"", ErrBusFailure, Options{UserMode: true}, false}, +// +// /* End superuser tests*/ +// diff --git a/helpers_test.go b/helpers_test.go index 9225265..a1debbb 100644 --- a/helpers_test.go +++ b/helpers_test.go @@ -40,8 +40,13 @@ func TestGetStartTime(t *testing.T) { // try existing unit in system mode as system {"nginx", nil, Options{UserMode: false}, false}, } + ctx, cancel := context.WithTimeout(context.Background(), 1*time.Second) + defer cancel() + Restart(ctx, "syncthing", Options{UserMode: true}) + Stop(ctx, "syncthing", Options{UserMode: true}) + time.Sleep(1 * time.Second) for _, tc := range testCases { - t.Run(fmt.Sprintf("%s as %s", tc.unit, userString), func(t *testing.T) { + t.Run(fmt.Sprintf("%s as %s, UserMode=%v", tc.unit, userString, tc.opts.UserMode), 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 { @@ -61,16 +66,18 @@ func TestGetStartTime(t *testing.T) { t.Skip("skipping superuser test while running as user") } - ctx, cancel := context.WithTimeout(context.Background(), 1*time.Second) + ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second) defer cancel() startTime, err := GetStartTime(ctx, "nginx", Options{UserMode: false}) if err != nil { t.Errorf("issue getting start time of nginx: %v", err) } + time.Sleep(1 * time.Second) err = Restart(ctx, "nginx", Options{UserMode: false}) if err != nil { t.Errorf("issue restarting nginx as %s: %v", userString, err) } + time.Sleep(100 * time.Millisecond) newStartTime, err := GetStartTime(ctx, "nginx", Options{UserMode: false}) if err != nil { t.Errorf("issue getting second start time of nginx: %v", err) diff --git a/systemctl.go b/systemctl.go index e779904..83927a2 100644 --- a/systemctl.go +++ b/systemctl.go @@ -144,6 +144,9 @@ func IsFailed(ctx context.Context, unit string, opts Options) (bool, error) { // Mask one or more units, as specified on the command line. This will link // these unit files to /dev/null, making it impossible to start them. +// +// Notably, Mask may return ErrDoesNotExist if a unit doesn't exist, but it will +// ocntinue masking anyway. func Mask(ctx context.Context, unit string, opts Options) error { var args = []string{"mask", "--system", unit} if opts.UserMode { @@ -213,6 +216,11 @@ func Stop(ctx context.Context, unit string, opts Options) error { // Unmask one or more unit files, as specified on the command line. // This will undo the effect of Mask. +// +// In line with systemd, Unmask will return ErrDoesNotExist if the unit +// doesn't exist, but only if it's not already masked. +// If the unit doesn't exist but it's masked anyway, no error will be +// returned. Gross, I know. Take it up with Poettering. func Unmask(ctx context.Context, unit string, opts Options) error { var args = []string{"unmask", "--system", unit} if opts.UserMode { diff --git a/systemctl_test.go b/systemctl_test.go index c42461e..538bf73 100644 --- a/systemctl_test.go +++ b/systemctl_test.go @@ -23,7 +23,6 @@ var userString string func TestMain(m *testing.M) { curUser, err := user.Current() - if err != nil { fmt.Println("Could not determine running user") } @@ -32,50 +31,73 @@ func TestMain(m *testing.M) { fmt.Println("Don't forget to run both root and user tests.") os.Exit(m.Run()) } - -func TestEnable(t *testing.T) { +func TestDaemonReload(t *testing.T) { testCases := []struct { unit string err error opts Options runAsUser bool }{ - // Run these tests only as a user + /* 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}, + // fail to reload system daemon as user + {"", ErrInsufficientPermissions, Options{UserMode: false}, true}, + // reload user's scope daemon + {"", nil, Options{UserMode: true}, true}, + /* End user tests*/ - // Run these tests only as a superuser + /* 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}, + // succeed to reload daemon + {"", nil, Options{UserMode: false}, false}, + // fail to connect to user bus as system + {"", ErrBusFailure, Options{UserMode: true}, false}, + + /* End superuser tests*/ } for _, tc := range testCases { t.Run(fmt.Sprintf("%s as %s", tc.unit, userString), func(t *testing.T) { - t.Parallel() 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(), 10*time.Second) + ctx, cancel := context.WithTimeout(context.Background(), 1*time.Second) defer cancel() - err := Enable(ctx, tc.unit, tc.opts) + err := DaemonReload(ctx, tc.opts) if err != tc.err { t.Errorf("error is %v, but should have been %v", err, tc.err) } }) } +} +func TestDisable(t *testing.T) { + t.Run(fmt.Sprintf(""), func(t *testing.T) { + if userString != "root" && userString != "system" { + t.Skip("skipping superuser test while running as user") + } + unit := "nginx" + ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second) + err := Mask(ctx, unit, Options{UserMode: false}) + defer cancel() + if err != nil { + Unmask(ctx, unit, Options{UserMode: false}) + t.Errorf("Unable to mask %s", unit) + } + err = Disable(ctx, unit, Options{UserMode: false}) + if err != ErrMasked { + Unmask(ctx, unit, Options{UserMode: false}) + t.Errorf("error is %v, but should have been %v", err, ErrMasked) + } + err = Unmask(ctx, unit, Options{UserMode: false}) + if err != nil { + t.Errorf("Unable to unmask %s", unit) + } + }) + +} +func TestEnable(t *testing.T) { + t.Run(fmt.Sprintf(""), func(t *testing.T) { if userString != "root" && userString != "system" { t.Skip("skipping superuser test while running as user") @@ -122,75 +144,40 @@ func ExampleEnable() { } } -func TestDisable(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}, - - /* 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*/ - } - 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 := Disable(ctx, tc.unit, tc.opts) - if err != tc.err { - t.Errorf("error is %v, but should have been %v", err, tc.err) - } - }) - } - t.Run(fmt.Sprintf(""), func(t *testing.T) { +func TestIsActive(t *testing.T) { + unit := "nginx" + t.Run(fmt.Sprintf("check active"), func(t *testing.T) { if userString != "root" && userString != "system" { t.Skip("skipping superuser test while running as user") } - unit := "nginx" ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second) - err := Mask(ctx, unit, Options{UserMode: false}) defer cancel() + err := Start(ctx, unit, Options{UserMode: false}) if err != nil { - Unmask(ctx, unit, Options{UserMode: false}) - t.Errorf("Unable to mask %s", unit) + t.Errorf("Unable to restart %s", unit) } - err = Disable(ctx, unit, Options{UserMode: false}) - if err != ErrMasked { - Unmask(ctx, unit, Options{UserMode: false}) - t.Errorf("error is %v, but should have been %v", err, ErrMasked) - } - err = Unmask(ctx, unit, Options{UserMode: false}) - if err != nil { - t.Errorf("Unable to unmask %s", unit) + time.Sleep(time.Second) + isActive, err := IsActive(ctx, unit, Options{UserMode: false}) + if !isActive { + t.Errorf("IsActive didn't return true for %s", unit) } }) - + t.Run(fmt.Sprintf("check masked"), func(t *testing.T) { + if userString != "root" && userString != "system" { + t.Skip("skipping superuser test while running as user") + } + ctx, cancel := context.WithTimeout(context.Background(), 1*time.Second) + defer cancel() + err := Mask(ctx, unit, Options{UserMode: false}) + if err != nil { + t.Errorf("Unable to mask %s", unit) + } + _, err = IsActive(ctx, unit, Options{UserMode: false}) + if err != nil { + t.Errorf("error is %v, but should have been %v", err, nil) + } + Unmask(ctx, unit, Options{UserMode: false}) + }) } // Runs through all defined Properties in parallel and checks for error cases diff --git a/util.go b/util.go index d8d0ce4..03f5cec 100644 --- a/util.go +++ b/util.go @@ -53,6 +53,12 @@ func filterErr(stderr string) error { if matched, _ := regexp.MatchString(`does not exist`, stderr); matched { return ErrDoesNotExist } + if matched, _ := regexp.MatchString(`not found.`, stderr); matched { + return ErrDoesNotExist + } + if matched, _ := regexp.MatchString(`not loaded.`, stderr); matched { + return ErrUnitNotLoaded + } if matched, _ := regexp.MatchString(`No such file or directory`, stderr); matched { return ErrDoesNotExist }