Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor tests for httpserver.HealthCheckHandler #5

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 1 addition & 4 deletions httpserver/health_check.go
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,6 @@ func (h *HealthCheckHandler) ServeHTTP(rw http.ResponseWriter, r *http.Request)
rw.WriteHeader(StatusClientClosedRequest)
return
}

rw.WriteHeader(http.StatusInternalServerError)
return
}
Expand All @@ -102,11 +101,9 @@ func (h *HealthCheckHandler) ServeHTTP(rw http.ResponseWriter, r *http.Request)
return
}

logger := middleware.GetLoggerFromContext(r.Context())

respStatus := http.StatusOK
if hasUnhealthyComponent {
respStatus = http.StatusServiceUnavailable
}
restapi.RespondCodeAndJSON(rw, respStatus, respData, logger)
restapi.RespondCodeAndJSON(rw, respStatus, respData, middleware.GetLoggerFromContext(r.Context()))
}
95 changes: 54 additions & 41 deletions httpserver/health_check_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ package httpserver
import (
"context"
"encoding/json"
"fmt"
"errors"
"net/http"
"net/http/httptest"
"testing"
Expand All @@ -19,24 +19,24 @@ import (

"github.com/acronis/go-libs/httpserver/middleware"
"github.com/acronis/go-libs/log"
"github.com/acronis/go-libs/log/logtest"
"github.com/acronis/go-libs/restapi"
)

func TestHealthCheckHandler_ServeHTTP(t *testing.T) {
makeRequest := func() *http.Request {
req := httptest.NewRequest(http.MethodGet, "/", nil)
return req.WithContext(middleware.NewContextWithLogger(req.Context(), log.NewDisabledLogger()))
}

t.Run("health-check returns error", func(t *testing.T) {
var internalError = errors.New("internal error")

h := NewHealthCheckHandler(func() (HealthCheckResult, error) {
return nil, fmt.Errorf("internal error")
return nil, internalError
})
resp := httptest.NewRecorder()
logRecorder := logtest.NewRecorder()

h.ServeHTTP(resp, makeRequest())
h.ServeHTTP(resp, makeHealthCheckRequest(context.Background(), logRecorder))

require.Equal(t, http.StatusInternalServerError, resp.Code)
requireHealthCheckErrorWasLogged(t, logRecorder, internalError)
})

t.Run("health-check with empty components", func(t *testing.T) {
Expand All @@ -45,7 +45,7 @@ func TestHealthCheckHandler_ServeHTTP(t *testing.T) {
})
resp := httptest.NewRecorder()

h.ServeHTTP(resp, makeRequest())
h.ServeHTTP(resp, makeHealthCheckRequest(context.Background(), log.NewDisabledLogger()))

require.Equal(t, http.StatusOK, resp.Code)
require.Equal(t, restapi.ContentTypeAppJSON, resp.Header().Get("Content-Type"))
Expand All @@ -60,7 +60,7 @@ func TestHealthCheckHandler_ServeHTTP(t *testing.T) {
})
resp := httptest.NewRecorder()

h.ServeHTTP(resp, makeRequest())
h.ServeHTTP(resp, makeHealthCheckRequest(context.Background(), log.NewDisabledLogger()))

require.Equal(t, http.StatusServiceUnavailable, resp.Code)
require.Equal(t, restapi.ContentTypeAppJSON, resp.Header().Get("Content-Type"))
Expand All @@ -79,7 +79,7 @@ func TestHealthCheckHandler_ServeHTTP(t *testing.T) {
})
resp := httptest.NewRecorder()

h.ServeHTTP(resp, makeRequest())
h.ServeHTTP(resp, makeHealthCheckRequest(context.Background(), log.NewDisabledLogger()))

require.Equal(t, http.StatusOK, resp.Code)
require.Equal(t, restapi.ContentTypeAppJSON, resp.Header().Get("Content-Type"))
Expand All @@ -94,33 +94,28 @@ func TestHealthCheckHandler_ServeHTTP(t *testing.T) {
resp := httptest.NewRecorder()

ctx, cancel := context.WithCancel(context.Background())
// cancel context immediately so default handler detects error
cancel()

ctx = middleware.NewContextWithLogger(ctx, log.NewDisabledLogger())
req := httptest.NewRequest(http.MethodGet, "/", nil).WithContext(ctx)
cancel() // cancel context immediately so default handler detects error

h.ServeHTTP(resp, req)
h.ServeHTTP(resp, makeHealthCheckRequest(ctx, log.NewDisabledLogger()))

require.Equal(t, StatusClientClosedRequest, resp.Code)
})
}

