logstore: drive-by test cleanup

Epic: none
Release note: none

Author: pav-kv

pkg/kv/kvserver/logstore/sideload_test.go

@@ -13,7 +13,6 @@ import (
 	"os"
 	"path/filepath"
 	"reflect"
-	"regexp"
 	"sort"
 	"strconv"
 	"strings"
@@ -368,18 +367,6 @@ func TestRaftSSTableSideloadingInline(t *testing.T) {
 	v1, v2 := raftlog.EntryEncodingStandardWithAC, raftlog.EntryEncodingSideloadedWithAC
 	rangeID := roachpb.RangeID(1)
 
-	type testCase struct {
-		// Entry passed into maybeInlineSideloadedRaftCommand and the entry
-		// after having (perhaps) been modified.
-		thin, fat raftpb.Entry
-		// Populate the raft entry cache and sideload storage before running the test.
-		setup func(*raftentry.Cache, SideloadStorage)
-		// If nonempty, the error expected from maybeInlineSideloadedRaftCommand.
-		expErr string
-		// If nonempty, a regex that the recorded trace span must match.
-		expTrace string
-	}
-
 	sstFat := kvserverpb.ReplicatedEvalResult_AddSSTable{
 		Data:  []byte("foo"),
 		CRC32: 0, // not checked
@@ -388,87 +375,84 @@ func TestRaftSSTableSideloadingInline(t *testing.T) {
 		CRC32: 0, // not checked
 	}
 
-	putOnDisk := func(ec *raftentry.Cache, ss SideloadStorage) {
-		if err := ss.Put(context.Background(), 5, 6, sstFat.Data); err != nil {
-			t.Fatal(err)
-		}
+	putOnDisk := func(_ *raftentry.Cache, ss SideloadStorage) {
+		require.NoError(t, ss.Put(context.Background(), 5, 6, sstFat.Data))
 	}
 
-	testCases := map[string]testCase{
+	for _, test := range []struct {
+		name string
+		// Entry passed into maybeInlineSideloadedRaftCommand and the entry
+		// after having (perhaps) been modified.
+		thin, fat raftpb.Entry
+		// Populate the raft entry cache and sideload storage before running the test.
+		setup func(*raftentry.Cache, SideloadStorage)
+		// If nonempty, the error expected from maybeInlineSideloadedRaftCommand.
+		expErr string
+		// If nonempty, a regex that the recorded trace span must match.
+		expTrace string
+	}{
 		// Plain old v1 Raft command without payload. Don't touch.
-		"v1-no-payload": {thin: mkEnt(v1, 5, 6, &sstThin), fat: mkEnt(v1, 5, 6, &sstThin)},
+		{name: "v1-no-payload", thin: mkEnt(v1, 5, 6, &sstThin), fat: mkEnt(v1, 5, 6, &sstThin)},
 		// With payload, but command is v1. Don't touch. Note that the
 		// first of the two shouldn't happen in practice or we have a
 		// huge problem once we try to apply this entry.
-		"v1-slim-with-payload": {thin: mkEnt(v1, 5, 6, &sstThin), fat: mkEnt(v1, 5, 6, &sstThin)},
-		"v1-with-payload":      {thin: mkEnt(v1, 5, 6, &sstFat), fat: mkEnt(v1, 5, 6, &sstFat)},
+		{name: "v1-slim-with-payload", thin: mkEnt(v1, 5, 6, &sstThin), fat: mkEnt(v1, 5, 6, &sstThin)},
+		{name: "v1-with-payload", thin: mkEnt(v1, 5, 6, &sstFat), fat: mkEnt(v1, 5, 6, &sstFat)},
 		// v2 with payload, but payload is AWOL. This would be fatal in practice.
-		"v2-with-payload-missing-file": {
+		{
+			name: "v2-with-payload-missing-file",
 			thin: mkEnt(v2, 5, 6, &sstThin), fat: mkEnt(v2, 5, 6, &sstThin),
 			expErr: "not found",
 		},
 		// v2 with payload that's actually there. The request we'll see in
 		// practice.
-		"v2-with-payload-with-file-no-cache": {
+		{
+			name: "v2-with-payload-with-file-no-cache",
 			thin: mkEnt(v2, 5, 6, &sstThin), fat: mkEnt(v2, 5, 6, &sstFat),
 			setup: putOnDisk, expTrace: "inlined entry not cached",
 		},
-		"v2-with-payload-with-file-with-cache": {
+		{
+			name: "v2-with-payload-with-file-with-cache",
 			thin: mkEnt(v2, 5, 6, &sstThin), fat: mkEnt(v2, 5, 6, &sstFat),
 			setup: func(ec *raftentry.Cache, ss SideloadStorage) {
 				putOnDisk(ec, ss)
 				ec.Add(rangeID, []raftpb.Entry{mkEnt(v2, 5, 6, &sstFat)}, true)
 			}, expTrace: "using cache hit",
 		},
-		"v2-fat-without-file": {
+		{
+			name: "v2-fat-without-file",
 			thin: mkEnt(v2, 5, 6, &sstFat), fat: mkEnt(v2, 5, 6, &sstFat),
 			setup:    func(ec *raftentry.Cache, ss SideloadStorage) {},
 			expTrace: "already inlined",
 		},
-	}
-
-	runOne := func(k string, test testCase) {
-		ctx, getRecAndFinish := tracing.ContextWithRecordingSpan(
-			context.Background(), tracing.NewTracer(), "test-recording")
-		defer getRecAndFinish()
-
-		eng := storage.NewDefaultInMemForTesting()
-		defer eng.Close()
-		ss := newTestingSideloadStorage(eng)
-		ec := raftentry.NewCache(1024) // large enough
-		if test.setup != nil {
-			test.setup(ec, ss)
-		}
+	} {
+		t.Run(test.name, func(t *testing.T) {
+			ctx, getRecAndFinish := tracing.ContextWithRecordingSpan(
+				context.Background(), tracing.NewTracer(), "test-recording")
+			defer getRecAndFinish()
 
-		thinCopy := *(protoutil.Clone(&test.thin).(*raftpb.Entry))
-		newEnt, err := MaybeInlineSideloadedRaftCommand(ctx, rangeID, thinCopy, ss, ec)
-		if err != nil {
-			if test.expErr == "" || !testutils.IsError(err, test.expErr) {
-				t.Fatalf("%s: %+v", k, err)
+			eng := storage.NewDefaultInMemForTesting()
+			defer eng.Close()
+			ss := newTestingSideloadStorage(eng)
+			ec := raftentry.NewCache(1024) // large enough
+			if test.setup != nil {
+				test.setup(ec, ss)
 			}
-		} else if test.expErr != "" {
-			t.Fatalf("%s: success, but expected error: %s", k, test.expErr)
-		} else {
-			mustEntryEq(t, thinCopy, test.thin)
-		}
-		mustEntryEq(t, newEnt, test.fat)
 
-		if dump := getRecAndFinish().String(); test.expTrace != "" {
-			if ok, err := regexp.MatchString(test.expTrace, dump); err != nil {
-				t.Fatalf("%s: %+v", k, err)
-			} else if !ok {
-				t.Fatalf("%s: expected trace matching:\n%s\n\nbut got\n%s", k, test.expTrace, dump)
+			thinCopy := *(protoutil.Clone(&test.thin).(*raftpb.Entry))
+			newEnt, err := MaybeInlineSideloadedRaftCommand(ctx, rangeID, thinCopy, ss, ec)
+			if want := test.expErr; want != "" {
+				require.ErrorContains(t, err, want)
+			} else {
+				require.NoError(t, err)
+				mustEntryEq(t, thinCopy, test.thin)
 			}
-		}
-	}
+			mustEntryEq(t, newEnt, test.fat)
 
-	keys := make([]string, 0, len(testCases))
-	for k := range testCases {
-		keys = append(keys, k)
-	}
-	sort.Strings(keys)
-	for _, k := range keys {
-		runOne(k, testCases[k])
+			if want := test.expTrace; want != "" {
+				require.Contains(t, getRecAndFinish().String(), want)
+			}
+		})
 	}
 }