clean up; don't keep original Fmt, consolidate tests, generate golden files for new test cases

Signed-off-by: Callum Styan <[email protected]>

Author: cstyan

internal/entryhuman/entry.go

@@ -179,108 +179,7 @@ func writeValueFast(w io.Writer, v interface{}) (bool, error) {
 	}
 }
 
-func Fmt(
-	buf interface {
-		io.StringWriter
-		io.Writer
-	}, termW io.Writer, ent slog.SinkEntry,
-) {
-	reset(buf, termW)
-	ts := ent.Time.Format(TimeFormat)
-	buf.WriteString(render(termW, timeStyle, ts+" "))
-
-	level := ent.Level.String()
-	level = strings.ToLower(level)
-	if len(level) > 4 {
-		level = level[:4]
-	}
-	level = "[" + level + "]"
-	buf.WriteString(render(termW, levelStyle(ent.Level), level))
-	buf.WriteString("  ")
-
-	if len(ent.LoggerNames) > 0 {
-		loggerName := quoteKey(strings.Join(ent.LoggerNames, ".")) + ": "
-		buf.WriteString(loggerName)
-	}
-
-	var multilineKey string
-	var multilineVal string
-	msg := strings.TrimSpace(ent.Message)
-	if strings.Contains(msg, "\n") {
-		multilineKey = "msg"
-		multilineVal = msg
-		msg = "..."
-		msg = quote(msg)
-	}
-	buf.WriteString(msg)
-
-	if ent.SpanContext.IsValid() {
-		ent.Fields = append(slog.M(
-			slog.F("trace", ent.SpanContext.TraceID),
-			slog.F("span", ent.SpanContext.SpanID),
-		), ent.Fields...)
-	}
-
-	for i, f := range ent.Fields {
-		if multilineVal != "" {
-			break
-		}
-
-		var s string
-		switch v := f.Value.(type) {
-		case string:
-			s = v
-		case error, xerrors.Formatter:
-			s = fmt.Sprintf("%+v", v)
-		}
-		s = strings.TrimSpace(s)
-		if !strings.Contains(s, "\n") {
-			continue
-		}
-
-		// Remove this field.
-		ent.Fields = append(ent.Fields[:i], ent.Fields[i+1:]...)
-		multilineKey = f.Name
-		multilineVal = s
-	}
-
-	keyStyle := timeStyle
-	// Help users distinguish logs by keeping some color in the equal signs.
-	equalsStyle := timeStyle
-
-	for i, f := range ent.Fields {
-		if i < len(ent.Fields) {
-			buf.WriteString(tab)
-		}
-		buf.WriteString(render(termW, keyStyle, quoteKey(f.Name)))
-		buf.WriteString(render(termW, equalsStyle, "="))
-		valueStr := formatValue(f.Value)
-		buf.WriteString(valueStr)
-	}
-
-	if multilineVal != "" {
-		if msg != "..." {
-			buf.WriteString(" ...")
-		}
-
-		// Proper indentation.
-		lines := strings.Split(multilineVal, "\n")
-		for i, line := range lines[1:] {
-			if line != "" {
-				lines[i+1] = strings.Repeat(" ", len(multilineKey)+2) + line
-			}
-		}
-		multilineVal = strings.Join(lines, "\n")
-
-		multilineKey = render(termW, keyStyle, multilineKey)
-		buf.WriteString("\n")
-		buf.WriteString(multilineKey)
-		buf.WriteString("= ")
-		buf.WriteString(multilineVal)
-	}
-}
-
-// OptimizedFmt returns a human readable format for ent. Assumes we have a bytes.Buffer
+// Fmt returns a human readable format for ent. Assumes we have a bytes.Buffer
 // which we will more easily be able to assume underlying reallocation of it's size is possible
 // if necessary than for an arbitrary io.Writer/io.StringWriter
 // Note that while bytes.Buffer can in theory return an error for writes, it only does so if the buffer size will