func TestHealthCheckHandlerContext_ServeHTTP(t *testing.T) {
makeRequest := func() *http.Request {
req := httptest.NewRequest(http.MethodGet, "/", nil)
return req.WithContext(middleware.NewContextWithLogger(req.Context(), log.NewDisabledLogger()))
}

t.Run("health-check returns error", func(t *testing.T) {
var internalError = errors.New("internal error")

h := NewHealthCheckHandlerContext(func(_ context.Context) (HealthCheckResult, error) {
return nil, fmt.Errorf("internal error")
return nil, internalError
})
resp := httptest.NewRecorder()
logRecorder := logtest.NewRecorder()

h.ServeHTTP(resp, makeRequest())
h.ServeHTTP(resp, makeHealthCheckRequest(context.Background(), logRecorder))

require.Equal(t, http.StatusInternalServerError, resp.Code)
requireHealthCheckErrorWasLogged(t, logRecorder, internalError)
})

t.Run("health-check with empty components", func(t *testing.T) {
Expand All @@ -129,7 +124,7 @@ func TestHealthCheckHandlerContext_ServeHTTP(t *testing.T) {
})
resp := httptest.NewRecorder()

h.ServeHTTP(resp, makeRequest())
h.ServeHTTP(resp, makeHealthCheckRequest(context.Background(), log.NewDisabledLogger()))

require.Equal(t, http.StatusOK, resp.Code)
require.Equal(t, restapi.ContentTypeAppJSON, resp.Header().Get("Content-Type"))
Expand All @@ -144,7 +139,7 @@ func TestHealthCheckHandlerContext_ServeHTTP(t *testing.T) {
})
resp := httptest.NewRecorder()

h.ServeHTTP(resp, makeRequest())
h.ServeHTTP(resp, makeHealthCheckRequest(context.Background(), log.NewDisabledLogger()))

require.Equal(t, http.StatusServiceUnavailable, resp.Code)
require.Equal(t, restapi.ContentTypeAppJSON, resp.Header().Get("Content-Type"))
Expand All @@ -163,7 +158,7 @@ func TestHealthCheckHandlerContext_ServeHTTP(t *testing.T) {
})
resp := httptest.NewRecorder()

h.ServeHTTP(resp, makeRequest())
h.ServeHTTP(resp, makeHealthCheckRequest(context.Background(), log.NewDisabledLogger()))

require.Equal(t, http.StatusOK, resp.Code)
require.Equal(t, restapi.ContentTypeAppJSON, resp.Header().Get("Content-Type"))
Expand All @@ -174,40 +169,43 @@ func TestHealthCheckHandlerContext_ServeHTTP(t *testing.T) {
})

t.Run("health-check responds error on client timeout", func(t *testing.T) {
timeout := 1 * time.Millisecond
const requestTimeout = 10 * time.Millisecond

h := NewHealthCheckHandlerContext(func(ctx context.Context) (HealthCheckResult, error) {
time.Sleep(timeout + 5*time.Millisecond)
select {
case <-ctx.Done():
case <-time.After(requestTimeout * 2):
}
return HealthCheckResult{}, ctx.Err()
})
resp := httptest.NewRecorder()
logRecorder := logtest.NewRecorder()

ctx, cancel := context.WithTimeout(context.Background(), timeout)
ctx, cancel := context.WithTimeout(context.Background(), requestTimeout)
defer cancel()
ctx = middleware.NewContextWithLogger(ctx, log.NewDisabledLogger())
req := httptest.NewRequest(http.MethodGet, "/", nil).WithContext(ctx)

h.ServeHTTP(resp, req)
h.ServeHTTP(resp, makeHealthCheckRequest(ctx, logRecorder))

require.Equal(t, http.StatusInternalServerError, resp.Code)
require.ErrorIs(t, ctx.Err(), context.DeadlineExceeded)
requireHealthCheckErrorWasLogged(t, logRecorder, ctx.Err())
})

t.Run("default HealthCheckContext function responds error on client cancel", func(t *testing.T) {
h := NewHealthCheckHandlerContext(func(ctx context.Context) (HealthCheckResult, error) {
return HealthCheckResult{}, ctx.Err()
})
resp := httptest.NewRecorder()
logRecorder := logtest.NewRecorder()

ctx, cancel := context.WithCancel(context.Background())
// cancel context immediately so default handler detects error
cancel()
cancel() // cancel context immediately so default handler detects error

ctx = middleware.NewContextWithLogger(ctx, log.NewDisabledLogger())
req := httptest.NewRequest(http.MethodGet, "/", nil).WithContext(ctx)

h.ServeHTTP(resp, req)
h.ServeHTTP(resp, makeHealthCheckRequest(ctx, logRecorder))

require.Equal(t, StatusClientClosedRequest, resp.Code)
require.ErrorIs(t, ctx.Err(), context.Canceled)
requireHealthCheckErrorWasLogged(t, logRecorder, ctx.Err())
})

t.Run("HealthCheckContext that doesn't return ctx.Err() responds error on client cancel", func(t *testing.T) {
Expand All @@ -217,8 +215,7 @@ func TestHealthCheckHandlerContext_ServeHTTP(t *testing.T) {
resp := httptest.NewRecorder()

ctx, cancel := context.WithCancel(context.Background())
// cancel context immediately so default handler detects error
cancel()
cancel() // cancel context immediately so default handler detects error

ctx = middleware.NewContextWithLogger(ctx, log.NewDisabledLogger())
req := httptest.NewRequest(http.MethodGet, "/", nil).WithContext(ctx)
Expand All @@ -228,3 +225,19 @@ func TestHealthCheckHandlerContext_ServeHTTP(t *testing.T) {
require.Equal(t, StatusClientClosedRequest, resp.Code)
})
}

func makeHealthCheckRequest(ctx context.Context, logger log.FieldLogger) *http.Request {
req := httptest.NewRequest(http.MethodGet, "/", http.NoBody)
return req.WithContext(middleware.NewContextWithLogger(ctx, logger))
}

func requireHealthCheckErrorWasLogged(t *testing.T, logRecorder *logtest.Recorder, wantErr error) {
t.Helper()
loggedEntries := logRecorder.Entries()
require.Len(t, loggedEntries, 1)
require.Equal(t, log.LevelError, loggedEntries[0].Level)
require.Contains(t, loggedEntries[0].Text, "error while checking health")
loggedErrorField, found := loggedEntries[0].FindField("error")
require.True(t, found)
require.Equal(t, wantErr, loggedErrorField.Any)
}
Loading