Improve unit tests on middlewares

.gitignore

@@ -1 +1,2 @@
-vendor/
+vendor/
+.test_coverage.txt

Makefile

@@ -1,10 +1,11 @@
 
-UNIT_TEST_CMD := go test `go list ./... | grep -v test\/integration` -race
-INTEGRATION_TEST_CMD := go test ./test/integration -race -tags='integration'
-BENCHMARK_CMD := go test `go list ./... | grep -v vendor` -benchmem -bench=.
-CHECK_CMD = golangci-lint run -E goimports
-DEPS_CMD := go mod tidy
-MOCKS_CMD := go generate ./internal/mocks
+UNIT_TEST_CMD 			:= go test `go list ./... | grep -v test\/integration` -race -coverprofile=.test_coverage.txt && \
+						   	go tool cover -func=.test_coverage.txt | tail -n1 | awk '{print "Total test coverage: " $$3}'
+INTEGRATION_TEST_CMD 	:= go test ./test/integration -race -tags='integration'
+BENCHMARK_CMD 			:= go test `go list ./... | grep -v vendor` -benchmem -bench=.
+CHECK_CMD 				:= golangci-lint run -E goimports
+DEPS_CMD 				:= go mod tidy
+MOCKS_CMD 				:= go generate ./internal/mocks
 
 help: ## Show this help.
 	@echo "Help"

go.mod

@@ -7,7 +7,7 @@ require (
 	github.com/julienschmidt/httprouter v1.3.0
 	github.com/labstack/echo/v4 v4.1.16
 	github.com/prometheus/client_golang v1.6.0
-	github.com/stretchr/testify v1.5.1
+	github.com/stretchr/testify v1.6.0
 	github.com/urfave/negroni v1.0.0
 	go.opencensus.io v0.22.3
 )

go.sum

@@ -106,6 +106,7 @@ github.com/modern-go/reflect2 v1.0.1 h1:9f412s+6RmYXLWZSEzVVgPGK7C2PphHj5RJrvfx9
 github.com/modern-go/reflect2 v1.0.1/go.mod h1:bx2lNnkwVCuqBIxFjflWJWanXIb3RllmbCylyMrvgv0=
 github.com/mwitkow/go-conntrack v0.0.0-20161129095857-cc309e4a2223/go.mod h1:qRWi+5nqEBWmkhHvq77mSJWrCKwh8bxhgT7d/eI7P4U=
 github.com/pkg/errors v0.8.0/go.mod h1:bwawxfHBFNV+L2hUp1rHADufV3IMtnDRdf1r5NINEl0=
+github.com/pkg/errors v0.8.1 h1:iURUrRGxPUNPdy5/HRSm+Yj6okJ6UtLINN0Q9M4+h3I=
 github.com/pkg/errors v0.8.1/go.mod h1:bwawxfHBFNV+L2hUp1rHADufV3IMtnDRdf1r5NINEl0=
 github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZbAQM=
 github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4=
@@ -143,8 +144,8 @@ github.com/stretchr/testify v1.3.0 h1:TivCn/peBQ7UY8ooIcPgZFpTNSz0Q2U6UrFlUfqbe0
 github.com/stretchr/testify v1.3.0/go.mod h1:M5WIy9Dh21IEIfnGCwXGc5bZfKNJtfHm1UVUgZn+9EI=
 github.com/stretchr/testify v1.4.0 h1:2E4SXV/wtOkTonXsotYi4li6zVWxYlZuYNCXe9XRJyk=
 github.com/stretchr/testify v1.4.0/go.mod h1:j7eGeouHqKxXV5pUuKE4zz7dFj8WfuZ+81PSLYec5m4=
-github.com/stretchr/testify v1.5.1 h1:nOGnQDM7FYENwehXlg/kFVnos3rEvtKTjRvOWSzb6H4=
-github.com/stretchr/testify v1.5.1/go.mod h1:5W2xD1RspED5o8YsWQXVCued0rvSQ+mT+I5cxcmMvtA=
+github.com/stretchr/testify v1.6.0 h1:jlIyCplCJFULU/01vCkhKuTyc3OorI3bJFuw6obfgho=
+github.com/stretchr/testify v1.6.0/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg=
 github.com/ugorji/go v1.1.7 h1:/68gy2h+1mWMrwZFeD1kQialdSzAb432dtpeJ42ovdo=
 github.com/ugorji/go v1.1.7/go.mod h1:kZn38zHttfInRq0xu/PH0az30d+z6vm202qpg1oXVMw=
 github.com/ugorji/go/codec v1.1.7 h1:2SvQaVZ1ouYrrKKwoSk2pzd4A9evlKJb9oTL+OaLUSs=
