From 99882832c00fb790155adcaf1164808e00fbbb1d Mon Sep 17 00:00:00 2001 From: Tai Groot Date: Fri, 6 Mar 2026 11:38:31 -0500 Subject: [PATCH] feat(log): add Logger.Debugln, expand test coverage, update Go/CI (#21) - Add missing Logger.Debugln method (package-level Debugln existed but Logger type lacked it) - Replace empty test stubs with real tests for Debug, Debugf, Info, Infof, Print, Printf, Notice, Warn, Warnf, Error, Errorf, Panic, Panicf, Panicln - Add tests for namespace filtering, multi-namespace clients, namespace registry, level storage, colorize, parseLevelString, Broadcast, matchesNamespace, fileInfo, Logger.Debugln, empty namespace default - Update go.mod to Go 1.26.1 - Update CI to actions/checkout@v4, actions/setup-go@v5, Go 1.26, add -race flag, trigger on pull_request - Fix stale CRUSH.md references (Go version, CI config, stderr bug) --- .github/workflows/ci.yaml | 13 +- CRUSH.md | 23 +- go.mod | 2 +- log/log_test.go | 508 ++++++++++++++++++++++++++++++++++---- log/logger.go | 14 ++ 5 files changed, 491 insertions(+), 69 deletions(-) diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml index 735f279..71f9101 100644 --- a/.github/workflows/ci.yaml +++ b/.github/workflows/ci.yaml @@ -1,23 +1,20 @@ name: Go package -on: [push] +on: [push, pull_request] jobs: test: runs-on: ubuntu-latest steps: - - uses: actions/checkout@v3 + - uses: actions/checkout@v4 - name: Set up Go - uses: actions/setup-go@v4 + uses: actions/setup-go@v5 with: - go-version: "1.25" - - - name: Install dependencies - run: go get . + go-version: "1.26" - name: Build run: go build -v ./... - name: Test - run: go test -v ./... + run: go test -race -v ./... diff --git a/CRUSH.md b/CRUSH.md index b3cee5a..bde9691 100644 --- a/CRUSH.md +++ b/CRUSH.md @@ -239,7 +239,7 @@ Still embeds viewer.html, but HTML now includes: ## Go Version & Dependencies -- **Go version**: 1.24.4 (specified in go.mod) +- **Go version**: 1.26.1 (specified in go.mod) - **Only external dependency**: `github.com/gorilla/websocket v1.5.3` ## Naming Conventions & Style @@ -314,27 +314,17 @@ UI provides "Reconnect" button for this purpose. ### 6. Stderr Client Uses All Namespaces The built-in stderr client (created in `init()`) listens to all namespaces: ```go -stderrClient = CreateClient(DefaultNamespace) +stderrClient = CreateClient() // No args = all namespaces ``` -But only prints logs matching its own namespace in `logStdErr()`: +It prints logs matching its level and namespace filter in `logStdErr()`: ```go if e.level >= c.LogLevel && c.matchesNamespace(e.Namespace) { fmt.Fprintf(os.Stderr, "%s\t%s\t[%s]\t%s\t%s\n", ...) } ``` -**Wait, that's a bug!** The stderr client is created with `DefaultNamespace` but should be created with no namespaces to see all logs. Let me check this. - -Actually looking at the code: -```go -stderrClient = CreateClient(DefaultNamespace) -``` - -This means stderr client only sees "default" namespace logs. This might be intentional, but seems like a bug. Should probably be: -```go -stderrClient = CreateClient() // No args = all namespaces -``` +Since `CreateClient()` is called with no arguments, the Namespaces slice is empty, which means it matches all namespaces. ### 7. Grid Layout Updated The log viewer grid changed from 4 to 5 columns: @@ -374,8 +364,9 @@ All existing tests pass with namespace support added. ## CI/CD GitHub Actions workflow (`.github/workflows/ci.yaml`): -- Still uses Go 1.21 (should update to 1.24.4 to match go.mod) -- No changes needed for v2 functionality +- Uses Go 1.26, actions/checkout@v4, actions/setup-go@v5 +- Runs tests with `-race` flag +- Triggers on push and pull_request ## Common Tasks diff --git a/go.mod b/go.mod index fd10c7e..63e31b4 100644 --- a/go.mod +++ b/go.mod @@ -1,5 +1,5 @@ module github.com/taigrr/log-socket/v2 -go 1.26.0 +go 1.26.1 require github.com/gorilla/websocket v1.5.3 diff --git a/log/log_test.go b/log/log_test.go index c6d26ec..e23eed7 100644 --- a/log/log_test.go +++ b/log/log_test.go @@ -4,8 +4,21 @@ import ( "strconv" "sync" "testing" + "time" ) +// getEntry reads from a client with a timeout to avoid hanging tests. +func getEntry(c *Client, timeout time.Duration) (Entry, bool) { + ch := make(chan Entry, 1) + go func() { ch <- c.Get() }() + select { + case e := <-ch: + return e, true + case <-time.After(timeout): + return Entry{}, false + } +} + // Test CreateClient() and Client.Destroy() func TestCreateDestroy(t *testing.T) { // Ensure only stderr exists at the beginning @@ -64,70 +77,477 @@ func TestOrder(t *testing.T) { t.Error("Trace input doesn't match output") } } + c.Destroy() } -// Debug prints out logs on debug level func TestDebug(t *testing.T) { - Debug("Test of Debug") - // if logLevel >= LDebug { - // entry := logger.WithFields(logrus.Fields{}) - // entry.Data["file"] = fileInfo(2) - // entry.Debug(args...) - // } + c := CreateClient(DefaultNamespace) + c.SetLogLevel(LDebug) + + Debug("debug message") + e, ok := getEntry(c, time.Second) + if !ok { + t.Fatal("timed out waiting for debug entry") + } + if e.Level != "DEBUG" { + t.Errorf("level = %q, want DEBUG", e.Level) + } + if e.Output != "debug message" { + t.Errorf("output = %q, want %q", e.Output, "debug message") + } + if e.Namespace != DefaultNamespace { + t.Errorf("namespace = %q, want %q", e.Namespace, DefaultNamespace) + } + c.Destroy() +} + +func TestDebugf(t *testing.T) { + c := CreateClient(DefaultNamespace) + c.SetLogLevel(LDebug) + + Debugf("hello %s %d", "world", 42) + e, ok := getEntry(c, time.Second) + if !ok { + t.Fatal("timed out") + } + if e.Output != "hello world 42" { + t.Errorf("output = %q, want %q", e.Output, "hello world 42") + } + c.Destroy() } -// Info prints out logs on info level func TestInfo(t *testing.T) { - // if logLevel >= LInfo { - // entry := logger.WithFields(logrus.Fields{}) - // entry.Data["file"] = fileInfo(2) - // entry.Info(args...) - // } + c := CreateClient(DefaultNamespace) + c.SetLogLevel(LInfo) + + Info("info message") + e, ok := getEntry(c, time.Second) + if !ok { + t.Fatal("timed out waiting for info entry") + } + if e.Level != "INFO" { + t.Errorf("level = %q, want INFO", e.Level) + } + if e.Output != "info message" { + t.Errorf("output = %q, want %q", e.Output, "info message") + } + c.Destroy() +} + +func TestInfof(t *testing.T) { + c := CreateClient(DefaultNamespace) + c.SetLogLevel(LInfo) + + Infof("count: %d", 99) + e, ok := getEntry(c, time.Second) + if !ok { + t.Fatal("timed out") + } + if e.Output != "count: 99" { + t.Errorf("output = %q, want %q", e.Output, "count: 99") + } + c.Destroy() } -// Print prints out logs on info level func TestPrint(t *testing.T) { - // if logLevel >= LInfo { - // entry := logger.WithFields(logrus.Fields{}) - // entry.Data["file"] = fileInfo(2) - // entry.Info(args...) - // } + c := CreateClient(DefaultNamespace) + c.SetLogLevel(LInfo) + + Print("print message") + e, ok := getEntry(c, time.Second) + if !ok { + t.Fatal("timed out") + } + // Print is an alias for Info + if e.Level != "INFO" { + t.Errorf("level = %q, want INFO", e.Level) + } + if e.Output != "print message" { + t.Errorf("output = %q, want %q", e.Output, "print message") + } + c.Destroy() +} + +func TestPrintf(t *testing.T) { + c := CreateClient(DefaultNamespace) + c.SetLogLevel(LInfo) + + Printf("formatted %s", "print") + e, ok := getEntry(c, time.Second) + if !ok { + t.Fatal("timed out") + } + if e.Output != "formatted print" { + t.Errorf("output = %q, want %q", e.Output, "formatted print") + } + c.Destroy() +} + +func TestNotice(t *testing.T) { + c := CreateClient(DefaultNamespace) + c.SetLogLevel(LNotice) + + Notice("notice message") + e, ok := getEntry(c, time.Second) + if !ok { + t.Fatal("timed out") + } + if e.Level != "NOTICE" { + t.Errorf("level = %q, want NOTICE", e.Level) + } + c.Destroy() } -// Warn prints out logs on warn level func TestWarn(t *testing.T) { - // if logLevel >= LWarn { - // entry := logger.WithFields(logrus.Fields{}) - // entry.Data["file"] = fileInfo(2) - // entry.Warn(args...) - // } + c := CreateClient(DefaultNamespace) + c.SetLogLevel(LWarn) + + Warn("warning message") + e, ok := getEntry(c, time.Second) + if !ok { + t.Fatal("timed out waiting for warn entry") + } + if e.Level != "WARN" { + t.Errorf("level = %q, want WARN", e.Level) + } + if e.Output != "warning message" { + t.Errorf("output = %q, want %q", e.Output, "warning message") + } + c.Destroy() +} + +func TestWarnf(t *testing.T) { + c := CreateClient(DefaultNamespace) + c.SetLogLevel(LWarn) + + Warnf("warn %d", 1) + e, ok := getEntry(c, time.Second) + if !ok { + t.Fatal("timed out") + } + if e.Output != "warn 1" { + t.Errorf("output = %q, want %q", e.Output, "warn 1") + } + c.Destroy() } -// Error prints out logs on error level func TestError(t *testing.T) { - // if logLevel >= LError { - // entry := logger.WithFields(logrus.Fields{}) - // entry.Data["file"] = fileInfo(2) - // entry.Error(args...) - // } + c := CreateClient(DefaultNamespace) + c.SetLogLevel(LError) + + Error("error message") + e, ok := getEntry(c, time.Second) + if !ok { + t.Fatal("timed out waiting for error entry") + } + if e.Level != "ERROR" { + t.Errorf("level = %q, want ERROR", e.Level) + } + if e.Output != "error message" { + t.Errorf("output = %q, want %q", e.Output, "error message") + } + c.Destroy() } -// Fatal prints out logs on fatal level -func TestFatal(t *testing.T) { - // if logLevel >= LFatal { - // entry := logger.WithFields(logrus.Fields{}) - // entry.Data["file"] = fileInfo(2) - // entry.Fatal(args...) - // } +func TestErrorf(t *testing.T) { + c := CreateClient(DefaultNamespace) + c.SetLogLevel(LError) + + Errorf("err: %s", "something broke") + e, ok := getEntry(c, time.Second) + if !ok { + t.Fatal("timed out") + } + if e.Output != "err: something broke" { + t.Errorf("output = %q, want %q", e.Output, "err: something broke") + } + c.Destroy() } -// Panic prints out logs on panic level func TestPanic(t *testing.T) { - // if logLevel >= LPanic { - // entry := logger.WithFields(logrus.Fields{}) - // entry.Data["file"] = fileInfo(2) - // entry.Panic(args...) - // } + c := CreateClient(DefaultNamespace) + c.SetLogLevel(LPanic) + + defer func() { + r := recover() + if r == nil { + t.Error("expected panic, got nil") + } + c.Destroy() + }() + + Panic("panic message") +} + +func TestPanicf(t *testing.T) { + c := CreateClient(DefaultNamespace) + c.SetLogLevel(LPanic) + + defer func() { + r := recover() + if r == nil { + t.Error("expected panic, got nil") + } + c.Destroy() + }() + + Panicf("panic %d", 42) +} + +func TestPanicln(t *testing.T) { + c := CreateClient(DefaultNamespace) + c.SetLogLevel(LPanic) + + defer func() { + r := recover() + if r == nil { + t.Error("expected panic, got nil") + } + c.Destroy() + }() + + Panicln("panic line") +} + +// TestLogLevelFiltering verifies that the client's log level is stored correctly. +// Note: level filtering only applies to stderr output, not to client channels. +// All entries matching the namespace are delivered to the client channel regardless of level. +func TestLogLevelFiltering(t *testing.T) { + c := CreateClient(DefaultNamespace) + c.SetLogLevel(LWarn) + + if c.GetLogLevel() != LWarn { + t.Errorf("expected log level LWarn, got %d", c.GetLogLevel()) + } + + // Both entries arrive at the client channel (level filtering is stderr-only) + Info("info message") + Warn("warn message") + + e1, ok := getEntry(c, time.Second) + if !ok { + t.Fatal("timed out waiting for first entry") + } + if e1.Output != "info message" { + t.Errorf("expected 'info message', got %q", e1.Output) + } + + e2, ok := getEntry(c, time.Second) + if !ok { + t.Fatal("timed out waiting for second entry") + } + if e2.Output != "warn message" { + t.Errorf("expected 'warn message', got %q", e2.Output) + } + c.Destroy() +} + +// TestNamespaceFiltering verifies clients only receive matching namespaces. +func TestNamespaceFiltering(t *testing.T) { + c := CreateClient("api") + c.SetLogLevel(LTrace) + + apiLogger := NewLogger("api") + dbLogger := NewLogger("database") + + // Log to database namespace — should not arrive at "api" client + dbLogger.Info("db message") + + // Log to api namespace — should arrive + apiLogger.Info("api message") + + e, ok := getEntry(c, time.Second) + if !ok { + t.Fatal("timed out waiting for api entry") + } + if e.Output != "api message" { + t.Errorf("expected 'api message', got %q", e.Output) + } + if e.Namespace != "api" { + t.Errorf("namespace = %q, want 'api'", e.Namespace) + } + c.Destroy() +} + +// TestMultiNamespaceClient verifies a client subscribed to multiple namespaces. +func TestMultiNamespaceClient(t *testing.T) { + c := CreateClient("api", "auth") + c.SetLogLevel(LTrace) + + apiLogger := NewLogger("api") + authLogger := NewLogger("auth") + dbLogger := NewLogger("database") + + dbLogger.Info("db message") // filtered out + apiLogger.Info("api message") // should arrive + authLogger.Info("auth message") // should arrive + + e1, ok := getEntry(c, time.Second) + if !ok { + t.Fatal("timed out waiting for first entry") + } + if e1.Output != "api message" { + t.Errorf("first entry = %q, want 'api message'", e1.Output) + } + + e2, ok := getEntry(c, time.Second) + if !ok { + t.Fatal("timed out waiting for second entry") + } + if e2.Output != "auth message" { + t.Errorf("second entry = %q, want 'auth message'", e2.Output) + } + c.Destroy() +} + +// TestGetNamespaces verifies the namespace registry. +func TestGetNamespaces(t *testing.T) { + l := NewLogger("test-ns-registry") + l.Info("register this namespace") + + nss := GetNamespaces() + found := false + for _, ns := range nss { + if ns == "test-ns-registry" { + found = true + break + } + } + if !found { + t.Errorf("expected 'test-ns-registry' in GetNamespaces(), got %v", nss) + } +} + +// TestLoggerDebugln verifies the Debugln method on Logger. +func TestLoggerDebugln(t *testing.T) { + c := CreateClient("debugln-test") + c.SetLogLevel(LDebug) + + l := NewLogger("debugln-test") + l.Debugln("debugln message") + + e, ok := getEntry(c, time.Second) + if !ok { + t.Fatal("timed out") + } + if e.Level != "DEBUG" { + t.Errorf("level = %q, want DEBUG", e.Level) + } + // Sprintln appends a newline + if e.Output != "debugln message\n" { + t.Errorf("output = %q, want %q", e.Output, "debugln message\n") + } + c.Destroy() +} + +// TestNewLoggerEmptyNamespace verifies empty namespace defaults to DefaultNamespace. +func TestNewLoggerEmptyNamespace(t *testing.T) { + l := NewLogger("") + if l.Namespace != DefaultNamespace { + t.Errorf("namespace = %q, want %q", l.Namespace, DefaultNamespace) + } +} + +// TestFileInfo verifies fileInfo returns a non-empty file:line string. +func TestFileInfo(t *testing.T) { + fi := fileInfo(1) + if fi == "" || fi == ":1" { + t.Errorf("fileInfo returned unexpected value: %q", fi) + } +} + +// TestColorize verifies color wrapping. +func TestColorize(t *testing.T) { + SetColorEnabled(true) + result := colorize("hello", colorRed) + expected := colorRed + "hello" + colorReset + if result != expected { + t.Errorf("colorize with color enabled: got %q, want %q", result, expected) + } + + SetColorEnabled(false) + result = colorize("hello", colorRed) + if result != "hello" { + t.Errorf("colorize with color disabled: got %q, want %q", result, "hello") + } + + // Restore default + SetColorEnabled(true) +} + +// TestParseLevelString verifies level string parsing. +func TestParseLevelString(t *testing.T) { + tests := []struct { + input string + want Level + }{ + {"TRACE", LTrace}, + {"DEBUG", LDebug}, + {"INFO", LInfo}, + {"NOTICE", LNotice}, + {"WARN", LWarn}, + {"ERROR", LError}, + {"PANIC", LPanic}, + {"FATAL", LFatal}, + {"UNKNOWN", LInfo}, // default + {"", LInfo}, // default + } + for _, tt := range tests { + got := parseLevelString(tt.input) + if got != tt.want { + t.Errorf("parseLevelString(%q) = %d, want %d", tt.input, got, tt.want) + } + } +} + +// TestBroadcast verifies the public Broadcast function. +func TestBroadcast(t *testing.T) { + c := CreateClient("broadcast-ns") + c.SetLogLevel(LTrace) + + e := Entry{ + Timestamp: time.Now(), + Output: "broadcast test", + File: "test.go:1", + Level: "WARN", + Namespace: "broadcast-ns", + } + Broadcast(e) + + got, ok := getEntry(c, time.Second) + if !ok { + t.Fatal("timed out") + } + if got.Output != "broadcast test" { + t.Errorf("output = %q, want %q", got.Output, "broadcast test") + } + if got.Level != "WARN" { + t.Errorf("level = %q, want WARN", got.Level) + } + c.Destroy() +} + +// TestMatchesNamespace verifies the namespace matching helper. +func TestMatchesNamespace(t *testing.T) { + // Client with no namespace filter matches everything + c := CreateClient() + if !c.matchesNamespace("anything") { + t.Error("empty Namespaces should match all") + } + c.Destroy() + + // Client with specific namespaces + c2 := CreateClient("api", "auth") + if !c2.matchesNamespace("api") { + t.Error("should match 'api'") + } + if !c2.matchesNamespace("auth") { + t.Error("should match 'auth'") + } + if c2.matchesNamespace("database") { + t.Error("should not match 'database'") + } + c2.Destroy() } func TestFlush(t *testing.T) { diff --git a/log/logger.go b/log/logger.go index c64d8bc..e446c3f 100644 --- a/log/logger.go +++ b/log/logger.go @@ -92,6 +92,20 @@ func (l Logger) Debugf(format string, args ...any) { createLog(e) } +// Debugln prints out logs on debug level with a newline +func (l Logger) Debugln(args ...any) { + output := fmt.Sprintln(args...) + e := Entry{ + Timestamp: time.Now(), + Output: output, + File: fileInfo(2 + l.FileInfoDepth), + Level: "DEBUG", + level: LDebug, + Namespace: l.Namespace, + } + createLog(e) +} + // Info prints out logs on info level func (l Logger) Info(args ...any) { output := fmt.Sprint(args...)