logmon: refactor Logging tests

Mostly to use testify assertions and close open resources

client/logmon/logging/rotator_test.go

@@ -10,6 +10,7 @@ import (
 
 	"github.com/hashicorp/nomad/helper/testlog"
 	"github.com/hashicorp/nomad/testutil"
+	"github.com/stretchr/testify/require"
 )
 
 var (
@@ -19,168 +20,141 @@ var (
 
 func TestFileRotator_IncorrectPath(t *testing.T) {
 	t.Parallel()
-	if _, err := NewFileRotator("/foo", baseFileName, 10, 10, testlog.HCLogger(t)); err == nil {
-		t.Fatalf("expected error")
-	}
+
+	_, err := NewFileRotator("/foo", baseFileName, 10, 10, testlog.HCLogger(t))
+	require.Error(t, err)
+	require.Contains(t, err.Error(), "no such file or directory")
 }
 
 func TestFileRotator_CreateNewFile(t *testing.T) {
 	t.Parallel()
-	var path string
-	var err error
-	if path, err = ioutil.TempDir("", pathPrefix); err != nil {
-		t.Fatalf("test setup err: %v", err)
-	}
+
+	path, err := ioutil.TempDir("", pathPrefix)
+	require.NoError(t, err)
 	defer os.RemoveAll(path)
 
-	_, err = NewFileRotator(path, baseFileName, 10, 10, testlog.HCLogger(t))
-	if err != nil {
-		t.Fatalf("test setup err: %v", err)
-	}
+	fr, err := NewFileRotator(path, baseFileName, 10, 10, testlog.HCLogger(t))
+	require.NoError(t, err)
+	defer fr.Close()
 
-	if _, err := os.Stat(filepath.Join(path, "redis.stdout.0")); err != nil {
-		t.Fatalf("expected file")
-	}
+	_, err = os.Stat(filepath.Join(path, "redis.stdout.0"))
+	require.NoError(t, err)
 }
 
 func TestFileRotator_OpenLastFile(t *testing.T) {
 	t.Parallel()
-	var path string
-	var err error
-	if path, err = ioutil.TempDir("", pathPrefix); err != nil {
-		t.Fatalf("test setup err: %v", err)
-	}
+
+	path, err := ioutil.TempDir("", pathPrefix)
+	require.NoError(t, err)
 	defer os.RemoveAll(path)
 
 	fname1 := filepath.Join(path, "redis.stdout.0")
 	fname2 := filepath.Join(path, "redis.stdout.2")
-	if _, err := os.Create(fname1); err != nil {
-		t.Fatalf("test setup failure: %v", err)
-	}
-	if _, err := os.Create(fname2); err != nil {
-		t.Fatalf("test setup failure: %v", err)
-	}
+
+	f1, err := os.Create(fname1)
+	require.NoError(t, err)
+	f1.Close()
+
+	f2, err := os.Create(fname2)
+	require.NoError(t, err)
+	f2.Close()
 
 	fr, err := NewFileRotator(path, baseFileName, 10, 10, testlog.HCLogger(t))
-	if err != nil {
-		t.Fatalf("test setup err: %v", err)
-	}
+	require.NoError(t, err)
+	defer fr.Close()
 
-	if fr.currentFile.Name() != fname2 {
-		t.Fatalf("expected current file: %v, got: %v", fname2, fr.currentFile.Name())
-	}
+	require.Equal(t, fname2, fr.currentFile.Name())
 }
 
 func TestFileRotator_WriteToCurrentFile(t *testing.T) {
 	t.Parallel()
-	var path string
-	var err error
-	if path, err = ioutil.TempDir("", pathPrefix); err != nil {
-		t.Fatalf("test setup err: %v", err)
-	}
+
+	path, err := ioutil.TempDir("", pathPrefix)
+	require.NoError(t, err)
 	defer os.RemoveAll(path)
 
 	fname1 := filepath.Join(path, "redis.stdout.0")
-	if _, err := os.Create(fname1); err != nil {
-		t.Fatalf("test setup failure: %v", err)
-	}
+	f1, err := os.Create(fname1)
+	require.NoError(t, err)
+	f1.Close()
 
 	fr, err := NewFileRotator(path, baseFileName, 10, 5, testlog.HCLogger(t))
-	if err != nil {
-		t.Fatalf("test setup err: %v", err)
-	}
+	require.NoError(t, err)
+	defer fr.Close()
 
 	fr.Write([]byte("abcde"))
 
-	var actual int64
 	testutil.WaitForResult(func() (bool, error) {
 		fi, err := os.Stat(fname1)
 		if err != nil {
-			return false, err
+			return false, fmt.Errorf("failed to stat file %v: %w", fname1, err)
 		}
-		actual = fi.Size()
+		actual := fi.Size()
 		if actual != 5 {
-			return false, nil
+			return false, fmt.Errorf("expected size %d but found %d", 5, actual)
 		}
 
 		return true, nil
 	}, func(err error) {
-		t.Fatalf("expected size: %v, actual: %v", 5, actual)
+		require.NoError(t, err)
 	})
 }
 
 func TestFileRotator_RotateFiles(t *testing.T) {
 	t.Parallel()
-	var path string
-	var err error
-	if path, err = ioutil.TempDir("", pathPrefix); err != nil {
-		t.Fatalf("test setup err: %v", err)
-	}
+
+	path, err := ioutil.TempDir("", pathPrefix)
+	require.NoError(t, err)
 	defer os.RemoveAll(path)
 
 	fr, err := NewFileRotator(path, baseFileName, 10, 5, testlog.HCLogger(t))
-	if err != nil {
-		t.Fatalf("test setup err: %v", err)
-	}
+	require.NoError(t, err)
+	defer fr.Close()
 
 	str := "abcdefgh"
 	nw, err := fr.Write([]byte(str))
-	if err != nil {
-		t.Fatalf("got error while writing: %v", err)
-	}
+	require.NoError(t, err)
+	require.Equal(t, len(str), nw)
 
-	if nw != len(str) {
-		t.Fatalf("expected %v, got %v", len(str), nw)
-	}
-
-	var lastErr error
 	testutil.WaitForResult(func() (bool, error) {
 		fname1 := filepath.Join(path, "redis.stdout.0")
 		fi, err := os.Stat(fname1)
 		if err != nil {
-			lastErr = err
-			return false, nil
+			return false, fmt.Errorf("failed to stat file %v: %w", fname1, err)
 		}
 		if fi.Size() != 5 {
-			lastErr = fmt.Errorf("expected size: %v, actual: %v", 5, fi.Size())
-			return false, nil
+			return false, fmt.Errorf("expected size: %v, actual: %v", 5, fi.Size())
 		}
 
 		fname2 := filepath.Join(path, "redis.stdout.1")
 		if _, err := os.Stat(fname2); err != nil {
-			lastErr = fmt.Errorf("expected file %v to exist", fname2)
-			return false, nil
+			return false, fmt.Errorf("expected file %v to exist", fname2)
 		}
 
 		if fi2, err := os.Stat(fname2); err == nil {
 			if fi2.Size() != 3 {
-				lastErr = fmt.Errorf("expected size: %v, actual: %v", 3, fi2.Size())
-				return false, nil
+				return false, fmt.Errorf("expected size: %v, actual: %v", 3, fi2.Size())
 			}
 		} else {
-			lastErr = fmt.Errorf("error getting the file info: %v", err)
-			return false, nil
+			return false, fmt.Errorf("error getting the file info: %v", err)
 		}
 
 		return true, nil
 	}, func(err error) {
-		t.Fatalf("%v", lastErr)
+		require.NoError(t, err)
 	})
 }
 
 func TestFileRotator_RotateFiles_Boundary(t *testing.T) {
 	t.Parallel()
-	var path string
-	var err error
-	if path, err = ioutil.TempDir("", pathPrefix); err != nil {
-		t.Fatalf("test setup err: %v", err)
-	}
+
+	path, err := ioutil.TempDir("", pathPrefix)
+	require.NoError(t, err)
 	defer os.RemoveAll(path)
 
 	fr, err := NewFileRotator(path, baseFileName, 10, 5, testlog.HCLogger(t))
-	if err != nil {
-		t.Fatalf("test setup err: %v", err)
-	}
+	require.NoError(t, err)
+	defer fr.Close()
 
 	// We will write three times:
 	// 1st: Write with new lines spanning two files
@@ -196,156 +170,120 @@ func TestFileRotator_RotateFiles_Boundary(t *testing.T) {
 
 	for _, str := range []string{"ab\ncdef\n", "1234567890", "\n"} {
 		nw, err := fr.Write([]byte(str))
-		if err != nil {
-			t.Fatalf("got error while writing: %v", err)
-		}
-
-		if nw != len(str) {
-			t.Fatalf("expected %v, got %v", len(str), nw)
-		}
+		require.NoError(t, err)
+		require.Equal(t, len(str), nw)
 	}
 
-	var lastErr error
 	testutil.WaitForResult(func() (bool, error) {
 
 		for i, exp := range expectations {
 			fname := filepath.Join(path, fmt.Sprintf("redis.stdout.%d", i))
 			fi, err := os.Stat(fname)
 			if err != nil {
-				lastErr = err
-				return false, nil
+				return false, fmt.Errorf("failed to stat file %v: %w", fname, err)
 			}
 			if int(fi.Size()) != len(exp) {
-				lastErr = fmt.Errorf("expected size: %v, actual: %v", len(exp), fi.Size())
-				return false, nil
+				return false, fmt.Errorf("expected size: %v, actual: %v", len(exp), fi.Size())
 			}
 		}
 
 		return true, nil
 	}, func(err error) {
-		t.Fatalf("%v", lastErr)
+		require.NoError(t, err)
 	})
 }
 
 func TestFileRotator_WriteRemaining(t *testing.T) {
 	t.Parallel()
-	var path string
-	var err error
-	if path, err = ioutil.TempDir("", pathPrefix); err != nil {
-		t.Fatalf("test setup err: %v", err)
-	}
+
+	path, err := ioutil.TempDir("", pathPrefix)
+	require.NoError(t, err)
 	defer os.RemoveAll(path)
 
 	fname1 := filepath.Join(path, "redis.stdout.0")
-	if f, err := os.Create(fname1); err == nil {
-		f.Write([]byte("abcd"))
-	} else {
-		t.Fatalf("test setup failure: %v", err)
-	}
+	err = ioutil.WriteFile(fname1, []byte("abcd"), 0600)
+	require.NoError(t, err)
 
 	fr, err := NewFileRotator(path, baseFileName, 10, 5, testlog.HCLogger(t))
-	if err != nil {
-		t.Fatalf("test setup err: %v", err)
-	}
+	require.NoError(t, err)
+	defer fr.Close()
 
 	str := "efghijkl"
 	nw, err := fr.Write([]byte(str))
-	if err != nil {
-		t.Fatalf("got error while writing: %v", err)
-	}
-	if nw != len(str) {
-		t.Fatalf("expected %v, got %v", len(str), nw)
-	}
-	var lastErr error
+	require.NoError(t, err)
+	require.Equal(t, len(str), nw)
+
 	testutil.WaitForResult(func() (bool, error) {
 		fi, err := os.Stat(fname1)
 		if err != nil {
-			lastErr = fmt.Errorf("error getting the file info: %v", err)
-			return false, nil
+			return false, fmt.Errorf("error getting the file info: %v", err)
 		}
 		if fi.Size() != 5 {
-			lastErr = fmt.Errorf("expected size: %v, actual: %v", 5, fi.Size())
-			return false, nil
+			return false, fmt.Errorf("expected size: %v, actual: %v", 5, fi.Size())
 		}
 
 		fname2 := filepath.Join(path, "redis.stdout.1")
 		if _, err := os.Stat(fname2); err != nil {
-			lastErr = fmt.Errorf("expected file %v to exist", fname2)
-			return false, nil
+			return false, fmt.Errorf("expected file %v to exist", fname2)
 		}
 
 		if fi2, err := os.Stat(fname2); err == nil {
 			if fi2.Size() != 5 {
-				lastErr = fmt.Errorf("expected size: %v, actual: %v", 5, fi2.Size())
-				return false, nil
+				return false, fmt.Errorf("expected size: %v, actual: %v", 5, fi2.Size())
 			}
 		} else {
-			lastErr = fmt.Errorf("error getting the file info: %v", err)
-			return false, nil
+			return false, fmt.Errorf("error getting the file info: %v", err)
 		}
 
 		fname3 := filepath.Join(path, "redis.stdout.2")
 		if _, err := os.Stat(fname3); err != nil {
-			lastErr = fmt.Errorf("expected file %v to exist", fname3)
-			return false, nil
+			return false, fmt.Errorf("expected file %v to exist", fname3)
 		}
 
 		if fi3, err := os.Stat(fname3); err == nil {
 			if fi3.Size() != 2 {
-				lastErr = fmt.Errorf("expected size: %v, actual: %v", 2, fi3.Size())
-				return false, nil
+				return false, fmt.Errorf("expected size: %v, actual: %v", 2, fi3.Size())
 			}
 		} else {
-			lastErr = fmt.Errorf("error getting the file info: %v", err)
-			return false, nil
+			return false, fmt.Errorf("error getting the file info: %v", err)
 		}
 
 		return true, nil
 	}, func(err error) {
-		t.Fatalf("%v", lastErr)
+		require.NoError(t, err)
 	})
 
 }
 
 func TestFileRotator_PurgeOldFiles(t *testing.T) {
 	t.Parallel()
-	var path string
-	var err error
-	if path, err = ioutil.TempDir("", pathPrefix); err != nil {
-		t.Fatalf("test setup err: %v", err)
-	}
+
+	path, err := ioutil.TempDir("", pathPrefix)
+	require.NoError(t, err)
 	defer os.RemoveAll(path)
 
 	fr, err := NewFileRotator(path, baseFileName, 2, 2, testlog.HCLogger(t))
-	if err != nil {
-		t.Fatalf("test setup err: %v", err)
-	}
+	require.NoError(t, err)
+	defer fr.Close()
 
 	str := "abcdeghijklmn"
 	nw, err := fr.Write([]byte(str))
-	if err != nil {
-		t.Fatalf("got error while writing: %v", err)
-	}
-	if nw != len(str) {
-		t.Fatalf("expected %v, got %v", len(str), nw)
-	}
+	require.NoError(t, err)
+	require.Equal(t, len(str), nw)
 
-	var lastErr error
 	testutil.WaitForResult(func() (bool, error) {
 		f, err := ioutil.ReadDir(path)
 		if err != nil {
-			lastErr = fmt.Errorf("test error: %v", err)
-			return false, nil
+			return false, fmt.Errorf("failed to read dir %v: %w", path, err)
 		}
 
 		if len(f) != 2 {
-			lastErr = fmt.Errorf("expected number of files: %v, got: %v", 2, len(f))
-			return false, nil
+			return false, fmt.Errorf("expected number of files: %v, got: %v %v", 2, len(f), f)
 		}
 
 		return true, nil
 	}, func(err error) {
-		t.Fatalf("%v", lastErr)
+		require.NoError(t, err)
 	})
 }
 
@@ -359,17 +297,14 @@ func BenchmarkRotator(b *testing.B) {
 }
 
 func benchmarkRotatorWithInputSize(size int, b *testing.B) {
-	var path string
-	var err error
-	if path, err = ioutil.TempDir("", pathPrefix); err != nil {
-		b.Fatalf("test setup err: %v", err)
-	}
+	path, err := ioutil.TempDir("", pathPrefix)
+	require.NoError(b, err)
 	defer os.RemoveAll(path)
 
 	fr, err := NewFileRotator(path, baseFileName, 5, 1024*1024, testlog.HCLogger(b))
-	if err != nil {
-		b.Fatalf("test setup err: %v", err)
-	}
+	require.NoError(b, err)
+	defer fr.Close()
+
 	b.ResetTimer()
 
 	// run the Fib function b.N times
@@ -377,9 +312,7 @@ func benchmarkRotatorWithInputSize(size int, b *testing.B) {
 		// Generate some input
 		data := make([]byte, size)
 		_, err := rand.Read(data)
-		if err != nil {
-			b.Fatalf("Error generating date: %v", err)
-		}
+		require.NoError(b, err)
 
 		// Insert random new lines
 		for i := 0; i < 100; i++ {
@@ -388,8 +321,7 @@ func benchmarkRotatorWithInputSize(size int, b *testing.B) {
 		}
 
 		// Write the data
-		if _, err := fr.Write(data); err != nil {
-			b.Fatalf("Failed to write data: %v", err)
-		}
+		_, err = fr.Write(data)
+		require.NoError(b, err)
 	}
 }