From 752e3ec0b14ed18b3ce8fc146984ccb44d8a66ec Mon Sep 17 00:00:00 2001 From: Daniel Nephin Date: Thu, 22 Oct 2020 14:53:32 -0400 Subject: [PATCH] logging: improve tests Standardize naming Use stricter assertions and reduce boilerplate to make the intent of the tests more obvious. Also explicitly sort the filenames so that the correct files are pruned, and so that the tests can not flake. --- logging/logfile.go | 2 + logging/logfile_test.go | 139 ++++++++++++++++++---------------------- 2 files changed, 65 insertions(+), 76 deletions(-) diff --git a/logging/logfile.go b/logging/logfile.go index 2317217500..153473482f 100644 --- a/logging/logfile.go +++ b/logging/logfile.go @@ -4,6 +4,7 @@ import ( "fmt" "os" "path/filepath" + "sort" "strconv" "strings" "sync" @@ -107,6 +108,7 @@ func (l *LogFile) pruneFiles() error { return nil } + sort.Strings(matches) last := len(matches) - l.MaxFiles return removeFiles(matches[:last]) } diff --git a/logging/logfile_test.go b/logging/logfile_test.go index 115f1bcff4..d56e4d42c0 100644 --- a/logging/logfile_test.go +++ b/logging/logfile_test.go @@ -2,135 +2,122 @@ package logging import ( "io/ioutil" + "os" "path/filepath" + "sort" "testing" "time" + "github.com/stretchr/testify/require" + "github.com/hashicorp/consul/sdk/testutil" ) -const ( - testFileName = "Consul.log" - testDuration = 50 * time.Millisecond - testBytes = 10 -) - -func TestLogFile_timeRotation(t *testing.T) { - tempDir := testutil.TempDir(t, "LogWriterTime") +func TestLogFile_Rotation_MaxDuration(t *testing.T) { + tempDir := testutil.TempDir(t, "") logFile := LogFile{ - fileName: testFileName, + fileName: "consul.log", logPath: tempDir, - duration: testDuration, + duration: 50 * time.Millisecond, } + logFile.Write([]byte("Hello World")) - time.Sleep(3 * testDuration) + time.Sleep(3 * logFile.duration) logFile.Write([]byte("Second File")) - want := 2 - if got, _ := ioutil.ReadDir(tempDir); len(got) != want { - t.Errorf("Expected %d files, got %v file(s)", want, len(got)) - } + require.Len(t, listDir(t, tempDir), 2) } func TestLogFile_openNew(t *testing.T) { - tempDir := testutil.TempDir(t, "LogWriterOpen") - logFile := LogFile{fileName: testFileName, logPath: tempDir, duration: testDuration} - if err := logFile.openNew(); err != nil { - t.Errorf("Expected open file %s, got an error (%s)", testFileName, err) + logFile := LogFile{ + fileName: "consul.log", + logPath: testutil.TempDir(t, ""), + duration: defaultRotateDuration, } + err := logFile.openNew() + require.NoError(t, err) - if _, err := ioutil.ReadFile(logFile.FileInfo.Name()); err != nil { - t.Errorf("Expected readable file %s, got an error (%s)", logFile.FileInfo.Name(), err) - } + msg := "[INFO] Something" + _, err = logFile.Write([]byte(msg)) + require.NoError(t, err) + + content, err := ioutil.ReadFile(logFile.FileInfo.Name()) + require.NoError(t, err) + require.Contains(t, string(content), msg) } -func TestLogFile_byteRotation(t *testing.T) { +func TestLogFile_Rotation_MaxBytes(t *testing.T) { tempDir := testutil.TempDir(t, "LogWriterBytes") logFile := LogFile{ - fileName: testFileName, + fileName: "somefile.log", logPath: tempDir, - MaxBytes: testBytes, - duration: 24 * time.Hour, + MaxBytes: 10, + duration: defaultRotateDuration, } logFile.Write([]byte("Hello World")) logFile.Write([]byte("Second File")) - want := 2 - tempFiles, _ := ioutil.ReadDir(tempDir) - if got := tempFiles; len(got) != want { - t.Errorf("Expected %d files, got %v file(s)", want, len(got)) - } + require.Len(t, listDir(t, tempDir), 2) } -func TestLogFile_deleteArchives(t *testing.T) { - tempDir := testutil.TempDir(t, "LogWriteDeleteArchives") +func TestLogFile_PruneFiles(t *testing.T) { + tempDir := testutil.TempDir(t, t.Name()) logFile := LogFile{ - fileName: testFileName, + fileName: "consul.log", logPath: tempDir, - MaxBytes: testBytes, - duration: 24 * time.Hour, + MaxBytes: 10, + duration: defaultRotateDuration, MaxFiles: 1, } logFile.Write([]byte("[INFO] Hello World")) logFile.Write([]byte("[INFO] Second File")) logFile.Write([]byte("[INFO] Third File")) - want := 2 - tempFiles, _ := ioutil.ReadDir(tempDir) - if got := tempFiles; len(got) != want { - t.Errorf("Expected %d files, got %v file(s)", want, len(got)) - return - } - for _, tempFile := range tempFiles { - var bytes []byte - var err error - path := filepath.Join(tempDir, tempFile.Name()) - if bytes, err = ioutil.ReadFile(path); err != nil { - t.Errorf(err.Error()) - return - } - contents := string(bytes) - if contents == "[INFO] Hello World" { - t.Errorf("Should have deleted the eldest log file") - return - } - } + logFiles := listDir(t, tempDir) + sort.Strings(logFiles) + require.Len(t, logFiles, 2) + + content, err := ioutil.ReadFile(filepath.Join(tempDir, logFiles[0])) + require.NoError(t, err) + require.Contains(t, string(content), "Second File") + + content, err = ioutil.ReadFile(filepath.Join(tempDir, logFiles[1])) + require.NoError(t, err) + require.Contains(t, string(content), "Third File") } -func TestLogFile_deleteArchivesDisabled(t *testing.T) { +func TestLogFile_PruneFiles_Disabled(t *testing.T) { tempDir := testutil.TempDir(t, t.Name()) logFile := LogFile{ - fileName: testFileName, + fileName: "somename.log", logPath: tempDir, - MaxBytes: testBytes, - duration: 24 * time.Hour, + MaxBytes: 10, + duration: defaultRotateDuration, MaxFiles: 0, } logFile.Write([]byte("[INFO] Hello World")) logFile.Write([]byte("[INFO] Second File")) logFile.Write([]byte("[INFO] Third File")) - want := 3 - tempFiles, _ := ioutil.ReadDir(tempDir) - if got := tempFiles; len(got) != want { - t.Errorf("Expected %d files, got %v file(s)", want, len(got)) - return - } + require.Len(t, listDir(t, tempDir), 3) } -func TestLogFile_rotationDisabled(t *testing.T) { +func TestLogFile_FileRotation_Disabled(t *testing.T) { tempDir := testutil.TempDir(t, t.Name()) logFile := LogFile{ - fileName: testFileName, + fileName: "consul.log", logPath: tempDir, - MaxBytes: testBytes, - duration: 24 * time.Hour, + MaxBytes: 10, MaxFiles: -1, } logFile.Write([]byte("[INFO] Hello World")) logFile.Write([]byte("[INFO] Second File")) logFile.Write([]byte("[INFO] Third File")) - want := 1 - tempFiles, _ := ioutil.ReadDir(tempDir) - if got := tempFiles; len(got) != want { - t.Errorf("Expected %d files, got %v file(s)", want, len(got)) - return - } + require.Len(t, listDir(t, tempDir), 1) +} + +func listDir(t *testing.T, name string) []string { + t.Helper() + fh, err := os.Open(name) + require.NoError(t, err) + files, err := fh.Readdirnames(100) + require.NoError(t, err) + return files }