Refactor tests stylistically

api/log/log.go

@@ -21,6 +21,7 @@ type StackTracedError interface {
 
 type fieldCarrier interface {
 	WithFields(map[string]any)
+	Fields() map[string]any
 }
 
 // Error adds to the response writer the given error if it implements

api/log/log_test.go

@@ -1,8 +1,8 @@
 package log
 
 import (
+	"net/http"
 	"net/http/httptest"
-	"strconv"
 	"testing"
 	"unsafe"
 
@@ -12,61 +12,68 @@ import (
 	"github.com/smallstep/certificates/logging"
 )
 
+type stackTracedError struct{}
+
+func (stackTracedError) Error() string {
+	return "a stacktraced error"
+}
+
+func (stackTracedError) StackTrace() pkgerrors.StackTrace {
+	f := struct{}{}
+	return pkgerrors.StackTrace{ // fake stacktrace
+		pkgerrors.Frame(unsafe.Pointer(&f)),
+		pkgerrors.Frame(unsafe.Pointer(&f)),
+	}
+}
+
 func TestError(t *testing.T) {
-	cases := []struct {
+	tests := []struct {
+		name string
 		error
+		rw               http.ResponseWriter
+		isFieldCarrier   bool
 		stepDebug        bool
 		expectStackTrace bool
 	}{
-		0: {nil, false, false},
-		1: {nil, true, false},
-		2: {assert.AnError, false, false},
-		3: {assert.AnError, true, false},
-		4: {new(stackTracedError), false, false},
-		5: {new(stackTracedError), true, true},
+		{"noLogger", nil, nil, false, false, false},
+		{"noError", nil, logging.NewResponseLogger(httptest.NewRecorder()), true, false, false},
+		{"noErrorDebug", nil, logging.NewResponseLogger(httptest.NewRecorder()), true, true, false},
+		{"anError", assert.AnError, logging.NewResponseLogger(httptest.NewRecorder()), true, false, false},
+		{"anErrorDebug", assert.AnError, logging.NewResponseLogger(httptest.NewRecorder()), true, true, false},
+		{"stackTracedError", new(stackTracedError), logging.NewResponseLogger(httptest.NewRecorder()), true, true, true},
+		{"stackTracedErrorDebug", new(stackTracedError), logging.NewResponseLogger(httptest.NewRecorder()), true, true, true},
 	}
 
-	for caseIndex := range cases {
-		kase := cases[caseIndex]
-
-		t.Run(strconv.Itoa(caseIndex), func(t *testing.T) {
-			if kase.stepDebug {
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			if tt.stepDebug {
 				t.Setenv("STEPDEBUG", "1")
 			} else {
 				t.Setenv("STEPDEBUG", "0")
 			}
 
-			rw := logging.NewResponseLogger(httptest.NewRecorder())
-			Error(rw, kase.error)
+			Error(tt.rw, tt.error)
 
-			fields := rw.Fields()
+			// return early if test case doesn't use logger
+			if !tt.isFieldCarrier {
+				return
+			}
 
-			// expect the error field to be set and to be the same error that was fed to Error
-			if kase.error == nil {
+			fields := tt.rw.(logging.ResponseLogger).Fields()
+
+			// expect the error field to be (not) set and to be the same error that was fed to Error
+			if tt.error == nil {
 				assert.Nil(t, fields["error"])
 			} else {
-				assert.Same(t, kase.error, fields["error"])
+				assert.Same(t, tt.error, fields["error"])
 			}
 
-			if _, hasStackTrace := fields["stack-trace"]; kase.expectStackTrace && !hasStackTrace {
-				t.Error("stack-trace was not set")
-			} else if !kase.expectStackTrace && hasStackTrace {
-				t.Error("stack-trace was set")
+			// check if stack-trace is set when expected
+			if _, hasStackTrace := fields["stack-trace"]; tt.expectStackTrace && !hasStackTrace {
+				t.Error(`ResponseLogger["stack-trace"] not set`)
+			} else if !tt.expectStackTrace && hasStackTrace {
+				t.Error(`ResponseLogger["stack-trace"] was set`)
 			}
 		})
 	}
 }
-
-type stackTracedError struct{}
-
-func (stackTracedError) Error() string {
-	return "a stacktraced error"
-}
-
-func (stackTracedError) StackTrace() pkgerrors.StackTrace {
-	f := struct{}{}
-	return pkgerrors.StackTrace{ // fake stacktrace
-		pkgerrors.Frame(unsafe.Pointer(&f)),
-		pkgerrors.Frame(unsafe.Pointer(&f)),
-	}
-}