@@ -292,7 +191,7 @@ func Fmt(
 // We also do not indent the fields as go's test does that automatically
 // for extra lines in a log so if we did it here, the fields would be indented
 // twice in test logs. So the Stderr logger indents all the fields itself.
-func OptimizedFmt(buf *bytes.Buffer, termW io.Writer, ent slog.SinkEntry) {
+func Fmt(buf *bytes.Buffer, termW io.Writer, ent slog.SinkEntry) {
 	reset(buf, termW)
 
 	// Timestamp + space

internal/entryhuman/entry_test.go

@@ -142,191 +142,6 @@ func TestEntry(t *testing.T) {
 				),
 			},
 		},
-	}
-	if *updateGoldenFiles {
-		ents, err := os.ReadDir("testdata")
-		if err != nil {
-			t.Fatal(err)
-		}
-		for _, ent := range ents {
-			os.Remove("testdata/" + ent.Name())
-		}
-	}
-
-	for _, tc := range ents {
-		tc := tc
-		t.Run(tc.name, func(t *testing.T) {
-			t.Parallel()
-			goldenPath := fmt.Sprintf("testdata/%s.golden", tc.name)
-
-			var gotBuf bytes.Buffer
-			entryhuman.Fmt(&gotBuf, io.Discard, tc.ent)
-
-			if *updateGoldenFiles {
-				err := os.WriteFile(goldenPath, gotBuf.Bytes(), 0o644)
-				if err != nil {
-					t.Fatal(err)
-				}
-				return
-			}
-
-			wantByt, err := os.ReadFile(goldenPath)
-			if err != nil {
-				t.Fatal(err)
-			}
-
-			assert.Equal(t, "entry matches", string(wantByt), gotBuf.String())
-		})
-	}
-
-	t.Run("isTTY during file close", func(t *testing.T) {
-		t.Parallel()
-
-		tmpdir := t.TempDir()
-		f, err := os.CreateTemp(tmpdir, "slog")
-		if err != nil {
-			t.Fatal(err)
-		}
-		defer f.Close()
-
-		done := make(chan struct{}, 2)
-		go func() {
-			entryhuman.Fmt(new(bytes.Buffer), f, slog.SinkEntry{
-				Level: slog.LevelCritical,
-				Fields: slog.M(
-					slog.F("hey", "hi"),
-				),
-			})
-			done <- struct{}{}
-		}()
-		go func() {
-			_ = f.Close()
-			done <- struct{}{}
-		}()
-		<-done
-		<-done
-	})
-}
-
-// Verifies that OptimizedFmt returtns the same result as Fmt.
-// We can remove this if we are okay with removing the existing Fmt function and replacing it with OptimizedFmt (can return an error).
-func TestEntry_Optimized(t *testing.T) {
-	t.Parallel()
-
-	type tcase struct {
-		name string
-		ent  slog.SinkEntry
-	}
-
-	ents := []tcase{
-		{
-			"simpleNoFields",
-			slog.SinkEntry{
-				Message: "wowowow\tizi",
-				Time:    kt,
-				Level:   slog.LevelDebug,
-
-				File: "myfile",
-				Line: 100,
-				Func: "mypkg.ignored",
-			},
-		},
-		{
-			"multilineMessage",
-			slog.SinkEntry{
-				Message: "line1\nline2",
-				Level:   slog.LevelInfo,
-			},
-		},
-		{
-			"multilineField",
-			slog.SinkEntry{
-				Message: "msg",
-				Level:   slog.LevelInfo,
-				Fields:  slog.M(slog.F("field", "line1\nline2")),
-			},
-		},
-		{
-			"named",
-			slog.SinkEntry{
-				Level:       slog.LevelWarn,
-				LoggerNames: []string{"some", "cat"},
-				Message:     "meow",
-				Fields: slog.M(
-					slog.F("breath", "stinky"),
-				),
-			},
-		},
-		{
-			"funky",
-			slog.SinkEntry{
-				Level: slog.LevelWarn,
-				Fields: slog.M(
-					slog.F("funky^%&^&^key", "value"),
-					slog.F("funky^%&^&^key2", "@#\t \t \n"),
-				),
-			},
-		},
-		{
-			"spacey",
-			slog.SinkEntry{
-				Level: slog.LevelWarn,
-				Fields: slog.M(
-					slog.F("space in my key", "value in my value"),
-				),
-			},
-		},
-		{
-			"nil",
-			slog.SinkEntry{
-				Level: slog.LevelWarn,
-				Fields: slog.M(
-					slog.F("nan", nil),
-				),
-			},
-		},
-		{
-			"bytes",
-			slog.SinkEntry{
-				Level: slog.LevelWarn,
-				Fields: slog.M(
-					slog.F("somefile", []byte("blah bla\x01h blah")),
-				),
-			},
-		},
-		{
-			"driverValue",
-			slog.SinkEntry{
-				Level: slog.LevelWarn,
-				Fields: slog.M(
-					slog.F("val", sql.NullString{String: "dog", Valid: true}),
-					slog.F("inval", sql.NullString{String: "cat", Valid: false}),
-				),
-			},
-		},
-		{
-			"object",
-			slog.SinkEntry{
-				Level: slog.LevelWarn,
-				Fields: slog.M(
-					slog.F("obj", slog.M(
-						slog.F("obj1", testObj{
-							foo: 1,
-							bar: 2,
-							dra: []byte("blah"),
-						}),
-						slog.F("obj2", testObj{
-							foo: 3,
-							bar: 4,
-							dra: []byte("blah"),
-						}),
-					)),
-					slog.F("map", map[string]string{
-						"key1": "value1",
-					}),
-				),
-			},
-		},
 		{
 			"primitiveTypes",
 			slog.SinkEntry{
@@ -401,19 +216,39 @@ func TestEntry_Optimized(t *testing.T) {
 			},
 		},
 	}
+	if *updateGoldenFiles {
+		ents, err := os.ReadDir("testdata")
+		if err != nil {
+			t.Fatal(err)
+		}
+		for _, ent := range ents {
+			os.Remove("testdata/" + ent.Name())
+		}
+	}
 
 	for _, tc := range ents {
 		tc := tc
 		t.Run(tc.name, func(t *testing.T) {
 			t.Parallel()
+			goldenPath := fmt.Sprintf("testdata/%s.golden", tc.name)
+
+			var gotBuf bytes.Buffer
+			entryhuman.Fmt(&gotBuf, io.Discard, tc.ent)
 
-			var fmtBuf bytes.Buffer
-			var optBuf bytes.Buffer
+			if *updateGoldenFiles {
+				err := os.WriteFile(goldenPath, gotBuf.Bytes(), 0o644)
+				if err != nil {
+					t.Fatal(err)
+				}
+				return
+			}
 
-			entryhuman.Fmt(&fmtBuf, io.Discard, tc.ent)
-			entryhuman.OptimizedFmt(&optBuf, io.Discard, tc.ent)
+			wantByt, err := os.ReadFile(goldenPath)
+			if err != nil {
+				t.Fatal(err)
+			}
 
-			assert.Equal(t, "outputs match", fmtBuf.String(), optBuf.String())
+			assert.Equal(t, "entry matches", string(wantByt), gotBuf.String())
 		})
 	}
 
@@ -429,7 +264,7 @@ func TestEntry_Optimized(t *testing.T) {
 
 		done := make(chan struct{}, 2)
 		go func() {
-			entryhuman.OptimizedFmt(new(bytes.Buffer), f, slog.SinkEntry{
+			entryhuman.Fmt(new(bytes.Buffer), f, slog.SinkEntry{
 				Level: slog.LevelCritical,
 				Fields: slog.M(
 					slog.F("hey", "hi"),
@@ -472,7 +307,7 @@ func BenchmarkFmt(b *testing.B) {
 				b.ResetTimer()
 				b.ReportAllocs()
 				for i := 0; i < b.N; i++ {
-					entryhuman.OptimizedFmt(bytes.NewBuffer(nil), w, se)
+					entryhuman.Fmt(bytes.NewBuffer(nil), w, se)
 				}
 			})
 		}