Put slog tests in a helper, move funcr test

funcr/slogsink_test.go

@@ -20,13 +20,15 @@ limitations under the License.
 package funcr
 
 import (
+	"bytes"
 	"fmt"
 	"log/slog"
 	"path/filepath"
 	"runtime"
 	"testing"
 
 	"github.com/go-logr/logr"
+	"github.com/go-logr/logr/internal/testhelp"
 )
 
 func TestSlogSink(t *testing.T) {
@@ -108,3 +110,44 @@ func TestSlogSinkWithCaller(t *testing.T) {
 		t.Errorf("\nexpected %q\n     got %q", expect, capt.log)
 	}
 }
+
+func TestRunSlogTests(t *testing.T) {
+	fn := func(buffer *bytes.Buffer) slog.Handler {
+		printfn := func(obj string) {
+			fmt.Fprintln(buffer, obj)
+		}
+		opts := Options{
+			LogTimestamp: true,
+			Verbosity:    10,
+			RenderBuiltinsHook: func(kvList []any) []any {
+				mappedKVList := make([]any, len(kvList))
+				for i := 0; i < len(kvList); i += 2 {
+					key := kvList[i]
+					switch key {
+					case "ts":
+						mappedKVList[i] = "time"
+					default:
+						mappedKVList[i] = key
+					}
+					mappedKVList[i+1] = kvList[i+1]
+				}
+				return mappedKVList
+			},
+		}
+		logger := NewJSON(printfn, opts)
+		return logr.ToSlogHandler(logger)
+	}
+	exceptions := []string{
+		"a Handler should ignore a zero Record.Time", // Time is generated by sink.
+	}
+	testhelp.RunSlogTests(t, fn, exceptions...)
+}
+
+func TestLogrSlogConversion(t *testing.T) {
+	f := New(func(prefix, args string) {}, Options{})
+	f2 := logr.FromSlogHandler(logr.ToSlogHandler(f))
+	if want, got := f, f2; got != want {
+		t.Helper()
+		t.Errorf("Expected %T %+v, got instead: %T %+v", want, want, got, got)
+	}
+}

internal/testhelp/slog.go

