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/hashicorp/consul/sdk/testutil" -) + "github.com/stretchr/testify/require" -const ( - testFileName = "Consul.log" - testDuration = 50 * time.Millisecond - testBytes = 10 + "github.com/hashicorp/consul/sdk/testutil" ) -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 }