diff --git a/api/log/log.go b/api/log/log.go index 3ddfe901..687d61c6 100644 --- a/api/log/log.go +++ b/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 diff --git a/api/log/log_test.go b/api/log/log_test.go index 28a6badf..7c08b771 100644 --- a/api/log/log_test.go +++ b/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)), - } -}