test: add benchmarks, adversarial tests, and improved fuzz coverage#50
test: add benchmarks, adversarial tests, and improved fuzz coverage#50
Conversation
Benchmarks (benchmark_test.go): - All handler types: Fanout, Failover, Pool, Router, FirstMatch, Pipe, Recovery - Sub-benchmarks varying handler counts, failure modes, match positions - Parallel benchmarks (b.RunParallel) for all core handlers - Composition benchmarks: Fanout-of-Failover, Pipe-with-Router, deep nesting - WithAttrs/WithGroup mutation benchmarks Adversarial tests (stress_test.go): - Random failure handler (deterministic chaos) with Fanout/Failover - Deep nesting (5 levels) under concurrent load - Concurrent derive (WithAttrs/WithGroup) + Handle on same base handler - Mixed panic types (string, error, int, struct) under concurrency - Pool distribution fairness verification (>1% per handler) - Router with 20 predicates under concurrent load - Recovery with concurrent mixed panic types - High contention (1000 goroutines x 1000 logs, single handler) New fuzz targets (fuzz_test.go): - FuzzPipeHandle: middleware chain with variable depth - FuzzRecoveryHandle: error/panic/nil combinations - FuzzAttrValueIs: attribute predicate with arbitrary strings - FuzzRouterWithAttrs: router + predicate matching via logger.With()
There was a problem hiding this comment.
Pull request overview
This PR expands the test suite for the slogmulti handlers by adding benchmarks, adversarial/concurrency stress tests, and several new fuzz targets to improve coverage of handler composition and failure modes.
Changes:
- Added a new
benchmark_test.gocovering serial/parallel benchmarks for core handlers and common compositions. - Extended
stress_test.gowith adversarial scenarios (random failures, deep nesting, concurrent derive+handle, mixed panic types, pool distribution, high contention). - Added new fuzz targets in
fuzz_test.gofor Pipe, Recovery, attribute predicates, and Router behavior withlogger.With(...).
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| stress_test.go | Adds new adversarial/concurrency stress tests and a deterministic failure handler. |
| fuzz_test.go | Adds fuzz targets for Pipe/Recovery and predicate/router behavior. |
| benchmark_test.go | Introduces comprehensive benchmarks for handlers and compositions (including parallel benchmarks). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| go func() { | ||
| defer wg.Done() | ||
| for i := 0; i < 100; i++ { | ||
| r := slog.NewRecord(time.Now(), slog.LevelInfo, "test", 0) | ||
| err := handler.Handle(ctx, r) | ||
| assert.Error(t, err) | ||
| } |
There was a problem hiding this comment.
assert.Error(t, err) is being called from multiple goroutines. Using *testing.T (including via testify's assert/require helpers) concurrently is not safe and will introduce races/flaky failures, especially under -race. Collect errors in a concurrency-safe way (e.g., atomic counter/channel) and perform assertions after wg.Wait().
| b.Run(fmt.Sprintf("handlers=%d", n), func(b *testing.B) { | ||
| b.ReportAllocs() | ||
| handler := Fanout(makeHandlers(n)...) | ||
| ctx := context.Background() | ||
| for i := 0; i < b.N; i++ { | ||
| _ = handler.Handle(ctx, benchRecord()) | ||
| } |
There was a problem hiding this comment.
These benchmarks call benchRecord() inside the hot loop, which includes time.Now() and AddAttrs work that can dominate results and make it hard to interpret handler overhead. Consider constructing the slog.Record once (or outside the timed section via b.StopTimer/b.StartTimer or b.ResetTimer) and reusing it so the benchmark primarily measures the handler logic.
| var inner slog.Handler | ||
| switch mode % 4 { | ||
| case 0: | ||
| inner = &errorHandler{err: nil} | ||
| case 1: | ||
| inner = &errorHandler{err: errors.New("fail")} | ||
| case 2: | ||
| inner = &panickingHandler{panicValue: "boom"} | ||
| case 3: | ||
| inner = &panickingHandler{panicValue: errors.New("panic-err")} | ||
| } |
There was a problem hiding this comment.
In this fuzz target, mode can be negative, and mode % 4 will also be negative in Go. That leaves inner unset (nil) and will panic when calling recovery(inner). Normalize mode into the range [0..3] before the switch (e.g., adjust negative values) so the handler is always initialized.
* perf(pool): eliminate slice allocation in Handle hot path Replace slice copy with index arithmetic for round-robin handler selection. This removes 1 allocation per log record (48-160B depending on handler count), yielding 13-37% latency improvement. * perf(inline): prealloc slices in WithAttrs and WithGroup Use make() with exact capacity instead of empty slice literals followed by double appends. Eliminates unnecessary reallocation during slice growth in InlineHandler and HandleInlineHandler. * test: add benchmarks, adversarial tests, and improved fuzz coverage (#50) * fix: address PR #51 review feedback - Fix Codecov condition: matrix.go == '1.22' (was '1.21' which no longer exists in the matrix) - Benchmark consistency: reuse pre-created record in all non-parallel benchmarks to avoid measuring benchRecord() allocation overhead - Deduplicate fuzz mode normalization: compute ((mode%4)+4)%4 once into normalizedMode variable
Benchmarks (benchmark_test.go):
Adversarial tests (stress_test.go):
New fuzz targets (fuzz_test.go):