@@ -241,4 +242,6 @@ gopkg.in/yaml.v2 v2.2.4/go.mod h1:hI93XBmqTisBFMUTm0b8Fm+jr3Dg1NNxqwp+5A1VGuI=
 gopkg.in/yaml.v2 v2.2.5/go.mod h1:hI93XBmqTisBFMUTm0b8Fm+jr3Dg1NNxqwp+5A1VGuI=
 gopkg.in/yaml.v2 v2.2.8 h1:obN1ZagJSUGI0Ek/LBmuj4SNLPfIny3KsKFopxRdj10=
 gopkg.in/yaml.v2 v2.2.8/go.mod h1:hI93XBmqTisBFMUTm0b8Fm+jr3Dg1NNxqwp+5A1VGuI=
+gopkg.in/yaml.v3 v3.0.0-20200313102051-9f266ea9e77c h1:dUUwHk2QECo/6vqA44rthZ8ie2QXMNeKRTHCNY2nXvo=
+gopkg.in/yaml.v3 v3.0.0-20200313102051-9f266ea9e77c/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM=
 honnef.co/go/tools v0.0.0-20190102054323-c2f93a96b099/go.mod h1:rf3lG4BRIbNafJWhAfAdb/ePZxsR/4RtNHQocxwk9r4=

middleware/echo/echo.go

