Clean up slog testing and restore coverage

logr_test.go

@@ -24,85 +24,6 @@ import (
 	"testing"
 )
 
-// testLogSink is a Logger just for testing that calls optional hooks on each method.
-type testLogSink struct {
-	fnInit       func(ri RuntimeInfo)
-	fnEnabled    func(lvl int) bool
-	fnInfo       func(lvl int, msg string, kv ...any)
-	fnError      func(err error, msg string, kv ...any)
-	fnWithValues func(kv ...any)
-	fnWithName   func(name string)
-
-	withValues []any
-
-	// testSlogSink contains some additional fields if (and only if) slog is supported by Go.
-	// See logr_slog_test.go.
-	//nolint:unused // Only unused with Go < 1.21.
-	testSlogSink
-}
-
-var _ LogSink = &testLogSink{}
-
-func (l *testLogSink) Init(ri RuntimeInfo) {
-	if l.fnInit != nil {
-		l.fnInit(ri)
-	}
-}
-
-func (l *testLogSink) Enabled(lvl int) bool {
-	if l.fnEnabled != nil {
-		return l.fnEnabled(lvl)
-	}
-	return false
-}
-
-func (l *testLogSink) Info(lvl int, msg string, kv ...any) {
-	if l.fnInfo != nil {
-		l.fnInfo(lvl, msg, kv...)
-	}
-}
-
-func (l *testLogSink) Error(err error, msg string, kv ...any) {
-	if l.fnError != nil {
-		l.fnError(err, msg, kv...)
-	}
-}
-
-func (l *testLogSink) WithValues(kv ...any) LogSink {
-	if l.fnWithValues != nil {
-		l.fnWithValues(kv...)
-	}
-	out := *l
-	n := len(out.withValues)
-	out.withValues = append(out.withValues[:n:n], kv...)
-	return &out
-}
-
-func (l *testLogSink) WithName(name string) LogSink {
-	if l.fnWithName != nil {
-		l.fnWithName(name)
-	}
-	out := *l
-	return &out
-}
-
-type testCallDepthLogSink struct {
-	testLogSink
-	callDepth       int
-	fnWithCallDepth func(depth int)
-}
-
-var _ CallDepthLogSink = &testCallDepthLogSink{}
-
-func (l *testCallDepthLogSink) WithCallDepth(depth int) LogSink {
-	if l.fnWithCallDepth != nil {
-		l.fnWithCallDepth(depth)
-	}
-	out := *l
-	out.callDepth += depth
-	return &out
-}
-
 func TestNew(t *testing.T) {
 	calledInit := 0
 

sloghandler.go

@@ -70,9 +70,7 @@ func (l *slogHandler) Handle(ctx context.Context, record slog.Record) error {
 
 	kvList := make([]any, 0, 2*record.NumAttrs())
 	record.Attrs(func(attr slog.Attr) bool {
-		if attr.Key != "" {
-			kvList = append(kvList, l.addGroupPrefix(attr.Key), attr.Value.Resolve().Any())
-		}
+		kvList = attrToKVs(attr, l.groupPrefix, kvList)
 		return true
 	})
 	if record.Level >= slog.LevelError {
@@ -114,9 +112,7 @@ func (l *slogHandler) WithAttrs(attrs []slog.Attr) slog.Handler {
 	} else {
 		kvList := make([]any, 0, 2*len(attrs))
 		for _, attr := range attrs {
-			if attr.Key != "" {
-				kvList = append(kvList, l.addGroupPrefix(attr.Key), attr.Value.Resolve().Any())
-			}
+			kvList = attrToKVs(attr, l.groupPrefix, kvList)
 		}
 		clone.sink = l.sink.WithValues(kvList...)
 	}
@@ -136,16 +132,41 @@ func (l *slogHandler) WithGroup(name string) slog.Handler {
 		clone.slogSink = l.slogSink.WithGroup(name)
 		clone.sink = clone.slogSink
 	} else {
-		clone.groupPrefix = clone.addGroupPrefix(name)
+		clone.groupPrefix = addPrefix(clone.groupPrefix, name)
 	}
 	return &clone
 }
 
-func (l *slogHandler) addGroupPrefix(name string) string {
-	if l.groupPrefix == "" {
+// attrToKVs appends a slog.Attr to a logr-style kvList.  It handle slog Groups
+// and other details of slog.
+func attrToKVs(attr slog.Attr, groupPrefix string, kvList []any) []any {
+	attrVal := attr.Value.Resolve()
+	if attrVal.Kind() == slog.KindGroup {
+		groupVal := attrVal.Group()
+		grpKVs := make([]any, 0, 2*len(groupVal))
+		prefix := groupPrefix
+		if attr.Key != "" {
+			prefix = addPrefix(groupPrefix, attr.Key)
+		}
+		for _, attr := range groupVal {
+			grpKVs = attrToKVs(attr, prefix, grpKVs)
+		}
+		kvList = append(kvList, grpKVs...)
+	} else if attr.Key != "" {
+		kvList = append(kvList, addPrefix(groupPrefix, attr.Key), attrVal.Any())
+	}
+
+	return kvList
+}
+
+func addPrefix(prefix, name string) string {
+	if prefix == "" {
 		return name
 	}
-	return l.groupPrefix + groupSeparator + name
+	if name == "" {
+		return prefix
+	}
+	return prefix + groupSeparator + name
 }
 
 // levelFromSlog adjusts the level by the logger's verbosity and negates it.