Skip to content

Commit 5cd0f0b

Browse files
test(logger): improve common_test.go with testify patterns and formatLogLine coverage
- Replace t.Errorf/t.Fatal patterns with testify assertions: - TestInitLogFile_InvalidDirectory: use require.Error instead of if/t.Fatal - TestInitLogFile_EmptyFileName: use require.Error instead of if/t.Fatal - TestInitLogger_FileLogger/JSONLLogger/MarkdownLogger: replace t.Errorf in error-handler callbacks with boolean flag + assert.False - TestInitLogger_FileLoggerFallback/JSONLLoggerError/MarkdownLoggerFallback: replace t.Errorf in setup-handler callbacks with boolean flag + assert.False - TestInitLogger_SetupError: same pattern for error-handler callback - Add TestFormatLogLine to cover the previously untested formatLogLine function: - Tests all four log levels (INFO, WARN, ERROR, DEBUG) - Verifies bracket structure [timestamp] [level] [category] message - Validates RFC3339 UTC timestamp format and time window - Tests format string interpolation with args - Tests empty category edge case Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent 6ba4d05 commit 5cd0f0b

1 file changed

Lines changed: 94 additions & 20 deletions

File tree

internal/logger/common_test.go

Lines changed: 94 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -4,8 +4,10 @@ import (
44
"fmt"
55
"os"
66
"path/filepath"
7+
"strings"
78
"sync"
89
"testing"
10+
"time"
911

1012
"github.com/stretchr/testify/assert"
1113
"github.com/stretchr/testify/require"
@@ -289,11 +291,10 @@ func TestInitLogFile_InvalidDirectory(t *testing.T) {
289291
fileName := "test.log"
290292

291293
file, err := initLogFile(logDir, fileName, os.O_APPEND)
292-
if err == nil {
294+
if file != nil {
293295
file.Close()
294-
t.Fatal("initLogFile should fail when directory can't be created")
295296
}
296-
297+
require.Error(err, "initLogFile should fail when directory can't be created")
297298
assert.Contains(err.Error(), "failed to create log directory", "Error should mention directory creation failure")
298299
}
299300

@@ -319,16 +320,16 @@ func TestInitLogFile_UnwritableDirectory(t *testing.T) {
319320

320321
func TestInitLogFile_EmptyFileName(t *testing.T) {
321322
assert := assert.New(t)
323+
require := require.New(t)
322324
tmpDir := t.TempDir()
323325
logDir := filepath.Join(tmpDir, "logs")
324326
fileName := ""
325327

326328
file, err := initLogFile(logDir, fileName, os.O_APPEND)
327-
if err == nil {
329+
if file != nil {
328330
file.Close()
329-
t.Fatal("initLogFile should fail with empty fileName")
330331
}
331-
332+
require.Error(err, "initLogFile should fail with empty fileName")
332333
assert.Contains(err.Error(), "failed to open log file", "Error should mention file opening failure")
333334
}
334335

@@ -388,6 +389,7 @@ func TestInitLogger_FileLogger(t *testing.T) {
388389
fileName := "test.log"
389390

390391
// Test successful initialization
392+
errorHandlerCalled := false
391393
logger, err := initLogger(
392394
logDir, fileName, os.O_APPEND,
393395
func(file *os.File, logDir, fileName string) (*FileLogger, error) {
@@ -399,12 +401,13 @@ func TestInitLogger_FileLogger(t *testing.T) {
399401
return fl, nil
400402
},
401403
func(err error, logDir, fileName string) (*FileLogger, error) {
402-
// Should not be called on success
403-
t.Errorf("Error handler should not be called on successful initialization")
404+
errorHandlerCalled = true
404405
return nil, err
405406
},
406407
)
407408

409+
assert.False(errorHandlerCalled, "Error handler should not be called on successful initialization")
410+
408411
require.NoError(err, "initLogger should not return error")
409412
require.NotNil(logger, "logger should not be nil")
410413
assert.Equal(logDir, logger.logDir, "logDir should match")
@@ -429,12 +432,12 @@ func TestInitLogger_FileLoggerFallback(t *testing.T) {
429432
fileName := "test.log"
430433

431434
errorHandlerCalled := false
435+
setupHandlerCalled := false
432436

433437
logger, err := initLogger(
434438
logDir, fileName, os.O_APPEND,
435439
func(file *os.File, logDir, fileName string) (*FileLogger, error) {
436-
// Should not be called on error
437-
t.Errorf("Setup handler should not be called on error")
440+
setupHandlerCalled = true
438441
return nil, nil
439442
},
440443
func(err error, logDir, fileName string) (*FileLogger, error) {
@@ -450,6 +453,8 @@ func TestInitLogger_FileLoggerFallback(t *testing.T) {
450453
},
451454
)
452455

456+
assert.False(setupHandlerCalled, "Setup handler should not be called on error")
457+
453458
assert.True(errorHandlerCalled, "Error handler should be called")
454459
require.NoError(err, "initLogger should not return error for fallback")
455460
require.NotNil(logger, "logger should not be nil")
@@ -465,6 +470,7 @@ func TestInitLogger_JSONLLogger(t *testing.T) {
465470
logDir := filepath.Join(tmpDir, "logs")
466471
fileName := "test.jsonl"
467472

473+
errorHandlerCalled := false
468474
logger, err := initLogger(
469475
logDir, fileName, os.O_APPEND,
470476
func(file *os.File, logDir, fileName string) (*JSONLLogger, error) {
@@ -476,12 +482,13 @@ func TestInitLogger_JSONLLogger(t *testing.T) {
476482
return jl, nil
477483
},
478484
func(err error, logDir, fileName string) (*JSONLLogger, error) {
479-
// Should not be called on success
480-
t.Errorf("Error handler should not be called on successful initialization")
485+
errorHandlerCalled = true
481486
return nil, err
482487
},
483488
)
484489

490+
assert.False(errorHandlerCalled, "Error handler should not be called on successful initialization")
491+
485492
require.NoError(err, "initLogger should not return error")
486493
require.NotNil(logger, "logger should not be nil")
487494
assert.Equal(logDir, logger.logDir, "logDir should match")
@@ -505,12 +512,12 @@ func TestInitLogger_JSONLLoggerError(t *testing.T) {
505512
fileName := "test.jsonl"
506513

507514
errorHandlerCalled := false
515+
setupHandlerCalled := false
508516

509517
logger, err := initLogger(
510518
logDir, fileName, os.O_APPEND,
511519
func(file *os.File, logDir, fileName string) (*JSONLLogger, error) {
512-
// Should not be called on error
513-
t.Errorf("Setup handler should not be called on error")
520+
setupHandlerCalled = true
514521
return nil, nil
515522
},
516523
func(err error, logDir, fileName string) (*JSONLLogger, error) {
@@ -521,6 +528,8 @@ func TestInitLogger_JSONLLoggerError(t *testing.T) {
521528
},
522529
)
523530

531+
assert.False(setupHandlerCalled, "Setup handler should not be called on error")
532+
524533
assert.True(errorHandlerCalled, "Error handler should be called")
525534
assert.Error(err, "initLogger should return error")
526535
assert.Nil(logger, "logger should be nil on error")
@@ -534,6 +543,7 @@ func TestInitLogger_MarkdownLogger(t *testing.T) {
534543
logDir := filepath.Join(tmpDir, "logs")
535544
fileName := "test.md"
536545

546+
errorHandlerCalled := false
537547
logger, err := initLogger(
538548
logDir, fileName, os.O_TRUNC,
539549
func(file *os.File, logDir, fileName string) (*MarkdownLogger, error) {
@@ -546,12 +556,13 @@ func TestInitLogger_MarkdownLogger(t *testing.T) {
546556
return ml, nil
547557
},
548558
func(err error, logDir, fileName string) (*MarkdownLogger, error) {
549-
// Should not be called on success
550-
t.Errorf("Error handler should not be called on successful initialization")
559+
errorHandlerCalled = true
551560
return nil, err
552561
},
553562
)
554563

564+
assert.False(errorHandlerCalled, "Error handler should not be called on successful initialization")
565+
555566
require.NoError(err, "initLogger should not return error")
556567
require.NotNil(logger, "logger should not be nil")
557568
assert.Equal(logDir, logger.logDir, "logDir should match")
@@ -577,12 +588,12 @@ func TestInitLogger_MarkdownLoggerFallback(t *testing.T) {
577588
fileName := "test.md"
578589

579590
errorHandlerCalled := false
591+
setupHandlerCalled := false
580592

581593
logger, err := initLogger(
582594
logDir, fileName, os.O_TRUNC,
583595
func(file *os.File, logDir, fileName string) (*MarkdownLogger, error) {
584-
// Should not be called on error
585-
t.Errorf("Setup handler should not be called on error")
596+
setupHandlerCalled = true
586597
return nil, nil
587598
},
588599
func(err error, logDir, fileName string) (*MarkdownLogger, error) {
@@ -598,6 +609,8 @@ func TestInitLogger_MarkdownLoggerFallback(t *testing.T) {
598609
},
599610
)
600611

612+
assert.False(setupHandlerCalled, "Setup handler should not be called on error")
613+
601614
assert.True(errorHandlerCalled, "Error handler should be called")
602615
require.NoError(err, "initLogger should not return error for fallback")
603616
require.NotNil(logger, "logger should not be nil")
@@ -611,19 +624,21 @@ func TestInitLogger_SetupError(t *testing.T) {
611624
logDir := filepath.Join(tmpDir, "logs")
612625
fileName := "test.log"
613626

627+
errorHandlerCalled := false
614628
logger, err := initLogger(
615629
logDir, fileName, os.O_APPEND,
616630
func(file *os.File, logDir, fileName string) (*FileLogger, error) {
617631
// Simulate setup error
618632
return nil, assert.AnError
619633
},
620634
func(err error, logDir, fileName string) (*FileLogger, error) {
621-
// Should not be called for setup errors
622-
t.Errorf("Error handler should not be called for setup errors")
635+
errorHandlerCalled = true
623636
return nil, err
624637
},
625638
)
626639

640+
a.False(errorHandlerCalled, "Error handler should not be called for setup errors")
641+
627642
a.Error(err, "initLogger should return error on setup failure")
628643
a.Equal(assert.AnError, err, "Error should match setup error")
629644
a.Nil(logger, "logger should be nil on setup error")
@@ -694,3 +709,62 @@ func TestInitLogger_FileFlags(t *testing.T) {
694709
assert.NotContains(string(content), "appended content", "File should not contain appended content")
695710
assert.Contains(string(content), "new content", "File should contain new content")
696711
}
712+
713+
// TestFormatLogLine tests the formatLogLine helper that builds standard log lines.
714+
func TestFormatLogLine(t *testing.T) {
715+
t.Run("output contains level bracket", func(t *testing.T) {
716+
levels := []LogLevel{LogLevelInfo, LogLevelWarn, LogLevelError, LogLevelDebug}
717+
for _, level := range levels {
718+
t.Run(string(level), func(t *testing.T) {
719+
result := formatLogLine(level, "test", "msg")
720+
assert.Contains(t, result, "["+string(level)+"]",
721+
"Log line should contain level in brackets")
722+
})
723+
}
724+
})
725+
726+
t.Run("output contains category in brackets", func(t *testing.T) {
727+
result := formatLogLine(LogLevelInfo, "startup", "message")
728+
assert.Contains(t, result, "[startup]", "Log line should contain the category")
729+
})
730+
731+
t.Run("output contains formatted message", func(t *testing.T) {
732+
result := formatLogLine(LogLevelInfo, "test", "count=%d name=%s", 42, "alice")
733+
assert.Contains(t, result, "count=42 name=alice", "Log line should contain formatted message")
734+
})
735+
736+
t.Run("output follows bracket structure", func(t *testing.T) {
737+
result := formatLogLine(LogLevelInfo, "auth", "event occurred")
738+
// Expected format: [timestamp] [INFO] [auth] event occurred
739+
assert.Regexp(t, `^\[\d{4}-\d{2}-\d{2}T\d{2}:\d{2}:\d{2}Z\] \[INFO\] \[auth\] event occurred$`, result)
740+
})
741+
742+
t.Run("timestamp is RFC3339 UTC within test window", func(t *testing.T) {
743+
before := time.Now().UTC().Truncate(time.Second)
744+
result := formatLogLine(LogLevelDebug, "test", "msg")
745+
after := time.Now().UTC().Add(time.Second)
746+
747+
// Extract the timestamp between the first pair of brackets
748+
assert.Regexp(t, `^\[\d{4}-\d{2}-\d{2}T\d{2}:\d{2}:\d{2}Z\]`, result,
749+
"Should start with RFC3339 UTC timestamp in brackets")
750+
751+
parts := strings.SplitN(result, "]", 2)
752+
require.Len(t, parts, 2, "Output should contain at least one closing bracket")
753+
tsStr := strings.TrimPrefix(parts[0], "[")
754+
755+
ts, err := time.Parse(time.RFC3339, tsStr)
756+
require.NoError(t, err, "Extracted timestamp should parse as RFC3339")
757+
assert.False(t, ts.Before(before), "Timestamp should not be before test start")
758+
assert.False(t, ts.After(after), "Timestamp should not be after test end")
759+
})
760+
761+
t.Run("format string used as-is when no args provided", func(t *testing.T) {
762+
result := formatLogLine(LogLevelWarn, "net", "plain message")
763+
assert.Contains(t, result, "plain message", "Should include the literal format string")
764+
})
765+
766+
t.Run("empty category produces empty bracket", func(t *testing.T) {
767+
result := formatLogLine(LogLevelInfo, "", "message")
768+
assert.Contains(t, result, "[]", "Empty category should produce empty bracket pair")
769+
})
770+
}

0 commit comments

Comments
 (0)