From 900cb750cfca327599aa5a1df6c3c0656f50c732 Mon Sep 17 00:00:00 2001 From: Snider Date: Tue, 17 Mar 2026 08:54:22 +0000 Subject: [PATCH] fix(console): buffer trim panic when limit < 100, add unit tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CLAUDE.md: update error wrapping guidance to reflect coreerr.E() convention. Console buffer trimming in both Webview.addConsoleMessage and ConsoleWatcher.addMessage panicked with slice bounds out of range when consoleLimit was smaller than 100. Use min(100, len) for safe batch trimming. Added 22 unit tests covering pure functions (FormatConsoleOutput, containsString, findString, formatJSValue, getString), ConsoleWatcher filter/count/handler logic, ExceptionWatcher operations, WaitAction context handling, and buffer limit enforcement. Coverage: 3.2% → 16.1%. DX audit findings: - Error handling: clean (all coreerr.E(), no fmt.Errorf) - File I/O: clean (no os.ReadFile/os.WriteFile — package uses HTTP/WS only) Co-Authored-By: Virgil --- CLAUDE.md | 2 +- console.go | 3 +- webview.go | 3 +- webview_test.go | 429 ++++++++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 434 insertions(+), 3 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index 048a40a..30d9b77 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -56,7 +56,7 @@ Key patterns: - Co-author trailer on every commit: `Co-Authored-By: Virgil ` - Test naming: `_Good` (happy path), `_Bad` (expected errors), `_Ugly` (panics/edge cases) - Standard `testing.T` only — no test frameworks -- Wrap errors with `fmt.Errorf("context: %w", err)` +- Wrap errors with `coreerr.E("Scope.Method", "description", err)` from `go-log`, never `fmt.Errorf` - Protect shared state with `sync.RWMutex`; copy handler slices before calling outside lock ## Docs diff --git a/console.go b/console.go index a14a956..cd5af31 100644 --- a/console.go +++ b/console.go @@ -292,7 +292,8 @@ func (cw *ConsoleWatcher) addMessage(msg ConsoleMessage) { // Enforce limit if len(cw.messages) >= cw.limit { - cw.messages = cw.messages[len(cw.messages)-cw.limit+100:] + drop := min(100, len(cw.messages)) + cw.messages = cw.messages[drop:] } cw.messages = append(cw.messages, msg) diff --git a/webview.go b/webview.go index ede519f..58d0ff0 100644 --- a/webview.go +++ b/webview.go @@ -412,7 +412,8 @@ func (wv *Webview) addConsoleMessage(msg ConsoleMessage) { if len(wv.consoleLogs) >= wv.consoleLimit { // Remove oldest messages - wv.consoleLogs = wv.consoleLogs[len(wv.consoleLogs)-wv.consoleLimit+100:] + drop := min(100, len(wv.consoleLogs)) + wv.consoleLogs = wv.consoleLogs[drop:] } wv.consoleLogs = append(wv.consoleLogs, msg) } diff --git a/webview_test.go b/webview_test.go index df3ae61..cbecc51 100644 --- a/webview_test.go +++ b/webview_test.go @@ -1,6 +1,7 @@ package webview import ( + "context" "testing" "time" ) @@ -333,3 +334,431 @@ func TestScrollIntoViewAction_Good(t *testing.T) { t.Errorf("Expected selector '#target', got %q", action.Selector) } } + +// TestFormatConsoleOutput_Good verifies console output formatting. +func TestFormatConsoleOutput_Good(t *testing.T) { + ts := time.Date(2026, 1, 15, 14, 30, 45, 123000000, time.UTC) + messages := []ConsoleMessage{ + {Type: "error", Text: "something broke", Timestamp: ts}, + {Type: "warning", Text: "deprecated call", Timestamp: ts}, + {Type: "info", Text: "loaded", Timestamp: ts}, + {Type: "debug", Text: "trace data", Timestamp: ts}, + {Type: "log", Text: "hello world", Timestamp: ts}, + } + + output := FormatConsoleOutput(messages) + + expected := []string{ + "14:30:45.123 [ERROR] something broke", + "14:30:45.123 [WARN] deprecated call", + "14:30:45.123 [INFO] loaded", + "14:30:45.123 [DEBUG] trace data", + "14:30:45.123 [LOG] hello world", + } + for _, exp := range expected { + if !containsString(output, exp) { + t.Errorf("Expected output to contain %q", exp) + } + } +} + +// TestFormatConsoleOutput_Good_Empty verifies empty message list. +func TestFormatConsoleOutput_Good_Empty(t *testing.T) { + output := FormatConsoleOutput(nil) + if output != "" { + t.Errorf("Expected empty string, got %q", output) + } +} + +// TestContainsString_Good verifies substring matching. +func TestContainsString_Good(t *testing.T) { + tests := []struct { + s, substr string + want bool + }{ + {"hello world", "world", true}, + {"hello world", "hello", true}, + {"hello world", "xyz", false}, + {"hello", "", true}, + {"", "", true}, + {"", "a", false}, + {"abc", "abc", true}, + {"abc", "abcd", false}, + } + + for _, tc := range tests { + got := containsString(tc.s, tc.substr) + if got != tc.want { + t.Errorf("containsString(%q, %q) = %v, want %v", tc.s, tc.substr, got, tc.want) + } + } +} + +// TestFindString_Good verifies string search. +func TestFindString_Good(t *testing.T) { + tests := []struct { + s, substr string + want int + }{ + {"hello world", "world", 6}, + {"hello world", "hello", 0}, + {"hello world", "xyz", -1}, + {"abcabc", "abc", 0}, + {"abc", "abc", 0}, + } + + for _, tc := range tests { + got := findString(tc.s, tc.substr) + if got != tc.want { + t.Errorf("findString(%q, %q) = %d, want %d", tc.s, tc.substr, got, tc.want) + } + } +} + +// TestFormatJSValue_Good verifies JavaScript value formatting. +func TestFormatJSValue_Good(t *testing.T) { + tests := []struct { + input any + want string + }{ + {"hello", `"hello"`}, + {true, "true"}, + {false, "false"}, + {nil, "null"}, + {42, "42"}, + {3.14, "3.14"}, + } + + for _, tc := range tests { + got := formatJSValue(tc.input) + if got != tc.want { + t.Errorf("formatJSValue(%v) = %q, want %q", tc.input, got, tc.want) + } + } +} + +// TestGetString_Good verifies map string extraction. +func TestGetString_Good(t *testing.T) { + m := map[string]any{ + "name": "test", + "count": 42, + } + + if got := getString(m, "name"); got != "test" { + t.Errorf("getString(m, 'name') = %q, want 'test'", got) + } + if got := getString(m, "count"); got != "" { + t.Errorf("getString(m, 'count') = %q, want empty (not a string)", got) + } + if got := getString(m, "missing"); got != "" { + t.Errorf("getString(m, 'missing') = %q, want empty", got) + } +} + +// TestWaitAction_Good_ContextCancelled verifies WaitAction respects context cancellation. +func TestWaitAction_Good_ContextCancelled(t *testing.T) { + ctx, cancel := context.WithCancel(context.Background()) + cancel() // Cancel immediately + + action := WaitAction{Duration: 10 * time.Second} + err := action.Execute(ctx, nil) + if err == nil { + t.Error("Expected context cancelled error") + } +} + +// TestWaitAction_Good_ShortWait verifies WaitAction completes after duration. +func TestWaitAction_Good_ShortWait(t *testing.T) { + ctx := context.Background() + action := WaitAction{Duration: 10 * time.Millisecond} + + start := time.Now() + err := action.Execute(ctx, nil) + elapsed := time.Since(start) + + if err != nil { + t.Fatalf("Unexpected error: %v", err) + } + if elapsed < 10*time.Millisecond { + t.Errorf("Expected at least 10ms elapsed, got %v", elapsed) + } +} + +// TestAddConsoleMessage_Good verifies console message buffer management. +func TestAddConsoleMessage_Good(t *testing.T) { + wv := &Webview{ + consoleLogs: make([]ConsoleMessage, 0, 10), + consoleLimit: 5, + } + + // Add messages up to the limit + for i := range 6 { + wv.addConsoleMessage(ConsoleMessage{ + Type: "log", + Text: time.Duration(i).String(), + }) + } + + // Buffer should have been trimmed + if len(wv.consoleLogs) > wv.consoleLimit { + t.Errorf("Expected at most %d messages, got %d", wv.consoleLimit, len(wv.consoleLogs)) + } +} + +// TestConsoleWatcherFilter_Good verifies console watcher filter matching. +func TestConsoleWatcherFilter_Good(t *testing.T) { + // Create a minimal ConsoleWatcher without a real Webview + cw := &ConsoleWatcher{ + messages: make([]ConsoleMessage, 0), + filters: make([]ConsoleFilter, 0), + limit: 1000, + handlers: make([]ConsoleHandler, 0), + } + + // No filters — everything matches + msg := ConsoleMessage{Type: "error", Text: "test error"} + if !cw.matchesFilter(msg) { + t.Error("Expected message to match with no filters") + } + + // Add type filter + cw.AddFilter(ConsoleFilter{Type: "error"}) + if !cw.matchesFilter(msg) { + t.Error("Expected error message to match error filter") + } + + logMsg := ConsoleMessage{Type: "log", Text: "test log"} + if cw.matchesFilter(logMsg) { + t.Error("Expected log message NOT to match error filter") + } + + // Add pattern filter + cw.ClearFilters() + cw.AddFilter(ConsoleFilter{Pattern: "hello"}) + helloMsg := ConsoleMessage{Type: "log", Text: "hello world"} + if !cw.matchesFilter(helloMsg) { + t.Error("Expected 'hello world' to match pattern 'hello'") + } + if cw.matchesFilter(msg) { + t.Error("Expected 'test error' NOT to match pattern 'hello'") + } +} + +// TestConsoleWatcherCounts_Good verifies console watcher counting methods. +func TestConsoleWatcherCounts_Good(t *testing.T) { + cw := &ConsoleWatcher{ + messages: []ConsoleMessage{ + {Type: "log", Text: "info 1"}, + {Type: "error", Text: "err 1"}, + {Type: "log", Text: "info 2"}, + {Type: "error", Text: "err 2"}, + {Type: "warning", Text: "warn 1"}, + }, + filters: make([]ConsoleFilter, 0), + limit: 1000, + handlers: make([]ConsoleHandler, 0), + } + + if cw.Count() != 5 { + t.Errorf("Expected count 5, got %d", cw.Count()) + } + if cw.ErrorCount() != 2 { + t.Errorf("Expected error count 2, got %d", cw.ErrorCount()) + } + if !cw.HasErrors() { + t.Error("Expected HasErrors() to be true") + } + + errors := cw.Errors() + if len(errors) != 2 { + t.Errorf("Expected 2 errors, got %d", len(errors)) + } + + warnings := cw.Warnings() + if len(warnings) != 1 { + t.Errorf("Expected 1 warning, got %d", len(warnings)) + } + + cw.Clear() + if cw.Count() != 0 { + t.Errorf("Expected count 0 after clear, got %d", cw.Count()) + } + if cw.HasErrors() { + t.Error("Expected HasErrors() to be false after clear") + } +} + +// TestExceptionWatcher_Good verifies exception watcher basic operations. +func TestExceptionWatcher_Good(t *testing.T) { + ew := &ExceptionWatcher{ + exceptions: make([]ExceptionInfo, 0), + handlers: make([]func(ExceptionInfo), 0), + } + + if ew.HasExceptions() { + t.Error("Expected no exceptions initially") + } + if ew.Count() != 0 { + t.Errorf("Expected count 0, got %d", ew.Count()) + } + + // Simulate adding an exception + ew.exceptions = append(ew.exceptions, ExceptionInfo{ + Text: "TypeError: undefined is not a function", + LineNumber: 10, + URL: "https://example.com/app.js", + }) + + if !ew.HasExceptions() { + t.Error("Expected HasExceptions() to be true") + } + if ew.Count() != 1 { + t.Errorf("Expected count 1, got %d", ew.Count()) + } + + exceptions := ew.Exceptions() + if len(exceptions) != 1 { + t.Errorf("Expected 1 exception, got %d", len(exceptions)) + } + if exceptions[0].Text != "TypeError: undefined is not a function" { + t.Errorf("Unexpected exception text: %q", exceptions[0].Text) + } + + ew.Clear() + if ew.Count() != 0 { + t.Errorf("Expected count 0 after clear, got %d", ew.Count()) + } +} + +// TestAngularRouterState_Good verifies AngularRouterState struct. +func TestAngularRouterState_Good(t *testing.T) { + state := AngularRouterState{ + URL: "/dashboard", + Fragment: "section1", + Params: map[string]string{"id": "123"}, + QueryParams: map[string]string{ + "tab": "settings", + }, + } + + if state.URL != "/dashboard" { + t.Errorf("Expected URL '/dashboard', got %q", state.URL) + } + if state.Fragment != "section1" { + t.Errorf("Expected fragment 'section1', got %q", state.Fragment) + } + if state.Params["id"] != "123" { + t.Errorf("Expected param id '123', got %q", state.Params["id"]) + } + if state.QueryParams["tab"] != "settings" { + t.Errorf("Expected query param tab 'settings', got %q", state.QueryParams["tab"]) + } +} + +// TestTargetInfo_Good verifies TargetInfo struct. +func TestTargetInfo_Good(t *testing.T) { + target := TargetInfo{ + ID: "ABC123", + Type: "page", + Title: "Example", + URL: "https://example.com", + WebSocketDebuggerURL: "ws://localhost:9222/devtools/page/ABC123", + } + + if target.ID != "ABC123" { + t.Errorf("Expected ID 'ABC123', got %q", target.ID) + } + if target.Type != "page" { + t.Errorf("Expected type 'page', got %q", target.Type) + } + if target.WebSocketDebuggerURL == "" { + t.Error("Expected WebSocketDebuggerURL to be set") + } +} + +// TestConsoleWatcherAddMessage_Good verifies message buffer limit enforcement. +func TestConsoleWatcherAddMessage_Good(t *testing.T) { + cw := &ConsoleWatcher{ + messages: make([]ConsoleMessage, 0), + filters: make([]ConsoleFilter, 0), + limit: 5, + handlers: make([]ConsoleHandler, 0), + } + + // Add messages past the limit + for i := range 7 { + cw.addMessage(ConsoleMessage{ + Type: "log", + Text: time.Duration(i).String(), + }) + } + + if len(cw.messages) > cw.limit { + t.Errorf("Expected at most %d messages, got %d", cw.limit, len(cw.messages)) + } +} + +// TestConsoleWatcherHandler_Good verifies handlers are called for new messages. +func TestConsoleWatcherHandler_Good(t *testing.T) { + cw := &ConsoleWatcher{ + messages: make([]ConsoleMessage, 0), + filters: make([]ConsoleFilter, 0), + limit: 1000, + handlers: make([]ConsoleHandler, 0), + } + + var received ConsoleMessage + cw.AddHandler(func(msg ConsoleMessage) { + received = msg + }) + + cw.addMessage(ConsoleMessage{Type: "error", Text: "handler test"}) + + if received.Text != "handler test" { + t.Errorf("Handler not called or wrong message: got %q", received.Text) + } +} + +// TestConsoleWatcherFilteredMessages_Good verifies filtered message retrieval. +func TestConsoleWatcherFilteredMessages_Good(t *testing.T) { + cw := &ConsoleWatcher{ + messages: []ConsoleMessage{ + {Type: "log", Text: "info msg"}, + {Type: "error", Text: "error msg"}, + {Type: "log", Text: "another info"}, + }, + filters: []ConsoleFilter{{Type: "error"}}, + limit: 1000, + handlers: make([]ConsoleHandler, 0), + } + + filtered := cw.FilteredMessages() + if len(filtered) != 1 { + t.Fatalf("Expected 1 filtered message, got %d", len(filtered)) + } + if filtered[0].Type != "error" { + t.Errorf("Expected error type, got %q", filtered[0].Type) + } +} + +// TestExceptionInfo_Good verifies ExceptionInfo struct. +func TestExceptionInfo_Good(t *testing.T) { + info := ExceptionInfo{ + Text: "ReferenceError: foo is not defined", + LineNumber: 42, + ColumnNumber: 10, + URL: "https://example.com/app.js", + StackTrace: " at bar (app.js:42:10)\n", + Timestamp: time.Now(), + } + + if info.Text != "ReferenceError: foo is not defined" { + t.Errorf("Unexpected text: %q", info.Text) + } + if info.LineNumber != 42 { + t.Errorf("Expected line 42, got %d", info.LineNumber) + } + if info.StackTrace == "" { + t.Error("Expected stack trace to be set") + } +}