update to fix some tests, remove panics, and wrap errors

Author: taigrr

README.md

@@ -5,7 +5,6 @@ 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, thoroughly-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
 

errors.go

@@ -21,7 +21,6 @@ var (
 	// Masked units can only be unmasked, but something else was attempted
 	// Unmask the unit before enabling or disabling it
 	ErrMasked = errors.New("unit masked")
-	// 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

errors_test.go

@@ -2,6 +2,7 @@ package systemctl
 
 import (
 	"context"
+	"errors"
 	"fmt"
 	"reflect"
 	"runtime"
@@ -63,7 +64,7 @@ func TestErrorFuncs(t *testing.T) {
 					ctx, cancel := context.WithTimeout(context.Background(), 1*time.Second)
 					defer cancel()
 					err := f(ctx, tc.unit, tc.opts)
-					if err != tc.err {
+					if !errors.Is(err, tc.err) {
 						t.Errorf("error is %v, but should have been %v", err, tc.err)
 					}
 				})

go.mod

@@ -1,3 +1,3 @@
 module github.com/taigrr/systemctl
 
-go 1.17
+go 1.12

helpers_test.go

@@ -2,6 +2,7 @@ package systemctl
 
 import (
 	"context"
+	"errors"
 	"fmt"
 	"syscall"
 	"testing"
@@ -58,13 +59,13 @@ func TestGetStartTime(t *testing.T) {
 			ctx, cancel := context.WithTimeout(context.Background(), 1*time.Second)
 			defer cancel()
 			_, err := GetStartTime(ctx, tc.unit, tc.opts)
-			if err != tc.err {
+			if !errors.Is(err, tc.err) {
 				t.Errorf("error is %v, but should have been %v", err, tc.err)
 			}
 		})
 	}
 	// Prove start time changes after a restart
-	t.Run(fmt.Sprintf("prove start time changes"), func(t *testing.T) {
+	t.Run("prove start time changes", func(t *testing.T) {
 		if userString != "root" && userString != "system" {
 			t.Skip("skipping superuser test while running as user")
 		}
@@ -93,12 +94,13 @@ func TestGetStartTime(t *testing.T) {
 }
 
 func TestGetNumRestarts(t *testing.T) {
-	testCases := []struct {
+	type testCase struct {
 		unit      string
 		err       error
 		opts      Options
 		runAsUser bool
-	}{
+	}
+	testCases := []testCase{
 		// Run these tests only as a user
 
 		// try nonexistant unit in user mode as user
@@ -118,23 +120,25 @@ func TestGetNumRestarts(t *testing.T) {
 		{"nginx", nil, Options{UserMode: false}, false},
 	}
 	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(), 1*time.Second)
-			defer cancel()
-			_, err := GetNumRestarts(ctx, tc.unit, tc.opts)
-			if err != tc.err {
-				t.Errorf("error is %v, but should have been %v", err, tc.err)
-			}
-		})
+		func(tc testCase) {
+			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(), 1*time.Second)
+				defer cancel()
+				_, err := GetNumRestarts(ctx, tc.unit, tc.opts)
+				if !errors.Is(err, tc.err) {
+					t.Errorf("error is %v, but should have been %v", err, tc.err)
+				}
+			})
+		}(tc)
 	}
 	// Prove restart count increases by one after a restart
-	t.Run(fmt.Sprintf("prove restart count increases by one after a restart"), func(t *testing.T) {
+	t.Run("prove restart count increases by one after a restart", func(t *testing.T) {
 		if testing.Short() {
 			t.Skip("skipping in short mode")
 		}
@@ -154,9 +158,9 @@ func TestGetNumRestarts(t *testing.T) {
 		}
 		syscall.Kill(pid, syscall.SIGKILL)
 		for {
-			running, err := IsActive(ctx, "nginx", Options{UserMode: false})
-			if err != nil {
-				t.Errorf("error asserting nginx is up: %v", err)
+			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
@@ -173,12 +177,13 @@ func TestGetNumRestarts(t *testing.T) {
 }
 
 func TestGetMemoryUsage(t *testing.T) {
-	testCases := []struct {
+	type testCase struct {
 		unit      string
 		err       error
 		opts      Options
 		runAsUser bool
-	}{
+	}
+	testCases := []testCase{
 		// Run these tests only as a user
 
 		// try nonexistant unit in user mode as user
@@ -198,23 +203,25 @@ func TestGetMemoryUsage(t *testing.T) {
 		{"nginx", nil, Options{UserMode: false}, false},
 	}
 	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(), 1*time.Second)
-			defer cancel()
-			_, err := GetMemoryUsage(ctx, tc.unit, tc.opts)
-			if err != tc.err {
-				t.Errorf("error is %v, but should have been %v", err, tc.err)
-			}
-		})
+		func(tc testCase) {
+			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(), 1*time.Second)
+				defer cancel()
+				_, err := GetMemoryUsage(ctx, tc.unit, tc.opts)
+				if !errors.Is(err, tc.err) {
+					t.Errorf("error is %v, but should have been %v", err, tc.err)
+				}
+			})
+		}(tc)
 	}
 	// Prove memory usage values change across services
-	t.Run(fmt.Sprintf("prove memory usage values change across services"), func(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})
@@ -232,12 +239,14 @@ func TestGetMemoryUsage(t *testing.T) {
 }
 
 func TestGetPID(t *testing.T) {
-	testCases := []struct {
+	type testCase struct {
 		unit      string
 		err       error
 		opts      Options
 		runAsUser bool
-	}{
+	}
+
+	testCases := []testCase{
 		// Run these tests only as a user
 
 		// try nonexistant unit in user mode as user
@@ -257,22 +266,24 @@ func TestGetPID(t *testing.T) {
 		{"nginx", nil, Options{UserMode: false}, false},
 	}
 	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(), 1*time.Second)
-			defer cancel()
-			_, err := GetPID(ctx, tc.unit, tc.opts)
-			if err != tc.err {
-				t.Errorf("error is %v, but should have been %v", err, tc.err)
-			}
-		})
+		func(tc testCase) {
+			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(), 1*time.Second)
+				defer cancel()
+				_, err := GetPID(ctx, tc.unit, tc.opts)
+				if !errors.Is(err, tc.err) {
+					t.Errorf("error is %v, but should have been %v", err, tc.err)
+				}
+			})
+		}(tc)
 	}
-	t.Run(fmt.Sprintf("prove pid changes"), func(t *testing.T) {
+	t.Run("prove pid changes", func(t *testing.T) {
 		if testing.Short() {
 			t.Skip("skipping in short mode")
 		}