From dd16c5a91f9d91562a80cdb36613075e831f24ad Mon Sep 17 00:00:00 2001 From: qiyoulin Date: Thu, 16 Apr 2026 15:06:37 +0800 Subject: [PATCH 1/5] fix(test): skip Windows permission tests, close unclosed resources, isolate Linux-specific code 1. Skip permission-related tests on Windows due to OS differences 2. Add missing resource cleanup (logger) in test cases 3. Move Linux-specific code (syscall.Umask) to build-tagged files --- timberjack_linux_test.go | 74 +++++++++++++++++++++++++++++++++++++ timberjack_test.go | 80 +++++++--------------------------------- 2 files changed, 88 insertions(+), 66 deletions(-) create mode 100644 timberjack_linux_test.go diff --git a/timberjack_linux_test.go b/timberjack_linux_test.go new file mode 100644 index 0000000..7ba9da6 --- /dev/null +++ b/timberjack_linux_test.go @@ -0,0 +1,74 @@ +//go:build !windows + +package timberjack + +import ( + "os" + "syscall" + "testing" + + "golang.org/x/sys/unix" +) + +func TestOpenNewDefaultPerm(t *testing.T) { + // if runtime.GOOS == "windows" { + // t.Skip("Skipping default perm test on Windows") + // } + + // Ensure no bits get masked out. + syscall.Umask(0o000) + + dir := makeTempDir("TestOpenNewDefaultPerm", t) + defer os.RemoveAll(dir) + + l := &Logger{ + Filename: logFile(dir), + } + defer l.Close() + + _, err := l.Write([]byte("foo")) + isNil(err, t) + hasPerm(logFile(dir), 0o640, t) +} + +func TestOpenNewCustomPerm(t *testing.T) { + // if runtime.GOOS == "windows" { + // t.Skip("Skipping custom perm test on Windows") + // } + + // Ensure no bits get masked out. + unix.Umask(0o000) + + dir := makeTempDir("TestOpenNewCustomPerm", t) + defer os.RemoveAll(dir) + + filename := logFile(dir) + l := &Logger{ + Filename: filename, + FileMode: 0o747, + } + _, err := l.Write([]byte("foo")) + isNil(err, t) + hasPerm(filename, 0o747, t) + l.Close() + + filename += ".1" + l = &Logger{ + Filename: filename, + FileMode: 0o200, + } + _, err = l.Write([]byte("foo")) + isNil(err, t) + hasPerm(filename, 0o200, t) + l.Close() + + filename += ".2" + l = &Logger{ + Filename: filename, + FileMode: 0o666, + } + _, err = l.Write([]byte("foo")) + isNil(err, t) + hasPerm(filename, 0o666, t) + l.Close() +} diff --git a/timberjack_test.go b/timberjack_test.go index 64bc2e4..91aaa13 100644 --- a/timberjack_test.go +++ b/timberjack_test.go @@ -12,7 +12,6 @@ import ( "sort" "strings" "sync" - "syscall" "testing" "time" @@ -77,6 +76,9 @@ func TestNewFile(t *testing.T) { } func TestOpenExisting(t *testing.T) { + if runtime.GOOS == "windows" { + t.Skip("Skipping default perm test on Windows") + } currentTime = fakeTime dir := makeTempDir("TestOpenExisting", t) defer os.RemoveAll(dir) @@ -1102,6 +1104,7 @@ func TestOpenExistingOrNew_Fallback(t *testing.T) { t.Fatalf("expected fallback to openNew, got error: %v", err) } + logger.Close() // Clean up the recreated file if rmErr := os.Remove(path); rmErr != nil && !os.IsNotExist(rmErr) { t.Errorf("cleanup failed: %v", rmErr) @@ -1162,14 +1165,15 @@ func TestBackupName(t *testing.T) { // default (before-ext) resultUTC := backupName(name, false, "size", rotationTime, backupTimeFormat, false) expectedUTC := "/tmp/test-2020-01-02T03-04-05.006-size.log" - if resultUTC != expectedUTC { + + if filepath.Base(resultUTC) != filepath.Base(expectedUTC) { t.Errorf("expected %q, got %q", expectedUTC, resultUTC) } // after-ext after := backupName(name, false, "size", rotationTime, backupTimeFormat, true) expectedAfter := "/tmp/test.log-2020-01-02T03-04-05.006-size" - if after != expectedAfter { + if filepath.Base(after) != filepath.Base(expectedAfter) { t.Errorf("expected %q, got %q", expectedAfter, after) } } @@ -1224,6 +1228,9 @@ func TestRunScheduledRotations_NoMarks(t *testing.T) { } func TestRotate_OpenNewFails(t *testing.T) { + if runtime.GOOS == "windows" { + t.Skip("Skipping default path test on Windows") + } badPath := "/bad/path/logfile.log" l := &Logger{ Filename: badPath, @@ -1407,6 +1414,9 @@ func TestOpenNew_StatUnexpectedError(t *testing.T) { } func TestCompressLogFile_CopyFails(t *testing.T) { + if runtime.GOOS == "windows" { + t.Skip("Skipping default perm test on Windows") + } dir := t.TempDir() src := filepath.Join(dir, "bad.log") dst := src + ".gz" @@ -1959,6 +1969,7 @@ func TestRotate_StartMillOnlyOnce_Observable(t *testing.T) { Compress: true, millCh: make(chan bool, 10), // Buffered so we can trigger multiple } + defer logger.Close() // Create two valid backup files to be compressed for i := 0; i < 2; i++ { @@ -2641,69 +2652,6 @@ func TestWriteToClosedLogger(t *testing.T) { } } -func TestOpenNewDefaultPerm(t *testing.T) { - if runtime.GOOS == "windows" { - t.Skip("Skipping default perm test on Windows") - } - - // Ensure no bits get masked out. - syscall.Umask(0o000) - - dir := makeTempDir("TestOpenNewDefaultPerm", t) - defer os.RemoveAll(dir) - - l := &Logger{ - Filename: logFile(dir), - } - defer l.Close() - - _, err := l.Write([]byte("foo")) - isNil(err, t) - hasPerm(logFile(dir), 0o640, t) -} - -func TestOpenNewCustomPerm(t *testing.T) { - if runtime.GOOS == "windows" { - t.Skip("Skipping custom perm test on Windows") - } - - // Ensure no bits get masked out. - syscall.Umask(0o000) - - dir := makeTempDir("TestOpenNewCustomPerm", t) - defer os.RemoveAll(dir) - - filename := logFile(dir) - l := &Logger{ - Filename: filename, - FileMode: 0o747, - } - _, err := l.Write([]byte("foo")) - isNil(err, t) - hasPerm(filename, 0o747, t) - l.Close() - - filename += ".1" - l = &Logger{ - Filename: filename, - FileMode: 0o200, - } - _, err = l.Write([]byte("foo")) - isNil(err, t) - hasPerm(filename, 0o200, t) - l.Close() - - filename += ".2" - l = &Logger{ - Filename: filename, - FileMode: 0o666, - } - _, err = l.Write([]byte("foo")) - isNil(err, t) - hasPerm(filename, 0o666, t) - l.Close() -} - // waitForFileWithSuffix polls dir for a file ending in suffix, up to timeout. func waitForFileWithSuffix(t *testing.T, dir, suffix string, timeout time.Duration) (string, error) { t.Helper() From 321713564a5846fff2e7c0c19850c2d6058d06c9 Mon Sep 17 00:00:00 2001 From: qiyoulin Date: Fri, 17 Apr 2026 09:55:04 +0800 Subject: [PATCH 2/5] Maintain backward compatibility with previous Go versions --- timberjack_linux_test.go | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/timberjack_linux_test.go b/timberjack_linux_test.go index 7ba9da6..73aa7aa 100644 --- a/timberjack_linux_test.go +++ b/timberjack_linux_test.go @@ -6,8 +6,6 @@ import ( "os" "syscall" "testing" - - "golang.org/x/sys/unix" ) func TestOpenNewDefaultPerm(t *testing.T) { @@ -37,7 +35,7 @@ func TestOpenNewCustomPerm(t *testing.T) { // } // Ensure no bits get masked out. - unix.Umask(0o000) + syscall.Umask(0o000) dir := makeTempDir("TestOpenNewCustomPerm", t) defer os.RemoveAll(dir) From 29e941815baa7159c06e2cd5d27450eb48fe6c90 Mon Sep 17 00:00:00 2001 From: qiyoulin Date: Fri, 17 Apr 2026 11:10:51 +0800 Subject: [PATCH 3/5] Skip inappropriate test cases when running as root --- timberjack_linux_test.go | 56 ++++++++++++++++++++++++++++++++++------ timberjack_test.go | 44 ------------------------------- 2 files changed, 48 insertions(+), 52 deletions(-) diff --git a/timberjack_linux_test.go b/timberjack_linux_test.go index 73aa7aa..ade7fff 100644 --- a/timberjack_linux_test.go +++ b/timberjack_linux_test.go @@ -4,15 +4,59 @@ package timberjack import ( "os" + "path/filepath" "syscall" "testing" ) -func TestOpenNewDefaultPerm(t *testing.T) { - // if runtime.GOOS == "windows" { - // t.Skip("Skipping default perm test on Windows") - // } +func TestRotate_OpenNewFails(t *testing.T) { + if os.Getuid() == 0 { + t.Skip("Skipping test when running as root") + } + badPath := "/bad/path/logfile.log" + if _, err := os.Stat(badPath); err == nil { + t.Skip("Skipping test that relies on non-existent path") + } + l := &Logger{ + Filename: badPath, + } + // force an invalid path to trigger openNew failure + err := l.rotate("manual") + if err == nil { + t.Fatal("expected error from rotate due to invalid openNew") + } +} +func TestCompressLogFile_CopyFails(t *testing.T) { + if os.Getuid() == 0 { + t.Skip("Skipping test when running as root") + } + dir := t.TempDir() + src := filepath.Join(dir, "bad.log") + dst := src + ".gz" + + if err := os.WriteFile(src, []byte("data"), 0o200); err != nil { // write-only + t.Fatalf("failed to create test file: %v", err) + } + defer os.Chmod(src, 0o644) + + originalStat := osStat + osStat = func(name string) (os.FileInfo, error) { + return os.Stat(src) + } + defer func() { osStat = originalStat }() + + l := &Logger{} + // snapshot patched osStat + l.resolveConfigLocked() + + err := l.compressLogFile(src, dst) + if err == nil { + t.Errorf("expected failure during compression, got: %v", err) + } +} + +func TestOpenNewDefaultPerm(t *testing.T) { // Ensure no bits get masked out. syscall.Umask(0o000) @@ -30,10 +74,6 @@ func TestOpenNewDefaultPerm(t *testing.T) { } func TestOpenNewCustomPerm(t *testing.T) { - // if runtime.GOOS == "windows" { - // t.Skip("Skipping custom perm test on Windows") - // } - // Ensure no bits get masked out. syscall.Umask(0o000) diff --git a/timberjack_test.go b/timberjack_test.go index 91aaa13..30848bb 100644 --- a/timberjack_test.go +++ b/timberjack_test.go @@ -1227,21 +1227,6 @@ func TestRunScheduledRotations_NoMarks(t *testing.T) { } } -func TestRotate_OpenNewFails(t *testing.T) { - if runtime.GOOS == "windows" { - t.Skip("Skipping default path test on Windows") - } - badPath := "/bad/path/logfile.log" - l := &Logger{ - Filename: badPath, - } - // force an invalid path to trigger openNew failure - err := l.rotate("manual") - if err == nil { - t.Fatal("expected error from rotate due to invalid openNew") - } -} - func TestRotate_TriggersTimeReason(t *testing.T) { currentTime = func() time.Time { return time.Date(2024, 5, 1, 12, 0, 0, 0, time.UTC) @@ -1413,35 +1398,6 @@ func TestOpenNew_StatUnexpectedError(t *testing.T) { } } -func TestCompressLogFile_CopyFails(t *testing.T) { - if runtime.GOOS == "windows" { - t.Skip("Skipping default perm test on Windows") - } - dir := t.TempDir() - src := filepath.Join(dir, "bad.log") - dst := src + ".gz" - - if err := os.WriteFile(src, []byte("data"), 0o200); err != nil { // write-only - t.Fatalf("failed to create test file: %v", err) - } - defer os.Chmod(src, 0o644) - - originalStat := osStat - osStat = func(name string) (os.FileInfo, error) { - return os.Stat(src) - } - defer func() { osStat = originalStat }() - - l := &Logger{} - // snapshot patched osStat - l.resolveConfigLocked() - - err := l.compressLogFile(src, dst) - if err == nil { - t.Errorf("expected failure during compression, got: %v", err) - } -} - func TestOpenExistingOrNew_StatFailure(t *testing.T) { originalStat := osStat defer func() { osStat = originalStat }() From fd4bf5c21da3f8044b5de41a35aab54db699e7d9 Mon Sep 17 00:00:00 2001 From: Dean Ruina <81315494+DeRuina@users.noreply.github.com> Date: Mon, 20 Apr 2026 14:56:22 +0300 Subject: [PATCH 4/5] Update timberjack_linux_test.go Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- timberjack_linux_test.go | 2 -- 1 file changed, 2 deletions(-) diff --git a/timberjack_linux_test.go b/timberjack_linux_test.go index ade7fff..1fb7c17 100644 --- a/timberjack_linux_test.go +++ b/timberjack_linux_test.go @@ -1,5 +1,3 @@ -//go:build !windows - package timberjack import ( From ec011f2998e03c4494755e4730d388d46fae0038 Mon Sep 17 00:00:00 2001 From: Dean Ruina <81315494+DeRuina@users.noreply.github.com> Date: Mon, 20 Apr 2026 15:44:17 +0300 Subject: [PATCH 5/5] Update timberjack_linux_test.go Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- timberjack_linux_test.go | 24 +++++++++++++++--------- 1 file changed, 15 insertions(+), 9 deletions(-) diff --git a/timberjack_linux_test.go b/timberjack_linux_test.go index 1fb7c17..c610b32 100644 --- a/timberjack_linux_test.go +++ b/timberjack_linux_test.go @@ -79,32 +79,38 @@ func TestOpenNewCustomPerm(t *testing.T) { defer os.RemoveAll(dir) filename := logFile(dir) - l := &Logger{ + l1 := &Logger{ Filename: filename, FileMode: 0o747, } - _, err := l.Write([]byte("foo")) + t.Cleanup(func() { + l1.Close() + }) + _, err := l1.Write([]byte("foo")) isNil(err, t) hasPerm(filename, 0o747, t) - l.Close() filename += ".1" - l = &Logger{ + l2 := &Logger{ Filename: filename, FileMode: 0o200, } - _, err = l.Write([]byte("foo")) + t.Cleanup(func() { + l2.Close() + }) + _, err = l2.Write([]byte("foo")) isNil(err, t) hasPerm(filename, 0o200, t) - l.Close() filename += ".2" - l = &Logger{ + l3 := &Logger{ Filename: filename, FileMode: 0o666, } - _, err = l.Write([]byte("foo")) + t.Cleanup(func() { + l3.Close() + }) + _, err = l3.Write([]byte("foo")) isNil(err, t) hasPerm(filename, 0o666, t) - l.Close() }