Skip to content

Commit 0385e14

Browse files
committed
Add comments around slog exceptions
1 parent 8fd86d2 commit 0385e14

File tree

1 file changed

+11
-6
lines changed

1 file changed

+11
-6
lines changed

slogr_test.go

Lines changed: 11 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -114,15 +114,20 @@ func TestWithCallDepth(t *testing.T) {
114114
}
115115
}
116116

117-
func TestRunSlogTestsOnSlogHandlerLogSink(t *testing.T) {
117+
func TestRunSlogTestsOnNaiveSlogHandler(t *testing.T) {
118118
// This proves that slogHandler passes slog's own tests when given a
119-
// non-SlogSink LogSink.
119+
// LogSink which does not implement SlogSink.
120120
exceptions := []string{
121121
// logr sinks handle time themselves
122122
"a Handler should ignore a zero Record.Time",
123123
// slogHandler does not do groups "properly", so these all fail with
124124
// "missing group". It's looking for `"G":{"a":"b"}` and getting
125125
// `"G.a": "b"`.
126+
//
127+
// NOTE: These make a weird coupling to Go versions. Newer Go versions
128+
// don't need some of these exceptions, but older ones do. It's unclear
129+
// if that is because something changed in slog or if the test was
130+
// removed.
126131
"a Handler should handle Group attributes",
127132
"a Handler should handle the WithGroup method",
128133
"a Handler should handle multiple WithGroup and WithAttr calls",
@@ -137,21 +142,21 @@ func TestRunSlogTestsOnSlogHandlerLogSink(t *testing.T) {
137142
// or SlogSink (since those get special treatment). We can trust that
138143
// the slog JSONHandler works.
139144
handler := slog.NewJSONHandler(buffer, nil)
140-
sink := &passthruLogSink{handler: handler}
145+
sink := &passthruLogSink{handler: handler} // passthruLogSink does not implement SlogSink.
141146
logger := New(sink)
142147
return ToSlogHandler(logger)
143148
}, exceptions...)
144149
}
145150

146-
func TestRunSlogTestsOnSlogHandlerSlogSink(t *testing.T) {
151+
func TestRunSlogTestsOnEnlightenedSlogHandler(t *testing.T) {
147152
// This proves that slogHandler passes slog's own tests when given a
148-
// SlogSink.
153+
// LogSink which implements SlogSink.
149154
exceptions := []string{}
150155
testhelp.RunSlogTests(t, func(buffer *bytes.Buffer) slog.Handler {
151156
// We want a known-good Logger that emits JSON and implements SlogSink,
152157
// to cover those paths. We can trust that the slog JSONHandler works.
153158
handler := slog.NewJSONHandler(buffer, nil)
154-
sink := &passthruSlogSink{handler: handler}
159+
sink := &passthruSlogSink{handler: handler} // passthruSlogSink implements SlogSink.
155160
logger := New(sink)
156161
return ToSlogHandler(logger)
157162
}, exceptions...)

0 commit comments

Comments
 (0)