From 296c0028e91283c40b36d0fa54f9c7ae4c98cc52 Mon Sep 17 00:00:00 2001 From: Maksim An Date: Fri, 29 Apr 2022 17:18:17 -0700 Subject: [PATCH 1/2] tests: add tests for wait-paths Follow up PR to add tests for wait-paths after initial PR #1258 was merged. Signed-off-by: Maksim An --- cmd/hooks/wait-paths/main.go | 21 ++-- cmd/hooks/wait-paths/wait_paths_test.go | 136 ++++++++++++++++++++++++ 2 files changed, 151 insertions(+), 6 deletions(-) create mode 100644 cmd/hooks/wait-paths/wait_paths_test.go diff --git a/cmd/hooks/wait-paths/main.go b/cmd/hooks/wait-paths/main.go index c3c49ccf9a..dd73c63c93 100644 --- a/cmd/hooks/wait-paths/main.go +++ b/cmd/hooks/wait-paths/main.go @@ -1,9 +1,8 @@ -// +build linux - package main import ( "context" + "errors" "fmt" "os" "strings" @@ -22,6 +21,14 @@ const ( // The hook has required list of comma-separated paths and a default timeout in seconds. func main() { + app := newCliApp() + if err := app.Run(os.Args); err != nil { + logrus.Fatalf("%s\n", err) + } + os.Exit(0) +} + +func newCliApp() *cli.App { app := cli.NewApp() app.Name = "wait-paths" app.Usage = "Provide a list paths and an optional timeout" @@ -38,14 +45,16 @@ func main() { }, } app.Action = run - if err := app.Run(os.Args); err != nil { - logrus.Fatalf("%s\n", err) - } - os.Exit(0) + return app } func run(cCtx *cli.Context) error { timeout := cCtx.GlobalInt(timeoutFlag) + + pathsVal := cCtx.GlobalString(pathsFlag) + if pathsVal == "" { + return errors.New("paths cannot be empty") + } paths := strings.Split(cCtx.GlobalString(pathsFlag), ",") waitCtx, cancel := context.WithTimeout(context.Background(), time.Duration(timeout)*time.Second) diff --git a/cmd/hooks/wait-paths/wait_paths_test.go b/cmd/hooks/wait-paths/wait_paths_test.go new file mode 100644 index 0000000000..df93f60f0f --- /dev/null +++ b/cmd/hooks/wait-paths/wait_paths_test.go @@ -0,0 +1,136 @@ +package main + +import ( + "math" + "os" + "path/filepath" + "strings" + "testing" + "time" +) + +func Test_Paths_EmptyString_NotAllowed(t *testing.T) { + args := []string{ + "wait-paths", + "-p", + "", + } + app := newCliApp() + err := app.Run(args) + if err == nil { + t.Fatal("expected error, got nothing") + } + if !strings.Contains(err.Error(), "cannot be empty") { + t.Fatalf("expected 'cannot be empty' error, got: %s", err) + } +} + +func Test_InvalidWaitPath_DefaultTimeout(t *testing.T) { + args := []string{ + "wait-paths", + "-p", + "non-existent", + } + app := newCliApp() + err := app.Run(args) + if err == nil { + t.Fatalf("expected error, got nothing") + } + if !strings.Contains(err.Error(), "timeout while waiting") { + t.Fatalf("expected 'timeout error', got: %s", err) + } +} + +func Test_InvalidWaitPath_5SecondTimeout(t *testing.T) { + args := []string{ + "wait-paths", + "-p", + "non-existent", + "-t", + "5", + } + app := newCliApp() + start := time.Now() + err := app.Run(args) + if err == nil { + t.Fatal("expected timeout error, got nothing") + } + if !strings.Contains(err.Error(), "timeout while waiting") { + t.Fatalf("expected 'timeout error', got: %s", err) + } + + end := time.Now() + diff := end.Sub(start) + diffSeconds := math.Round(diff.Seconds()) + if diffSeconds != 5 { + t.Fatalf("expected 5 second timeout, got: %f", diffSeconds) + } +} + +func Test_Valid_Paths_AlreadyPresent(t *testing.T) { + tmpDir := t.TempDir() + var files []string + for _, name := range []string{"file1", "file2"} { + filePath := filepath.Join(tmpDir, name) + if f, err := os.Create(filePath); err != nil { + t.Fatalf("failed to create temporary file %s: %s", name, err) + } else { + _ = f.Close() + } + files = append(files, filePath) + } + defer func() { + _ = os.RemoveAll(tmpDir) + }() + pathsArg := strings.Join(files, ",") + + args := []string{ + "wait-paths", + "-p", + pathsArg, + } + app := newCliApp() + if err := app.Run(args); err != nil { + t.Fatalf("expected no error, got: %s", err) + } +} + +func Test_Valid_Paths_BecomeAvailableLater(t *testing.T) { + tmpDir := t.TempDir() + var files []string + for _, name := range []string{"file1", "file2"} { + files = append(files, filepath.Join(tmpDir, name)) + } + pathsArg := strings.Join(files, ",") + defer func() { + _ = os.RemoveAll(tmpDir) + }() + + errChan := make(chan error) + args := []string{ + "wait-paths", + "-p", + pathsArg, + } + app := newCliApp() + go func() { + errChan <- app.Run(args) + }() + + go func() { + time.Sleep(5 * time.Second) + for _, fileName := range files { + if f, err := os.Create(fileName); err != nil { + errChan <- err + return + } else { + _ = f.Close() + } + } + }() + + if err := <-errChan; err != nil { + close(errChan) + t.Fatalf("expected no error, got: %s", err) + } +} From c2975a43439257e1046184ef4db1525cbc11a023 Mon Sep 17 00:00:00 2001 From: Maksim An Date: Mon, 2 May 2022 17:58:29 -0700 Subject: [PATCH 2/2] pr feedback Signed-off-by: Maksim An --- cmd/hooks/wait-paths/main.go | 6 +++-- cmd/hooks/wait-paths/wait_paths_test.go | 33 +++++++++++-------------- 2 files changed, 19 insertions(+), 20 deletions(-) diff --git a/cmd/hooks/wait-paths/main.go b/cmd/hooks/wait-paths/main.go index dd73c63c93..e821f4d470 100644 --- a/cmd/hooks/wait-paths/main.go +++ b/cmd/hooks/wait-paths/main.go @@ -17,6 +17,8 @@ const ( timeoutFlag = "timeout" ) +var errEmptyPaths = errors.New("paths cannot be empty") + // This is a hook that waits for a specific path to appear. // The hook has required list of comma-separated paths and a default timeout in seconds. @@ -53,7 +55,7 @@ func run(cCtx *cli.Context) error { pathsVal := cCtx.GlobalString(pathsFlag) if pathsVal == "" { - return errors.New("paths cannot be empty") + return errEmptyPaths } paths := strings.Split(cCtx.GlobalString(pathsFlag), ",") @@ -68,7 +70,7 @@ func run(cCtx *cli.Context) error { } select { case <-waitCtx.Done(): - return fmt.Errorf("timeout while waiting for path %q to appear", path) + return fmt.Errorf("timeout while waiting for path %q to appear: %w", path, context.DeadlineExceeded) default: time.Sleep(time.Millisecond * 10) continue diff --git a/cmd/hooks/wait-paths/wait_paths_test.go b/cmd/hooks/wait-paths/wait_paths_test.go index df93f60f0f..499aa7ed57 100644 --- a/cmd/hooks/wait-paths/wait_paths_test.go +++ b/cmd/hooks/wait-paths/wait_paths_test.go @@ -1,10 +1,13 @@ package main import ( + "context" + "errors" "math" "os" "path/filepath" "strings" + "sync" "testing" "time" ) @@ -17,10 +20,7 @@ func Test_Paths_EmptyString_NotAllowed(t *testing.T) { } app := newCliApp() err := app.Run(args) - if err == nil { - t.Fatal("expected error, got nothing") - } - if !strings.Contains(err.Error(), "cannot be empty") { + if !errors.Is(err, errEmptyPaths) { t.Fatalf("expected 'cannot be empty' error, got: %s", err) } } @@ -33,10 +33,7 @@ func Test_InvalidWaitPath_DefaultTimeout(t *testing.T) { } app := newCliApp() err := app.Run(args) - if err == nil { - t.Fatalf("expected error, got nothing") - } - if !strings.Contains(err.Error(), "timeout while waiting") { + if cause := errors.Unwrap(err); !errors.Is(cause, context.DeadlineExceeded) { t.Fatalf("expected 'timeout error', got: %s", err) } } @@ -52,10 +49,7 @@ func Test_InvalidWaitPath_5SecondTimeout(t *testing.T) { app := newCliApp() start := time.Now() err := app.Run(args) - if err == nil { - t.Fatal("expected timeout error, got nothing") - } - if !strings.Contains(err.Error(), "timeout while waiting") { + if cause := errors.Unwrap(err); !errors.Is(cause, context.DeadlineExceeded) { t.Fatalf("expected 'timeout error', got: %s", err) } @@ -79,9 +73,6 @@ func Test_Valid_Paths_AlreadyPresent(t *testing.T) { } files = append(files, filePath) } - defer func() { - _ = os.RemoveAll(tmpDir) - }() pathsArg := strings.Join(files, ",") args := []string{ @@ -102,22 +93,29 @@ func Test_Valid_Paths_BecomeAvailableLater(t *testing.T) { files = append(files, filepath.Join(tmpDir, name)) } pathsArg := strings.Join(files, ",") + + errChan := make(chan error) + var wg sync.WaitGroup defer func() { - _ = os.RemoveAll(tmpDir) + wg.Wait() + close(errChan) }() - errChan := make(chan error) args := []string{ "wait-paths", "-p", pathsArg, } app := newCliApp() + wg.Add(1) go func() { + defer wg.Done() errChan <- app.Run(args) }() + wg.Add(1) go func() { + defer wg.Done() time.Sleep(5 * time.Second) for _, fileName := range files { if f, err := os.Create(fileName); err != nil { @@ -130,7 +128,6 @@ func Test_Valid_Paths_BecomeAvailableLater(t *testing.T) { }() if err := <-errChan; err != nil { - close(errChan) t.Fatalf("expected no error, got: %s", err) } }