@@ -0,0 +1,79 @@
+//go:build go1.21
+// +build go1.21
+
+/*
+Copyright 2023 The logr Authors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+// Package testhelp holds helper functions for the testing of logr and built-in
+// implementations.
+package testhelp
+
+import (
+	"bytes"
+	"encoding/json"
+	"log/slog"
+	"strings"
+	"testing"
+	"testing/slogtest"
+)
+
+// RunSlogTests runs slogtest.TestHandler on a given slog.Handler, which is
+// expected to emit JSON into the provided buffer.
+func RunSlogTests(t *testing.T, createHandler func(buffer *bytes.Buffer) slog.Handler, exceptions ...string) {
+	var buffer bytes.Buffer
+	handler := createHandler(&buffer)
+	err := slogtest.TestHandler(handler, func() []map[string]any {
+		var ms []map[string]any
+		for _, line := range bytes.Split(buffer.Bytes(), []byte{'\n'}) {
+			if len(line) == 0 {
+				continue
+			}
+			var m map[string]any
+			if err := json.Unmarshal(line, &m); err != nil {
+				t.Errorf("%v: %q", err, string(line))
+			}
+			ms = append(ms, m)
+		}
+		return ms
+	})
+
+	// Correlating failures with individual test cases is hard with the current API.
+	// See https://github.com/golang/go/issues/61758
+	t.Logf("Output:\n%s", buffer.String())
+	if err != nil {
+		if unwrappable, ok := err.(interface {
+			Unwrap() []error
+		}); ok {
+			for _, err := range unwrappable.Unwrap() {
+				if !containsOne(err.Error(), exceptions...) {
+					t.Errorf("Unexpected error: %v", err)
+				}
+			}
+		} else {
+			// Shouldn't be reached, errors from errors.Join can be split up.
+			t.Errorf("Unexpected errors:\n%v", err)
+		}
+	}
+}
+
+func containsOne(hay string, needles ...string) bool {
+	for _, needle := range needles {
+		if strings.Contains(hay, needle) {
+			return true
+		}
+	}
+	return false
+}

internal/testhelp/slog_test.go

@@ -0,0 +1,33 @@
+//go:build go1.21
+// +build go1.21
+
+/*
+Copyright 2023 The logr Authors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package testhelp
+
+import (
+	"bytes"
+	"log/slog"
+	"testing"
+)
+
+func TestRunSlogTestsOnSlogSink(t *testing.T) {
+	// This proves that RunSlogTests works.
+	RunSlogTests(t, func(buffer *bytes.Buffer) slog.Handler {
+		return slog.NewJSONHandler(buffer, nil)
+	})
+}

slogr_test.go

@@ -21,7 +21,6 @@ package logr_test
 
 import (
 	"bytes"
-	"encoding/json"
 	"errors"
 	"fmt"
 	"io"
@@ -31,10 +30,10 @@ import (
 	"runtime"
 	"strings"
 	"testing"
-	"testing/slogtest"
 
 	"github.com/go-logr/logr"
 	"github.com/go-logr/logr/funcr"
+	"github.com/go-logr/logr/internal/testhelp"
 )
 
 var debugWithoutTime = &slog.HandlerOptions{
@@ -105,92 +104,16 @@ func TestWithCallDepth(t *testing.T) {
 	}
 }
 
-func TestJSONHandler(t *testing.T) {
+func TestRunSlogTestsOnSlogSink(t *testing.T) {
 	// This proves that slogSink passes slog's own tests.
-	testSlog(t, func(buffer *bytes.Buffer) slog.Handler {
-		return slog.NewJSONHandler(buffer, nil)
-	})
-}
-
-func TestFuncrHandler(t *testing.T) {
-	fn := func(buffer *bytes.Buffer) slog.Handler {
-		printfn := func(obj string) {
-			fmt.Fprintln(buffer, obj)
-		}
-		opts := funcr.Options{
-			LogTimestamp: true,
-			Verbosity:    10,
-			RenderBuiltinsHook: func(kvList []any) []any {
-				mappedKVList := make([]any, len(kvList))
-				for i := 0; i < len(kvList); i += 2 {
-					key := kvList[i]
-					switch key {
-					case "ts":
-						mappedKVList[i] = "time"
-					default:
-						mappedKVList[i] = key
-					}
-					mappedKVList[i+1] = kvList[i+1]
-				}
-				return mappedKVList
-			},
-		}
-		logger := funcr.NewJSON(printfn, opts)
+	testhelp.RunSlogTests(t, func(buffer *bytes.Buffer) slog.Handler {
+		handler := slog.NewJSONHandler(buffer, nil)
+		logger := logr.FromSlogHandler(handler)
 		return logr.ToSlogHandler(logger)
-	}
-	exceptions := []string{
-		"a Handler should ignore a zero Record.Time", // Time is generated by sink.
-	}
-	testSlog(t, fn, exceptions...)
-}
-
-func testSlog(t *testing.T, createHandler func(buffer *bytes.Buffer) slog.Handler, exceptions ...string) {
-	var buffer bytes.Buffer
-	handler := createHandler(&buffer)
-	err := slogtest.TestHandler(handler, func() []map[string]any {
-		var ms []map[string]any
-		for _, line := range bytes.Split(buffer.Bytes(), []byte{'\n'}) {
-			if len(line) == 0 {
-				continue
-			}
-			var m map[string]any
-			if err := json.Unmarshal(line, &m); err != nil {
-				t.Errorf("%v: %q", err, string(line))
-			}
-			ms = append(ms, m)
-		}
-		return ms
 	})
-
-	// Correlating failures with individual test cases is hard with the current API.
-	// See https://github.com/golang/go/issues/61758
-	t.Logf("Output:\n%s", buffer.String())
-	if err != nil {
-		if unwrappable, ok := err.(interface {
-			Unwrap() []error
-		}); ok {
-			for _, err := range unwrappable.Unwrap() {
-				if !containsOne(err.Error(), exceptions...) {
-					t.Errorf("Unexpected error: %v", err)
-				}
-			}
-		} else {
-			// Shouldn't be reached, errors from errors.Join can be split up.
-			t.Errorf("Unexpected errors:\n%v", err)
-		}
-	}
 }
 
-func containsOne(hay string, needles ...string) bool {
-	for _, needle := range needles {
-		if strings.Contains(hay, needle) {
-			return true
-		}
-	}
-	return false
-}
-
-func TestDiscard(_ *testing.T) {
+func TestSlogSinkOnDiscard(_ *testing.T) {
 	// Compile-test
 	logger := slog.New(logr.ToSlogHandler(logr.Discard()))
 	logger.WithGroup("foo").With("x", 1).Info("hello")
@@ -205,10 +128,6 @@ func TestConversion(t *testing.T) {
 	e2 := logr.FromSlogHandler(logr.ToSlogHandler(e))
 	expectEqual(t, e, e2)
 
-	f := funcr.New(func(prefix, args string) {}, funcr.Options{})
-	f2 := logr.FromSlogHandler(logr.ToSlogHandler(f))
-	expectEqual(t, f, f2)
-
 	text := slog.NewTextHandler(io.Discard, nil)
 	text2 := logr.ToSlogHandler(logr.FromSlogHandler(text))
 	expectEqual(t, text, text2)