@@ -1,5 +1,4 @@
-// Package echo is a helper package to get an echo compatible
-// handler/middleware from the standard net/http Middleware factory.
+// Package echo is a helper package to get an echo compatible middleware
 package echo
 
 import (

middleware/echo/echo_test.go

@@ -1,79 +1,91 @@
 package echo_test
 
 import (
+	"io/ioutil"
 	"net/http"
 	"net/http/httptest"
 	"testing"
 
 	"github.com/labstack/echo/v4"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/mock"
+	"github.com/stretchr/testify/require"
+
 	mmetrics "github.com/slok/go-http-metrics/internal/mocks/metrics"
 	"github.com/slok/go-http-metrics/metrics"
 	"github.com/slok/go-http-metrics/middleware"
 	echoMiddleware "github.com/slok/go-http-metrics/middleware/echo"
-	"github.com/stretchr/testify/assert"
-	"github.com/stretchr/testify/mock"
 )
 
-func getTestHandler(statusCode int) echo.HandlerFunc {
-	return func(context echo.Context) error {
-		context.Response().WriteHeader(statusCode)
-		return nil
-	}
-}
-
 func TestMiddleware(t *testing.T) {
 	tests := map[string]struct {
-		handlerID     string
-		statusCode    int
-		req           *http.Request
-		config        middleware.Config
-		expHandlerID  string
-		expService    string
-		expMethod     string
-		expStatusCode string
+		handlerID   string
+		config      middleware.Config
+		req         func() *http.Request
+		mock        func(m *mmetrics.Recorder)
+		handler     func() echo.HandlerFunc
+		expRespCode int
+		expRespBody string
 	}{
 		"A default HTTP middleware should call the recorder to measure.": {
-			statusCode:    http.StatusAccepted,
-			req:           httptest.NewRequest(http.MethodPost, "/test", nil),
-			expHandlerID:  "/test",
-			expMethod:     http.MethodPost,
-			expStatusCode: "202",
+			req: func() *http.Request {
+				return httptest.NewRequest(http.MethodPost, "/test", nil)
+			},
+			mock: func(m *mmetrics.Recorder) {
+				expHTTPReqProps := metrics.HTTPReqProperties{
+					ID:      "/test",
+					Service: "",
+					Method:  "POST",
+					Code:    "202",
+				}
+				m.On("ObserveHTTPRequestDuration", mock.Anything, expHTTPReqProps, mock.Anything).Once()
+				m.On("ObserveHTTPResponseSize", mock.Anything, expHTTPReqProps, int64(5)).Once()
+
+				expHTTPProps := metrics.HTTPProperties{
+					ID:      "/test",
+					Service: "",
+				}
+				m.On("AddInflightRequests", mock.Anything, expHTTPProps, 1).Once()
+				m.On("AddInflightRequests", mock.Anything, expHTTPProps, -1).Once()
+			},
+			handler: func() echo.HandlerFunc {
+				return func(context echo.Context) error {
+					resp := context.Response()
+					resp.WriteHeader(202)
+					_, err := resp.Write([]byte("test1"))
+					return err
+				}
+			},
+			expRespCode: 202,
+			expRespBody: "test1",
 		},
 	}
 
 	for name, test := range tests {
 		t.Run(name, func(t *testing.T) {
 			assert := assert.New(t)
+			require := require.New(t)
 
 			// Mocks.
 			mr := &mmetrics.Recorder{}
-			expHTTPReqProps := metrics.HTTPReqProperties{
-				ID:      test.expHandlerID,
-				Service: test.expService,
-				Method:  test.expMethod,
-				Code:    test.expStatusCode,
-			}
-			expHTTPProps := metrics.HTTPProperties{
-				ID:      test.expHandlerID,
-				Service: test.expService,
-			}
-			mr.On("ObserveHTTPRequestDuration", mock.Anything, expHTTPReqProps, mock.Anything).Once()
-			mr.On("ObserveHTTPResponseSize", mock.Anything, expHTTPReqProps, mock.Anything).Once()
-			mr.On("AddInflightRequests", mock.Anything, expHTTPProps, 1).Once()
-			mr.On("AddInflightRequests", mock.Anything, expHTTPProps, -1).Once()
+			test.mock(mr)
 
-			// Create our echo instance with the middleware.
+			// Create our instance with the middleware.
 			mdlw := middleware.New(middleware.Config{Recorder: mr})
 			e := echo.New()
-			e.POST("/test", getTestHandler(test.statusCode), echoMiddleware.Handler("", mdlw))
+			req := test.req()
+			e.Add(req.Method, req.URL.Path, test.handler(), echoMiddleware.Handler(test.handlerID, mdlw))
 
 			// Make the request.
 			resp := httptest.NewRecorder()
-			e.ServeHTTP(resp, test.req)
+			e.ServeHTTP(resp, test.req())
 
 			// Check.
 			mr.AssertExpectations(t)
-			assert.Equal(test.statusCode, resp.Result().StatusCode)
+			assert.Equal(test.expRespCode, resp.Result().StatusCode)
+			gotBody, err := ioutil.ReadAll(resp.Result().Body)
+			require.NoError(err)
+			assert.Equal(test.expRespBody, string(gotBody))
 		})
 	}
 }

middleware/gin/gin.go

@@ -1,5 +1,4 @@
-// Package gin is a helper package to get a gin compatible
-// handler/middleware from the standard net/http Middleware factory.
+// Package gin is a helper package to get a gin compatible middleware.
 package gin
 
 import (

middleware/gin/gin_test.go

@@ -1,81 +1,120 @@
 package gin_test
 
 import (
+	"io/ioutil"
 	"net/http"
 	"net/http/httptest"
 	"testing"
 
 	"github.com/gin-gonic/gin"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/mock"
+	"github.com/stretchr/testify/require"
 
 	mmetrics "github.com/slok/go-http-metrics/internal/mocks/metrics"
 	"github.com/slok/go-http-metrics/metrics"
 	"github.com/slok/go-http-metrics/middleware"
 	ginmiddleware "github.com/slok/go-http-metrics/middleware/gin"
 )
 
-func getTestHandler(statusCode int) gin.HandlerFunc {
-	return func(c *gin.Context) {
-		c.String(statusCode, "Hello world")
-	}
-}
-
 func TestMiddleware(t *testing.T) {
 	tests := map[string]struct {
-		handlerID     string
-		statusCode    int
-		req           *http.Request
-		config        middleware.Config
-		expHandlerID  string
-		expService    string
-		expMethod     string
-		expStatusCode string
+		handlerID   string
+		config      middleware.Config
+		req         func() *http.Request
+		mock        func(m *mmetrics.Recorder)
+		handler     func() gin.HandlerFunc
+		expRespCode int
+		expRespBody string
 	}{
 		"A default HTTP middleware should call the recorder to measure.": {
-			statusCode:    http.StatusAccepted,
-			req:           httptest.NewRequest(http.MethodPost, "/test", nil),
-			expHandlerID:  "/test",
-			expMethod:     http.MethodPost,
-			expStatusCode: "202",
+			req: func() *http.Request {
+				return httptest.NewRequest(http.MethodPost, "/test", nil)
+			},
+			mock: func(m *mmetrics.Recorder) {
+				expHTTPReqProps := metrics.HTTPReqProperties{
+					ID:      "/test",
+					Service: "",
+					Method:  "POST",
+					Code:    "202",
+				}
+				m.On("ObserveHTTPRequestDuration", mock.Anything, expHTTPReqProps, mock.Anything).Once()
+				m.On("ObserveHTTPResponseSize", mock.Anything, expHTTPReqProps, int64(5)).Once()
+
+				expHTTPProps := metrics.HTTPProperties{
+					ID:      "/test",
+					Service: "",
+				}
+				m.On("AddInflightRequests", mock.Anything, expHTTPProps, 1).Once()
+				m.On("AddInflightRequests", mock.Anything, expHTTPProps, -1).Once()
+			},
+			handler: func() gin.HandlerFunc {
+				return func(c *gin.Context) {
+					c.String(202, "test1")
+				}
+			},
+			expRespCode: 202,
+			expRespBody: "test1",
+		},
+
+		"A default HTTP middleware using JSON should call the recorder to measure (Regression test: https://github.com/slok/go-http-metrics/issues/31).": {
+			req: func() *http.Request {
+				return httptest.NewRequest(http.MethodPost, "/test", nil)
+			},
+			mock: func(m *mmetrics.Recorder) {
+				expHTTPReqProps := metrics.HTTPReqProperties{
+					ID:      "/test",
+					Service: "",
+					Method:  "POST",
+					Code:    "202",
+				}
+				m.On("ObserveHTTPRequestDuration", mock.Anything, expHTTPReqProps, mock.Anything).Once()
+				m.On("ObserveHTTPResponseSize", mock.Anything, expHTTPReqProps, int64(14)).Once()
+
+				expHTTPProps := metrics.HTTPProperties{
+					ID:      "/test",
+					Service: "",
+				}
+				m.On("AddInflightRequests", mock.Anything, expHTTPProps, 1).Once()
+				m.On("AddInflightRequests", mock.Anything, expHTTPProps, -1).Once()
+			},
+			handler: func() gin.HandlerFunc {
+				return func(c *gin.Context) {
+					c.JSON(202, map[string]string{"test": "one"})
+				}
+			},
+			expRespCode: 202,
+			expRespBody: `{"test":"one"}`,
 		},
 	}
 
 	for name, test := range tests {
 		t.Run(name, func(t *testing.T) {
 			assert := assert.New(t)
+			require := require.New(t)
 
 			// Mocks.
 			mr := &mmetrics.Recorder{}
-			expHTTPReqProps := metrics.HTTPReqProperties{
-				ID:      test.expHandlerID,
-				Service: test.expService,
-				Method:  test.expMethod,
-				Code:    test.expStatusCode,
-			}
-			expHTTPProps := metrics.HTTPProperties{
-				ID:      test.expHandlerID,
-				Service: test.expService,
-			}
-			mr.On("ObserveHTTPRequestDuration", mock.Anything, expHTTPReqProps, mock.Anything).Once()
-			mr.On("ObserveHTTPResponseSize", mock.Anything, expHTTPReqProps, mock.Anything).Once()
-			mr.On("AddInflightRequests", mock.Anything, expHTTPProps, 1).Once()
-			mr.On("AddInflightRequests", mock.Anything, expHTTPProps, -1).Once()
+			test.mock(mr)
 
 			// Create our instance with the middleware.
 			mdlw := middleware.New(middleware.Config{Recorder: mr})
 			engine := gin.New()
-			engine.POST("/test",
-				ginmiddleware.Handler("", mdlw),
-				getTestHandler(test.statusCode))
+			req := test.req()
+			engine.Handle(req.Method, req.URL.Path,
+				ginmiddleware.Handler(test.handlerID, mdlw),
+				test.handler())
 
 			// Make the request.
 			resp := httptest.NewRecorder()
-			engine.ServeHTTP(resp, test.req)
+			engine.ServeHTTP(resp, req)
 
 			// Check.
 			mr.AssertExpectations(t)
-			assert.Equal(test.statusCode, resp.Result().StatusCode)
+			assert.Equal(test.expRespCode, resp.Result().StatusCode)
+			gotBody, err := ioutil.ReadAll(resp.Result().Body)
+			require.NoError(err)
+			assert.Equal(test.expRespBody, string(gotBody))
 		})
 	}
 }

middleware/gorestful/gorestful.go

@@ -1,5 +1,4 @@
-// Package gorestful is a helper package to get a gorestful compatible
-// handler/middleware from the standard net/http Middleware factory.
+// Package gorestful is a helper package to get a gorestful compatible middleware.
 package gorestful
 
 import (