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.

Sign up for GitHub

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

agent/http: use httptest.Server for handler tests #308

Merged
merged 4 commits into from
Aug 21, 2023
Merged

agent/http: use httptest.Server for handler tests #308

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
a84515a
agent/http: use httptest.Server for handler tests
nadiamoe Aug 21, 2023
2f3342c
agent/http: always use `http.DefaultClient` in httpHandler
nadiamoe Aug 21, 2023
371bb82
agent/http: test: clarify naming of upstream body/headers, do not sen…
nadiamoe Aug 21, 2023
9b62c15
agent/http: test: replace Errorf+return with Fatalf
nadiamoe Aug 21, 2023
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
10 changes: 1 addition & 9 deletions pkg/agent/protocol/http/proxy.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -75,17 +75,10 @@ func NewProxy(c ProxyConfig, d Disruption) (protocol.Proxy, error) {
}, nil
}

// httpClient defines the method for executing HTTP requests. It is used to allow mocking
// the client in tests
type httpClient interface {
Do(req *http.Request) (*http.Response, error)
}

// httpHandler implements a http.Handler for disrupting request to a upstream server
type httpHandler struct {
upstreamURL url.URL
disruption Disruption
client httpClient
metrics *protocol.MetricMap
}

Expand All @@ -111,7 +104,7 @@ func (h *httpHandler) forward(rw http.ResponseWriter, req *http.Request, delay t
upstreamReq.URL.Scheme = h.upstreamURL.Scheme
upstreamReq.RequestURI = "" // It is an error to set this field in an HTTP client request.

response, err := h.client.Do(upstreamReq)
response, err := http.DefaultClient.Do(upstreamReq)
<-timer
if err != nil {
rw.WriteHeader(http.StatusBadGateway)
Expand Down Expand Up @@ -183,7 +176,6 @@ func (p *proxy) Start() error {
handler := &httpHandler{
upstreamURL: *upstreamURL,
disruption: p.disruption,
client: http.DefaultClient,
metrics: &p.metrics,
}

Expand Down
141 changes: 49 additions & 92 deletions pkg/agent/protocol/http/proxy_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,41 +2,16 @@ package http

import (
"bytes"
"fmt"
"io"
"net/http"
"net/http/httptest"
"net/url"
"strings"
"testing"

"github.com/google/go-cmp/cmp"
"github.com/grafana/xk6-disruptor/pkg/agent/protocol"
)

// fakeHTTPClient mocks the execution of a request returning the predefines
// status and body
type fakeHTTPClient struct {
status int
headers http.Header
body []byte
}

func (f *fakeHTTPClient) Do(_ *http.Request) (*http.Response, error) {
resp := &http.Response{
Proto: "HTTP/1.1",
ProtoMajor: 1,
ProtoMinor: 1,
StatusCode: f.status,
Status: http.StatusText(f.status),
Header: f.headers,
Body: io.NopCloser(strings.NewReader(string(f.body))),
ContentLength: int64(len(f.body)),
}

return resp, nil
}

func Test_Validations(t *testing.T) {
t.Parallel()

Expand Down Expand Up @@ -195,12 +170,11 @@ func Test_ProxyHandler(t *testing.T) {
type TestCase struct {
title string
disruption Disruption
config ProxyConfig
method string
path string
statusCode int
headers http.Header
body []byte
upstreamHeaders http.Header
upstreamBody []byte
expectedStatus int
expectedHeaders http.Header
expectedBody []byte
Expand All @@ -216,13 +190,9 @@ func Test_ProxyHandler(t *testing.T) {
ErrorCode: 0,
Excluded: nil,
},
config: ProxyConfig{
ListenAddress: ":9080",
UpstreamAddress: "http://127.0.0.1:8080",
},
path: "",
statusCode: 200,
body: []byte("content body"),
upstreamBody: []byte("content body"),
expectedStatus: 200,
expectedBody: []byte("content body"),
},
Expand All @@ -235,13 +205,9 @@ func Test_ProxyHandler(t *testing.T) {
ErrorCode: 500,
Excluded: nil,
},
config: ProxyConfig{
ListenAddress: ":9080",
UpstreamAddress: "http://127.0.0.1:8080",
},
path: "",
statusCode: 200,
body: []byte("content body"),
upstreamBody: []byte("content body"),
expectedStatus: 500,
expectedBody: []byte(""),
},
Expand All @@ -254,13 +220,9 @@ func Test_ProxyHandler(t *testing.T) {
ErrorCode: 500,
Excluded: []string{"/excluded/path"},
},
config: ProxyConfig{
ListenAddress: ":9080",
UpstreamAddress: "http://127.0.0.1:8080",
},
path: "/excluded/path",
statusCode: 200,
body: []byte("content body"),
upstreamBody: []byte("content body"),
expectedStatus: 200,
expectedBody: []byte("content body"),
},
Expand All @@ -273,13 +235,9 @@ func Test_ProxyHandler(t *testing.T) {
ErrorCode: 500,
Excluded: []string{"/excluded/path"},
},
config: ProxyConfig{
ListenAddress: ":9080",
UpstreamAddress: "http://127.0.0.1:8080",
},
path: "/non-excluded/path",
statusCode: 200,
body: []byte("content body"),
upstreamBody: []byte("content body"),
expectedStatus: 500,
expectedBody: []byte(""),
},
Expand All @@ -293,13 +251,9 @@ func Test_ProxyHandler(t *testing.T) {
ErrorBody: "{\"error\": 500, \"message\":\"internal server error\"}",
Excluded: nil,
},
config: ProxyConfig{
ListenAddress: ":9080",
UpstreamAddress: "http://127.0.0.1:8080",
},
path: "",
statusCode: 200,
body: []byte("content body"),
upstreamBody: []byte("content body"),
expectedStatus: 500,
expectedBody: []byte("{\"error\": 500, \"message\":\"internal server error\"}"),
},
Expand All @@ -308,16 +262,12 @@ func Test_ProxyHandler(t *testing.T) {
disruption: Disruption{
Excluded: []string{"/excluded"},
},
config: ProxyConfig{
ListenAddress: ":9080",
UpstreamAddress: "http://127.0.0.1:8080",
},
path: "/excluded",
statusCode: 200,
headers: http.Header{
upstreamHeaders: http.Header{
"X-Test-Header": []string{"A-Test"},
},
body: []byte("content body"),
upstreamBody: []byte("content body"),
expectedStatus: 200,
expectedHeaders: http.Header{
"X-Test-Header": []string{"A-Test"},
Expand All @@ -329,15 +279,11 @@ func Test_ProxyHandler(t *testing.T) {
disruption: Disruption{
ErrorRate: 0,
},
config: ProxyConfig{
ListenAddress: ":9080",
UpstreamAddress: "http://127.0.0.1:8080",
},
statusCode: 200,
headers: http.Header{
upstreamHeaders: http.Header{
"X-Test-Header": []string{"A-Test"},
},
body: []byte("content body"),
upstreamBody: []byte("content body"),
expectedStatus: 200,
expectedHeaders: http.Header{
"X-Test-Header": []string{"A-Test"},
Expand All @@ -350,15 +296,11 @@ func Test_ProxyHandler(t *testing.T) {
ErrorRate: 1.0,
ErrorCode: 500,
},
config: ProxyConfig{
ListenAddress: ":9080",
UpstreamAddress: "http://127.0.0.1:8080",
},
statusCode: 200,
headers: http.Header{
upstreamHeaders: http.Header{
"X-Test-Header": []string{"A-Test"},
},
body: []byte("content body"),
upstreamBody: []byte("content body"),
expectedStatus: 500,
expectedHeaders: http.Header{},
expectedBody: nil,
Expand All @@ -371,48 +313,64 @@ func Test_ProxyHandler(t *testing.T) {
t.Run(tc.title, func(t *testing.T) {
t.Parallel()

client := &fakeHTTPClient{
body: tc.body,
status: tc.expectedStatus,
headers: tc.headers,
}
upstreamServer := httptest.NewServer(http.HandlerFunc(func(rw http.ResponseWriter, r *http.Request) {
for k, values := range tc.upstreamHeaders {
for _, v := range values {
rw.Header().Add(k, v)
}
}
rw.WriteHeader(tc.statusCode)

upstreamURL, err := url.Parse(tc.config.UpstreamAddress)
_, err := rw.Write(tc.upstreamBody)
if err != nil {
t.Fatalf("writing upstream body: %v", err)
}
}))

upstreamURL, err := url.Parse(upstreamServer.URL)
if err != nil {
t.Errorf("error parsing upstream address %v", err)
return
t.Fatalf("error parsing httptest url")
}

handler := &httpHandler{
upstreamURL: *upstreamURL,
client: client,
disruption: tc.disruption,
metrics: &protocol.MetricMap{},
}

reqURL := fmt.Sprintf("http://%s%s", tc.config.ListenAddress, tc.path)
req := httptest.NewRequest(tc.method, reqURL, strings.NewReader(string(tc.body)))
recorder := httptest.NewRecorder()
handler.ServeHTTP(recorder, req)
resp := recorder.Result()
proxyServer := httptest.NewServer(handler)

req, err := http.NewRequest(tc.method, proxyServer.URL+tc.path, nil)
if err != nil {
t.Fatalf("building request to proxy: %v", err)
}

resp, err := http.DefaultClient.Do(req)
if err != nil {
t.Fatalf("making request to proxy: %v", err)
}

if tc.expectedStatus != resp.StatusCode {
t.Errorf("expected status code '%d' but '%d' received ", tc.expectedStatus, resp.StatusCode)
return
t.Fatalf("expected status code '%d' but '%d' received ", tc.expectedStatus, resp.StatusCode)
}

// Remove standard response headers so we don't need to specify them on every test case.
resp.Header.Del("content-length")
resp.Header.Del("content-type")
resp.Header.Del("date")

// Compare headers only if either expected or returned have items.
// We have to check for length explicitly as otherwise a nil map would not be equal to an empty map.
if len(tc.headers) > 0 || len(tc.expectedHeaders) > 0 {
if len(tc.upstreamHeaders) > 0 || len(tc.expectedHeaders) > 0 {
if diff := cmp.Diff(tc.expectedHeaders, resp.Header); diff != "" {
t.Errorf("Expected headers did not match returned:\n%s", diff)
t.Fatalf("Expected headers did not match returned:\n%s", diff)
}
}

var body bytes.Buffer
_, _ = io.Copy(&body, resp.Body)
if !bytes.Equal(tc.expectedBody, body.Bytes()) {
t.Errorf("expected body '%s' but '%s' received ", tc.expectedBody, body.Bytes())
return
t.Fatalf("expected body '%s' but '%s' received ", tc.expectedBody, body.Bytes())
}
})
}
Expand Down Expand Up @@ -466,7 +424,6 @@ func Test_Metrics(t *testing.T) {
handler := &httpHandler{
upstreamURL: *upstreamURL,
disruption: tc.config,
client: http.DefaultClient,
metrics: &metrics,
}

Expand Down