Skip to content

Commit

Permalink
Introduced ErrServer, ErrClient and improved table tests.
Browse files Browse the repository at this point in the history 
Without t.Run() it is very hard to figure out which test in a table has
failed.

Signed-off-by: Marcin Owsiany <[email protected]>
  • Loading branch information
@porridge
porridge committed Oct 25, 2018
1 parent ed1974a commit 3899585
Show file tree
Hide file tree
Showing 2 changed files with 124 additions and 72 deletions.
17 changes: 15 additions & 2 deletions api/prometheus/v1/api.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,8 @@ const (
ErrCanceled = "canceled"
ErrExec = "execution"
ErrBadResponse = "bad_response"
ErrServer = "server_error"
ErrClient = "client_error"

// Possible values for HealthStatus.
HealthGood HealthStatus = "up"
Expand Down Expand Up @@ -461,6 +463,16 @@ func apiError(code int) bool {
return code == statusAPIError || code == http.StatusBadRequest
}

func errorTypeAndMsgFor(resp *http.Response) (ErrorType, string) {
switch resp.StatusCode / 100 {
case 4:
return ErrClient, fmt.Sprintf("client error: %d", resp.StatusCode)
case 5:
return ErrServer, fmt.Sprintf("server error: %d", resp.StatusCode)
}
return ErrBadResponse, fmt.Sprintf("bad response code %d", resp.StatusCode)
}

func (c apiClient) Do(ctx context.Context, req *http.Request) (*http.Response, []byte, error) {
resp, body, err := c.Client.Do(ctx, req)
if err != nil {
Expand All @@ -470,9 +482,10 @@ func (c apiClient) Do(ctx context.Context, req *http.Request) (*http.Response, [
code := resp.StatusCode

if code/100 != 2 && !apiError(code) {
errorType, errorMsg := errorTypeAndMsgFor(resp)
return resp, body, &Error{
Type: ErrBadResponse,
Msg: fmt.Sprintf("bad response code %d", resp.StatusCode),
Type: errorType,
Msg: errorMsg,
Detail: string(body),
}
}
Expand Down
179 changes: 109 additions & 70 deletions api/prometheus/v1/api_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -203,8 +203,8 @@ func TestAPIs(t *testing.T) {
inRes: "some body",
inStatusCode: 500,
inErr: &Error{
Type: ErrBadResponse,
Msg: "bad response code: 500",
Type: ErrServer,
Msg: "server error: 500",
Detail: "some body",
},

Expand All @@ -214,7 +214,25 @@ func TestAPIs(t *testing.T) {
"query": []string{"2"},
"time": []string{testTime.Format(time.RFC3339Nano)},
},
err: errors.New("bad_response: bad response code: 500"),
err: errors.New("server_error: server error: 500"),
},
{
do: doQuery("2", testTime),
inRes: "some body",
inStatusCode: 404,
inErr: &Error{
Type: ErrClient,
Msg: "client error: 404",
Detail: "some body",
},

reqMethod: "GET",
reqPath: "/api/v1/query",
reqParam: url.Values{
"query": []string{"2"},
"time": []string{testTime.Format(time.RFC3339Nano)},
},
err: errors.New("client_error: client error: 404"),
},

{
Expand Down Expand Up @@ -520,34 +538,36 @@ func TestAPIs(t *testing.T) {
var tests []apiTest
tests = append(tests, queryTests...)

for _, test := range tests {
client.curTest = test
for i, test := range tests {
t.Run(fmt.Sprintf("%d", i), func(t *testing.T) {
client.curTest = test

res, err := test.do()
res, err := test.do()

if test.err != nil {
if err == nil {
t.Errorf("expected error %q but got none", test.err)
continue
}
if err.Error() != test.err.Error() {
t.Errorf("unexpected error: want %s, got %s", test.err, err)
}
if apiErr, ok := err.(*Error); ok {
if apiErr.Detail != test.inRes {
t.Errorf("%q should be %q", apiErr.Detail, test.inRes)
if test.err != nil {
if err == nil {
t.Errorf("expected error %q but got none", test.err)
return
}
if err.Error() != test.err.Error() {
t.Errorf("unexpected error: want %s, got %s", test.err, err)
}
if apiErr, ok := err.(*Error); ok {
if apiErr.Detail != test.inRes {
t.Errorf("%q should be %q", apiErr.Detail, test.inRes)
}
}
return
}
if err != nil {
t.Errorf("unexpected error: %s", err)
return
}
continue
}
if err != nil {
t.Errorf("unexpected error: %s", err)
continue
}

if !reflect.DeepEqual(res, test.res) {
t.Errorf("unexpected result: want %v, got %v", test.res, res)
}
if !reflect.DeepEqual(res, test.res) {
t.Errorf("unexpected result: want %v, got %v", test.res, res)
}
})
}
}

Expand All @@ -559,10 +579,10 @@ type testClient struct {
}

type apiClientTest struct {
code int
response interface{}
expected string
err *Error
code int
response interface{}
expectedBody string
expectedErr *Error
}

func (c *testClient) URL(ep string, args map[string]string) *url.URL {
Expand Down Expand Up @@ -602,98 +622,108 @@ func (c *testClient) Do(ctx context.Context, req *http.Request) (*http.Response,
func TestAPIClientDo(t *testing.T) {
tests := []apiClientTest{
{
code: statusAPIError,
response: &apiResponse{
Status: "error",
Data: json.RawMessage(`null`),
ErrorType: ErrBadData,
Error: "failed",
},
err: &Error{
expectedErr: &Error{
Type: ErrBadData,
Msg: "failed",
},
code: statusAPIError,
expected: `null`,
expectedBody: `null`,
},
{
code: statusAPIError,
response: &apiResponse{
Status: "error",
Data: json.RawMessage(`"test"`),
ErrorType: ErrTimeout,
Error: "timed out",
},
err: &Error{
expectedErr: &Error{
Type: ErrTimeout,
Msg: "timed out",
},
code: statusAPIError,
expected: `test`,
expectedBody: `test`,
},
{
response: "bad json",
err: &Error{
Type: ErrBadResponse,
Msg: "bad response code 500",
code: http.StatusInternalServerError,
response: "500 error details",
expectedErr: &Error{
Type: ErrServer,
Msg: "server error: 500",
Detail: "500 error details",
},
},
{
code: http.StatusNotFound,
response: "404 error details",
expectedErr: &Error{
Type: ErrClient,
Msg: "client error: 404",
Detail: "404 error details",
},
code: http.StatusInternalServerError,
},
{
code: http.StatusBadRequest,
response: &apiResponse{
Status: "error",
Data: json.RawMessage(`null`),
ErrorType: ErrBadData,
Error: "end timestamp must not be before start time",
},
err: &Error{
expectedErr: &Error{
Type: ErrBadData,
Msg: "end timestamp must not be before start time",
},
code: http.StatusBadRequest,
},
{
code: statusAPIError,
response: "bad json",
err: &Error{
expectedErr: &Error{
Type: ErrBadResponse,
Msg: "invalid character 'b' looking for beginning of value",
},
code: statusAPIError,
},
{
code: statusAPIError,
response: &apiResponse{
Status: "success",
Data: json.RawMessage(`"test"`),
},
err: &Error{
expectedErr: &Error{
Type: ErrBadResponse,
Msg: "inconsistent body for response code",
},
code: statusAPIError,
},
{
code: statusAPIError,
response: &apiResponse{
Status: "success",
Data: json.RawMessage(`"test"`),
ErrorType: ErrTimeout,
Error: "timed out",
},
err: &Error{
expectedErr: &Error{
Type: ErrBadResponse,
Msg: "inconsistent body for response code",
},
code: statusAPIError,
},
{
code: http.StatusOK,
response: &apiResponse{
Status: "error",
Data: json.RawMessage(`"test"`),
ErrorType: ErrTimeout,
Error: "timed out",
},
err: &Error{
expectedErr: &Error{
Type: ErrBadResponse,
Msg: "inconsistent body for response code",
},
code: http.StatusOK,
},
}

Expand All @@ -704,30 +734,39 @@ func TestAPIClientDo(t *testing.T) {
}
client := &apiClient{tc}

for _, test := range tests {
for i, test := range tests {
t.Run(fmt.Sprintf("%d", i), func(t *testing.T) {

tc.ch <- test
tc.ch <- test

_, body, err := client.Do(context.Background(), tc.req)
_, body, err := client.Do(context.Background(), tc.req)

if test.err != nil {
if err == nil {
t.Errorf("expected error %q but got none", test.err)
continue
if test.expectedErr != nil {
if err == nil {
t.Errorf("expected error %q but got none", test.expectedErr)
return
}
if test.expectedErr.Error() != err.Error() {
t.Errorf("unexpected error: want %q, got %q", test.expectedErr, err)
}
if test.expectedErr.Detail != "" {
apiERr := err.(*Error)
if apiERr.Detail != test.expectedErr.Detail {
t.Errorf("unexpected error details: want %q, got %q", test.expectedErr.Detail, apiERr.Detail)
}
}
return
}
if test.err.Error() != err.Error() {
t.Errorf("unexpected error: want %q, got %q", test.err, err)
if err != nil {
t.Errorf("unexpeceted error %s", err)
return
}
continue
}
if err != nil {
t.Errorf("unexpeceted error %s", err)
continue
}

want, got := test.expected, string(body)
if want != got {
t.Errorf("unexpected body: want %q, got %q", want, got)
}
want, got := test.expectedBody, string(body)
if want != got {
t.Errorf("unexpected body: want %q, got %q", want, got)
}
})

}
}

0 comments on commit 3899585

Please sign in to comment.