diff --git a/pexec/managed_process_test.go b/pexec/managed_process_test.go index e9557e01..3949fcf1 100644 --- a/pexec/managed_process_test.go +++ b/pexec/managed_process_test.go @@ -4,6 +4,7 @@ import ( "bufio" "bytes" "context" + "errors" "fmt" "io" "os" @@ -18,8 +19,6 @@ import ( "time" "github.com/edaniels/golog" - "github.com/fsnotify/fsnotify" - "github.com/pkg/errors" "go.viam.com/test" "go.viam.com/utils" @@ -72,13 +71,8 @@ func TestManagedProcessStart(t *testing.T) { }) t.Run("starting with an eventually canceled context should fail", func(t *testing.T) { logger := golog.NewTestLogger(t) - tempFile := testutils.TempFile(t, "something.txt") - defer tempFile.Close() - watcher, err := fsnotify.NewWatcher() - test.That(t, err, test.ShouldBeNil) - defer watcher.Close() - watcher.Add(tempFile.Name()) + watcher, tempFile := testutils.WatchedFile(t) ctx, cancel := context.WithCancel(context.Background()) go func() { @@ -92,7 +86,7 @@ func TestManagedProcessStart(t *testing.T) { OneShot: true, Log: true, }, logger) - err = proc.Start(ctx) + err := proc.Start(ctx) test.That(t, err, test.ShouldNotBeNil) if runtime.GOOS == "windows" { test.That(t, err.Error(), test.ShouldContainSubstring, "exit status 1") @@ -103,8 +97,8 @@ func TestManagedProcessStart(t *testing.T) { t.Run("starting with a normal context", func(t *testing.T) { logger := golog.NewTestLogger(t) - tempFile := testutils.TempFile(t, "something.txt") - defer tempFile.Close() + tempFile := testutils.TempFile(t) + proc := NewManagedProcess(ProcessConfig{ Name: "bash", Args: []string{"-c", fmt.Sprintf(`echo hello >> '%s'`, tempFile.Name())}, @@ -129,9 +123,6 @@ func TestManagedProcessStart(t *testing.T) { }) t.Run("OnUnexpectedExit is ignored", func(t *testing.T) { logger := golog.NewTestLogger(t) - - tempFile := testutils.TempFile(t, "something.txt") - defer tempFile.Close() proc := NewManagedProcess(ProcessConfig{ Name: "bash", Args: []string{"-c", "exit 1"}, @@ -175,13 +166,7 @@ func TestManagedProcessStart(t *testing.T) { t.Run("starting with a normal context should run until stop", func(t *testing.T) { logger := golog.NewTestLogger(t) - tempFile := testutils.TempFile(t, "something.txt") - defer tempFile.Close() - - watcher, err := fsnotify.NewWatcher() - test.That(t, err, test.ShouldBeNil) - defer watcher.Close() - watcher.Add(tempFile.Name()) + watcher, tempFile := testutils.WatchedFile(t) proc := NewManagedProcess(ProcessConfig{ Name: "bash", @@ -238,13 +223,7 @@ func TestManagedProcessManage(t *testing.T) { t.Run("a managed process that dies should be restarted", func(t *testing.T) { logger := golog.NewTestLogger(t) - tempFile := testutils.TempFile(t, "something.txt") - defer tempFile.Close() - - watcher, err := fsnotify.NewWatcher() - test.That(t, err, test.ShouldBeNil) - defer watcher.Close() - watcher.Add(tempFile.Name()) + watcher, tempFile := testutils.WatchedFile(t) proc := NewManagedProcess(ProcessConfig{ Name: "bash", @@ -256,7 +235,7 @@ func TestManagedProcessManage(t *testing.T) { <-watcher.Events <-watcher.Events - err = proc.Stop() + err := proc.Stop() // sometimes we simply cannot get the status if err != nil { test.That(t, err.Error(), test.ShouldContainSubstring, "exit status 1") @@ -329,13 +308,7 @@ func TestManagedProcessStop(t *testing.T) { t.Run("stopping a managed process gives it a chance to finish", func(t *testing.T) { logger := golog.NewTestLogger(t) - tempFile := testutils.TempFile(t, "something.txt") - defer tempFile.Close() - - watcher, err := fsnotify.NewWatcher() - test.That(t, err, test.ShouldBeNil) - defer watcher.Close() - watcher.Add(tempFile.Name()) + watcher, tempFile := testutils.WatchedFile(t) proc := NewManagedProcess(ProcessConfig{ Name: "bash", @@ -366,7 +339,7 @@ func TestManagedProcessStop(t *testing.T) { <-watcher.Events test.That(t, proc.Status(), test.ShouldBeNil) - err = proc.Stop() + err := proc.Stop() if runtime.GOOS == "windows" { test.That(t, err, test.ShouldBeNil) } else { @@ -396,13 +369,7 @@ func TestManagedProcessStop(t *testing.T) { } logger := golog.NewTestLogger(t) - tempFile := testutils.TempFile(t, "something.txt") - defer tempFile.Close() - - watcher, err := fsnotify.NewWatcher() - test.That(t, err, test.ShouldBeNil) - defer watcher.Close() - watcher.Add(tempFile.Name()) + watcher, tempFile := testutils.WatchedFile(t) var bashScriptBuilder strings.Builder for _, sig := range knownSignals { @@ -428,7 +395,7 @@ done`, tempFile.Name())) test.That(t, proc.Start(context.Background()), test.ShouldBeNil) <-watcher.Events test.That(t, proc.Status(), test.ShouldBeNil) - err = proc.Stop() + err := proc.Stop() test.That(t, err, test.ShouldNotBeNil) test.That(t, err.Error(), test.ShouldContainSubstring, "exit status 115") test.That(t, proc.Status(), test.ShouldNotBeNil) @@ -454,13 +421,7 @@ done`, tempFile.Name())) } logger := golog.NewTestLogger(t) - tempFile := testutils.TempFile(t, "something.txt") - defer tempFile.Close() - - watcher1, err := fsnotify.NewWatcher() - test.That(t, err, test.ShouldBeNil) - defer watcher1.Close() - watcher1.Add(tempFile.Name()) + watcher1, tempFile := testutils.WatchedFile(t) bashScript1 := fmt.Sprintf(` trap "echo SIGTERM >> '%s'" SIGTERM @@ -499,27 +460,9 @@ done`, tempFile.Name())) } logger := golog.NewTestLogger(t) - tempFile1 := testutils.TempFile(t, "something.txt") - defer tempFile1.Close() - tempFile2 := testutils.TempFile(t, "something.txt") - defer tempFile2.Close() - tempFile3 := testutils.TempFile(t, "something.txt") - defer tempFile3.Close() - - watcher1, err := fsnotify.NewWatcher() - test.That(t, err, test.ShouldBeNil) - defer watcher1.Close() - watcher1.Add(tempFile1.Name()) - - watcher2, err := fsnotify.NewWatcher() - test.That(t, err, test.ShouldBeNil) - defer watcher2.Close() - watcher2.Add(tempFile2.Name()) - - watcher3, err := fsnotify.NewWatcher() - test.That(t, err, test.ShouldBeNil) - defer watcher3.Close() - watcher3.Add(tempFile3.Name()) + watcher1, tempFile1 := testutils.WatchedFile(t) + watcher2, tempFile2 := testutils.WatchedFile(t) + watcher3, tempFile3 := testutils.WatchedFile(t) trapScript := ` trap "echo SIGTERM >> '%s'" SIGTERM @@ -580,13 +523,7 @@ done`, tempFile.Name())) } logger := golog.NewTestLogger(t) - tempFile := testutils.TempFile(t, "something.txt") - defer tempFile.Close() - - watcher1, err := fsnotify.NewWatcher() - test.That(t, err, test.ShouldBeNil) - defer watcher1.Close() - watcher1.Add(tempFile.Name()) + watcher1, tempFile := testutils.WatchedFile(t) proc := NewManagedProcess(ProcessConfig{ Name: "bash", diff --git a/pexec/process_manager_test.go b/pexec/process_manager_test.go index 2b703d05..3cdfd4f2 100644 --- a/pexec/process_manager_test.go +++ b/pexec/process_manager_test.go @@ -8,7 +8,6 @@ import ( "testing" "github.com/edaniels/golog" - "github.com/fsnotify/fsnotify" "go.viam.com/test" "go.viam.com/utils" @@ -190,11 +189,9 @@ func TestProcessManagerStart(t *testing.T) { test.That(t, pm.Start(context.Background()), test.ShouldBeNil) t.Run("adding a process after starting starts it", func(t *testing.T) { - temp, err := os.CreateTemp("", "*.txt") - test.That(t, err, test.ShouldBeNil) - defer os.Remove(temp.Name()) + temp := testutils.TempFile(t) - _, err = pm.AddProcessFromConfig(context.Background(), + _, err := pm.AddProcessFromConfig(context.Background(), ProcessConfig{ ID: "1", Name: "bash", @@ -232,16 +229,10 @@ func TestProcessManagerStart(t *testing.T) { // a "timed" ctx should only have an effect on one shots ctx, cancel = context.WithCancel(context.Background()) - tempFile1 := testutils.TempFile(t, "something.txt") - defer tempFile1.Close() - tempFile2 := testutils.TempFile(t, "something.txt") - defer tempFile2.Close() + watcher, tempFiles := testutils.WatchedFiles(t, 2) + tempFile1 := tempFiles[0] + tempFile2 := tempFiles[1] - watcher, err := fsnotify.NewWatcher() - test.That(t, err, test.ShouldBeNil) - defer watcher.Close() - watcher.Add(tempFile1.Name()) - watcher.Add(tempFile2.Name()) go func() { <-watcher.Events <-watcher.Events @@ -271,11 +262,9 @@ func TestProcessManagerStart(t *testing.T) { test.That(t, pm.Stop(), test.ShouldBeNil) }() - temp, err := os.CreateTemp("", "*.txt") - test.That(t, err, test.ShouldBeNil) - defer os.Remove(temp.Name()) + temp := testutils.TempFile(t) - _, err = pm.AddProcessFromConfig(context.Background(), + _, err := pm.AddProcessFromConfig(context.Background(), ProcessConfig{ ID: "1", Name: "bash", @@ -342,21 +331,12 @@ func TestProcessManagerStop(t *testing.T) { logger := golog.NewTestLogger(t) pm := NewProcessManager(logger) - tempFile1 := testutils.TempFile(t, "something.txt") - defer tempFile1.Close() - tempFile2 := testutils.TempFile(t, "something.txt") - defer tempFile2.Close() - tempFile3 := testutils.TempFile(t, "something.txt") - defer tempFile3.Close() + watcher, tempFiles := testutils.WatchedFiles(t, 3) + tempFile1 := tempFiles[0] + tempFile2 := tempFiles[1] + tempFile3 := tempFiles[2] - watcher, err := fsnotify.NewWatcher() - test.That(t, err, test.ShouldBeNil) - defer watcher.Close() - watcher.Add(tempFile1.Name()) - watcher.Add(tempFile2.Name()) - watcher.Add(tempFile3.Name()) - - _, err = pm.AddProcessFromConfig(context.Background(), ProcessConfig{ID: "1", Name: "bash", Args: []string{ + _, err := pm.AddProcessFromConfig(context.Background(), ProcessConfig{ID: "1", Name: "bash", Args: []string{ "-c", fmt.Sprintf("trap \"exit 0\" SIGTERM; echo one >> '%s'\nwhile true; do echo hey1; sleep 1; done", tempFile1.Name()), }}) test.That(t, err, test.ShouldBeNil) @@ -396,18 +376,11 @@ func TestProcessManagerStop(t *testing.T) { pm := NewProcessManager(logger) test.That(t, pm.Start(context.Background()), test.ShouldBeNil) - tempFile1 := testutils.TempFile(t, "something.txt") - defer tempFile1.Close() - tempFile2 := testutils.TempFile(t, "something.txt") - defer tempFile2.Close() - - watcher, err := fsnotify.NewWatcher() - test.That(t, err, test.ShouldBeNil) - defer watcher.Close() - watcher.Add(tempFile1.Name()) - watcher.Add(tempFile2.Name()) + watcher, tempFiles := testutils.WatchedFiles(t, 2) + tempFile1 := tempFiles[0] + tempFile2 := tempFiles[1] - _, err = pm.AddProcessFromConfig(context.Background(), ProcessConfig{ID: "1", Name: "bash", Args: []string{ + _, err := pm.AddProcessFromConfig(context.Background(), ProcessConfig{ID: "1", Name: "bash", Args: []string{ "-c", fmt.Sprintf( "trap \"echo done >> '%[1]s';exit 0\" SIGTERM; echo one >> '%[1]s'\nwhile true; do echo hey1; sleep 1; done", diff --git a/testutils/file.go b/testutils/file.go index cf723a01..7eda5aca 100644 --- a/testutils/file.go +++ b/testutils/file.go @@ -6,6 +6,7 @@ import ( "path/filepath" "testing" + "github.com/fsnotify/fsnotify" "go.viam.com/test" ) @@ -30,12 +31,64 @@ func TempDir(dir, pattern string) (string, error) { return dir, err } -// TempFile returns a new unique temporary file using the given name. -func TempFile(tb testing.TB, name string) *os.File { +// TempFile returns a unique temporary file named "something.txt" or fails the test if it +// cannot. It automatically closes and removes the file after the test and all its +// subtests complete. +func TempFile(tb testing.TB) *os.File { tb.Helper() - tempFile := filepath.Join(tb.TempDir(), name) + tempFile := filepath.Join(tb.TempDir(), "something.txt") //nolint:gosec f, err := os.Create(tempFile) test.That(tb, err, test.ShouldBeNil) + + tb.Cleanup(func() { + test.That(tb, f.Close(), test.ShouldBeNil) + // Since the file was placed in a directory that was created via TB.TempDir, it + // will automatically be deleted after the test and all its subtests complete, so + // we do not need to remove it manually. + }) + return f } + +// WatchedFiles creates a file watcher and n unique temporary files all named +// "something.txt", or fails the test if it cannot. It returns the watcher and a slice of +// files. It automatically closes the watcher, and closes and removes all files after the +// test and all its subtests complete. +// +// For safety, this function will not create more than 50 files. +func WatchedFiles(tb testing.TB, n int) (*fsnotify.Watcher, []*os.File) { + tb.Helper() + + if n > 50 { + tb.Fatal("will not create more than 50 temporary files, sorry") + } + + watcher, err := fsnotify.NewWatcher() + test.That(tb, err, test.ShouldBeNil) + tb.Cleanup(func() { + test.That(tb, watcher.Close(), test.ShouldBeNil) + }) + + var tempFiles []*os.File + for i := 0; i < n; i++ { + f := TempFile(tb) + tempFiles = append(tempFiles, f) + test.That(tb, watcher.Add(f.Name()), test.ShouldBeNil) + } + + return watcher, tempFiles +} + +// WatchedFile creates a file watcher and a unique temporary file named "something.txt", +// or fails the test if it cannot. It returns the watcher and the file. It automatically +// closes the watcher, and closes and removes the file after the test and all its +// subtests complete. +func WatchedFile(tb testing.TB) (*fsnotify.Watcher, *os.File) { + tb.Helper() + + watcher, tempFiles := WatchedFiles(tb, 1) + test.That(tb, len(tempFiles), test.ShouldEqual, 1) + + return watcher, tempFiles[0] +}