From 571ed38bb2335dd9acfdd5aa4a7b678b5d4930de Mon Sep 17 00:00:00 2001 From: Manas Srivastava Date: Fri, 22 May 2026 00:55:37 +0530 Subject: [PATCH] =?UTF-8?q?test(coverage/logctx):=20drive=20logctx=20to=20?= =?UTF-8?q?=E2=89=A595%=20(was=2087.1%)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds tests for the uncovered branches in common/logctx: - WithGroup on the wrapper handler (was 0% — entire function uncovered) now exercised via TestHandler_WithGroupPreservesService, which also asserts that service + commit_id continue to be injected on records emitted through a grouped child handler. - The `if ctx == nil` guards in WithTraceID / WithTID / WithTeamID (each 66.7% — only the happy path was hit) covered by TestKeys_WithSettersAcceptNilCtx, which passes an explicitly nil context to each setter and round-trips through the matching getter. Result: logctx coverage 87.1% → 100.0%. Co-Authored-By: Claude Opus 4.7 (1M context) --- logctx/handler_test.go | 66 ++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 66 insertions(+) diff --git a/logctx/handler_test.go b/logctx/handler_test.go index 9429ca2..85a263d 100644 --- a/logctx/handler_test.go +++ b/logctx/handler_test.go @@ -181,3 +181,69 @@ func TestHandler_WithAttrsPreservesService(t *testing.T) { t.Errorf("WithAttrs dropped extra attr") } } + +// TestHandler_WithGroupPreservesService confirms that WithGroup returns a +// wrapper that still injects service + commit_id, and that the underlying +// base handler's grouping behaviour is preserved — every subsequent attr +// lands under the named group. Without this guard a refactor that drops +// the field copy would silently emit log lines without service / commit_id +// once any caller used slog.Logger.WithGroup. +func TestHandler_WithGroupPreservesService(t *testing.T) { + buf, h := newTestHandler(t, "api") + child := h.WithGroup("grp") + if err := child.Handle(context.Background(), newRecord("hi")); err != nil { + t.Fatalf("Handle: %v", err) + } + rec := decode(t, buf) + // service + commit_id are added by the wrapper AFTER the group is + // active on the base, so they land under "grp" in JSON output. That + // is the documented behaviour for slog.Handler.WithGroup composition. + grp, ok := rec["grp"].(map[string]any) + if !ok { + t.Fatalf("expected nested grp object, got %v", rec) + } + if grp[FieldService] != "api" { + t.Errorf("WithGroup dropped service: %v", grp[FieldService]) + } + if grp[FieldCommitID] == nil { + t.Errorf("WithGroup dropped commit_id") + } +} + +// TestKeys_WithSettersAcceptNilCtx covers the `if ctx == nil` branch in +// each of WithTraceID / WithTID / WithTeamID. These branches exist to +// guard against callers passing nil — slog.Logger.Log historically did +// this when the user code passed a nil context — and the function must +// internally fall back to context.Background rather than panic on +// context.WithValue(nil, …). +func TestKeys_WithSettersAcceptNilCtx(t *testing.T) { + // Each setter must not panic on a nil ctx, and the value must be + // retrievable through the matching getter on the returned ctx. + tcs := []struct { + name string + set func(context.Context) context.Context + get func(context.Context) string + want string + }{ + {"trace_id", func(c context.Context) context.Context { return WithTraceID(c, "t-1") }, TraceIDFromContext, "t-1"}, + {"tid", func(c context.Context) context.Context { return WithTID(c, "j-2") }, TIDFromContext, "j-2"}, + {"team_id", func(c context.Context) context.Context { return WithTeamID(c, "tm-3") }, TeamIDFromContext, "tm-3"}, + } + for _, tc := range tcs { + t.Run(tc.name, func(t *testing.T) { + defer func() { + if r := recover(); r != nil { + t.Fatalf("setter panicked on nil ctx: %v", r) + } + }() + //nolint:staticcheck // intentionally passing nil ctx to exercise the guard + ctx := tc.set(nil) + if ctx == nil { + t.Fatal("setter returned nil ctx") + } + if got := tc.get(ctx); got != tc.want { + t.Errorf("getter = %q, want %q", got, tc.want) + } + }) + } +}