From 69d6ff10dd0bf7c6a6ae8811f345809eba03b5c7 Mon Sep 17 00:00:00 2001 From: Adam Duke Date: Wed, 11 Dec 2024 15:27:47 -0500 Subject: [PATCH] set case names and use t.Run(name, func(){...}) fixes https://github.com/bmf-san/goblin/issues/109 --- context_test.go | 13 +++- router_test.go | 143 +++++++++++++++++----------------- trie_test.go | 199 ++++++++++++++++++++++++++++++------------------ 3 files changed, 205 insertions(+), 150 deletions(-) diff --git a/context_test.go b/context_test.go index df7ecc9..27b0418 100644 --- a/context_test.go +++ b/context_test.go @@ -21,30 +21,37 @@ func TestGetParam(t *testing.T) { ngCtx := context.WithValue(context.Background(), ParamsKey, "not a []Param") cases := []struct { + name string actual string expected string }{ { + name: "id_param", actual: GetParam(ctx, "id"), expected: "123", }, { + name: "name_param", actual: GetParam(ctx, "name"), expected: "john", }, { + name: "not_exist_param", actual: GetParam(ctx, "not-exist-key"), expected: "", }, { + name: "param_value_wrong_type", actual: GetParam(ngCtx, "ng ctx"), expected: "", }, } for _, c := range cases { - if c.actual != c.expected { - t.Errorf("actual:%v expected:%v", c.actual, c.expected) - } + t.Run(c.name, func(t *testing.T) { + if c.actual != c.expected { + t.Errorf("actual:%v expected:%v", c.actual, c.expected) + } + }) } } diff --git a/router_test.go b/router_test.go index cf8ca22..740a248 100644 --- a/router_test.go +++ b/router_test.go @@ -20,6 +20,17 @@ func TestNewRouter(t *testing.T) { } } +type routerTest struct { + path string + method string + code int + body string +} + +func (tc routerTest) name() string { + return fmt.Sprintf("%s_%s_%d", tc.method, tc.path, tc.code) +} + func TestRouterMiddleware(t *testing.T) { r := NewRouter() @@ -34,12 +45,7 @@ func TestRouterMiddleware(t *testing.T) { fmt.Fprintf(w, "/middlewares\n") })) - cases := []struct { - path string - method string - code int - body string - }{ + cases := []routerTest{ { path: "/globalmiddleware", method: http.MethodGet, @@ -61,22 +67,25 @@ func TestRouterMiddleware(t *testing.T) { } for _, c := range cases { - req := httptest.NewRequest(c.method, c.path, nil) - rec := httptest.NewRecorder() - - r.ServeHTTP(rec, req) - - if rec.Code != c.code { - t.Errorf("actual: %v expected: %v\n", rec.Code, c.code) - } - - recBody, _ := io.ReadAll(rec.Body) - body := string(recBody) - if body != c.body { - t.Errorf("actual: %v expected: %v\n", body, c.body) - } + t.Run(c.name(), func(t *testing.T) { + req := httptest.NewRequest(c.method, c.path, nil) + rec := httptest.NewRecorder() + + r.ServeHTTP(rec, req) + + if rec.Code != c.code { + t.Errorf("actual: %v expected: %v\n", rec.Code, c.code) + } + + recBody, _ := io.ReadAll(rec.Body) + body := string(recBody) + if body != c.body { + t.Errorf("actual: %v expected: %v\n", body, c.body) + } + }) } } + func TestRouter(t *testing.T) { r := NewRouter() @@ -142,12 +151,7 @@ func TestRouter(t *testing.T) { fmt.Fprintf(w, "/%v", id) })) - cases := []struct { - path string - method string - code int - body string - }{ + cases := []routerTest{ { path: "/", method: http.MethodGet, @@ -247,20 +251,22 @@ func TestRouter(t *testing.T) { } for _, c := range cases { - req := httptest.NewRequest(c.method, c.path, nil) - rec := httptest.NewRecorder() - - r.ServeHTTP(rec, req) - - if rec.Code != c.code { - t.Errorf("actual: %v expected: %v\n", rec.Code, c.code) - } - - recBody, _ := io.ReadAll(rec.Body) - body := string(recBody) - if body != c.body { - t.Errorf("actual: %v expected: %v\n", body, c.body) - } + t.Run(c.name(), func(t *testing.T) { + req := httptest.NewRequest(c.method, c.path, nil) + rec := httptest.NewRecorder() + + r.ServeHTTP(rec, req) + + if rec.Code != c.code { + t.Errorf("actual: %v expected: %v\n", rec.Code, c.code) + } + + recBody, _ := io.ReadAll(rec.Body) + body := string(recBody) + if body != c.body { + t.Errorf("actual: %v expected: %v\n", body, c.body) + } + }) } } @@ -269,11 +275,7 @@ func TestDefaultErrorHandler(t *testing.T) { r.Methods(http.MethodGet).Handler(`/defaulterrorhandler`, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {})) r.Methods(http.MethodGet).Handler(`/methodnotallowed`, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {})) - cases := []struct { - path string - method string - code int - }{ + cases := []routerTest{ { path: "/", method: http.MethodGet, @@ -287,14 +289,16 @@ func TestDefaultErrorHandler(t *testing.T) { } for _, c := range cases { - req := httptest.NewRequest(c.method, c.path, nil) - rec := httptest.NewRecorder() + t.Run(c.name(), func(t *testing.T) { + req := httptest.NewRequest(c.method, c.path, nil) + rec := httptest.NewRecorder() - r.ServeHTTP(rec, req) + r.ServeHTTP(rec, req) - if rec.Code != c.code { - t.Errorf("actual: %v expected: %v\n", rec.Code, c.code) - } + if rec.Code != c.code { + t.Errorf("actual: %v expected: %v\n", rec.Code, c.code) + } + }) } } @@ -311,12 +315,7 @@ func TestCustomErrorHandler(t *testing.T) { r.Methods(http.MethodGet).Handler(`/custommethodnotfound`, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {})) r.Methods(http.MethodGet).Handler(`/custommethodnotallowed`, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {})) - cases := []struct { - path string - method string - code int - body string - }{ + cases := []routerTest{ { path: "/", method: http.MethodGet, @@ -332,20 +331,22 @@ func TestCustomErrorHandler(t *testing.T) { } for _, c := range cases { - req := httptest.NewRequest(c.method, c.path, nil) - rec := httptest.NewRecorder() - - r.ServeHTTP(rec, req) - - if rec.Code != c.code { - t.Errorf("actual: %v expected: %v\n", rec.Code, c.code) - } - - recBody, _ := io.ReadAll(rec.Body) - body := string(recBody) - if body != c.body { - t.Errorf("actual: %v expected: %v\n", body, c.body) - } + t.Run(c.name(), func(t *testing.T) { + req := httptest.NewRequest(c.method, c.path, nil) + rec := httptest.NewRecorder() + + r.ServeHTTP(rec, req) + + if rec.Code != c.code { + t.Errorf("actual: %v expected: %v\n", rec.Code, c.code) + } + + recBody, _ := io.ReadAll(rec.Body) + body := string(recBody) + if body != c.body { + t.Errorf("actual: %v expected: %v\n", body, c.body) + } + }) } } diff --git a/trie_test.go b/trie_test.go index 61c0b9c..e5216a5 100644 --- a/trie_test.go +++ b/trie_test.go @@ -63,6 +63,14 @@ type caseWithFailure struct { expectedParams Params } +func (c caseWithFailure) name() string { + prefix := "" + if !c.hasError { + prefix = "no " + } + return fmt.Sprintf("%serror with %s path", prefix, c.item.path) +} + // insertItem is a struct for insert method. type insertItem struct { path string @@ -75,16 +83,19 @@ type searchItem struct { path string } +type searchFailureTest struct { + name string + insertItems []insertItem + searchItem *searchItem + expected error +} + func TestSearchFailure(t *testing.T) { fooHandler := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {}) - cases := []struct { - insertItems []insertItem - searchItem *searchItem - expected error - }{ + cases := []searchFailureTest{ { - // no matching path was found. + name: "no matching path was found", insertItems: []insertItem{ { path: `/foo`, @@ -98,7 +109,7 @@ func TestSearchFailure(t *testing.T) { expected: ErrNotFound, }, { - // no matching Param was found. + name: "no matching Param was found with path regex", insertItems: []insertItem{ { path: `/foo/:id[^\d+$]`, @@ -112,7 +123,7 @@ func TestSearchFailure(t *testing.T) { expected: ErrNotFound, }, { - // no matching Param was found. + name: "no matching Param was found", insertItems: []insertItem{ { path: `/foo`, @@ -126,7 +137,7 @@ func TestSearchFailure(t *testing.T) { expected: ErrNotFound, }, { - // no matching handler and middlewares was found. + name: "matching handler and middlewares was found 1", insertItems: []insertItem{ { path: `/foo`, @@ -140,7 +151,7 @@ func TestSearchFailure(t *testing.T) { expected: ErrNotFound, }, { - // no matching handler and middlewares was found. + name: "matching handler and middlewares was found 2", insertItems: []insertItem{ { path: `/foo`, @@ -156,18 +167,20 @@ func TestSearchFailure(t *testing.T) { } for _, c := range cases { - tree := newTree() - for _, i := range c.insertItems { - tree.Insert(i.path, i.handler, i.middlewares) - } - actualAction, actualParams, err := tree.Search(c.searchItem.path) - if actualAction != nil || actualParams != nil { - t.Fatalf("actualAction: %v actualParams: %v expected err: %v", actualAction, actualParams, err) - } - - if err != c.expected { - t.Fatalf("err: %v expected: %v\n", err, c.expected) - } + t.Run(c.name, func(t *testing.T) { + tree := newTree() + for _, i := range c.insertItems { + tree.Insert(i.path, i.handler, i.middlewares) + } + actualAction, actualParams, err := tree.Search(c.searchItem.path) + if actualAction != nil || actualParams != nil { + t.Fatalf("actualAction: %v actualParams: %v expected err: %v", actualAction, actualParams, err) + } + + if err != c.expected { + t.Fatalf("err: %v expected: %v\n", err, c.expected) + } + }) } } @@ -967,58 +980,59 @@ func TestSearchWildCardRegexp(t *testing.T) { } func testWithFailure(t *testing.T, tree *tree, cases []caseWithFailure) { + t.Helper() for _, c := range cases { - actualAction, actualParams, err := tree.Search(c.item.path) - - if c.hasError { - if err == nil { - t.Fatalf("actualAction: %v actualParams: %v expected err: %v", actualAction, actualParams, err) - } + t.Run(c.name(), func(t *testing.T) { + actualAction, actualParams, err := tree.Search(c.item.path) - if actualAction != c.expectedAction { - t.Errorf("actualAction:%v expectedAction:%v", actualAction, c.expectedAction) - } - - if len(actualParams) != len(c.expectedParams) { - t.Errorf("actualParams: %v expectedParams: %v\n", len(actualParams), len(c.expectedParams)) - } + if c.hasError { + if err == nil { + t.Fatalf("actualAction: %v actualParams: %v expected err: %v", actualAction, actualParams, err) + } - for i, Param := range actualParams { - if !reflect.DeepEqual(Param, c.expectedParams[i]) { - t.Errorf("actualParams: %v expectedParams: %v\n", Param, c.expectedParams[i]) + if actualAction != c.expectedAction { + t.Errorf("actualAction:%v expectedAction:%v", actualAction, c.expectedAction) } - } - continue - } + if len(actualParams) != len(c.expectedParams) { + t.Errorf("actualParams: %v expectedParams: %v\n", len(actualParams), len(c.expectedParams)) + } - if err != nil { - t.Fatalf("actualAction: %v actualParams: %v expected err: %v", actualAction, actualParams, err) - } + for i, Param := range actualParams { + if !reflect.DeepEqual(Param, c.expectedParams[i]) { + t.Errorf("actualParams: %v expectedParams: %v\n", Param, c.expectedParams[i]) + } + } + } else { + if err != nil { + t.Fatalf("actualAction: %v actualParams: %v expected err: %v", actualAction, actualParams, err) + } - if reflect.ValueOf(actualAction.handler) != reflect.ValueOf(c.expectedAction.handler) { - t.Errorf("actualActionHandler:%v expectedActionHandler:%v", actualAction.handler, c.expectedAction.handler) - } + if reflect.ValueOf(actualAction.handler) != reflect.ValueOf(c.expectedAction.handler) { + t.Errorf("actualActionHandler:%v expectedActionHandler:%v", actualAction.handler, c.expectedAction.handler) + } - if len(actualAction.middlewares) != len(c.expectedAction.middlewares) { - t.Errorf("actualActionMiddlewares: %v expectedActionsMiddleware: %v\n", len(actualAction.middlewares), len(c.expectedAction.middlewares)) - } + if len(actualAction.middlewares) != len(c.expectedAction.middlewares) { + t.Errorf("actualActionMiddlewares: %v expectedActionsMiddleware: %v\n", len(actualAction.middlewares), len(c.expectedAction.middlewares)) + } - for i, mws := range actualAction.middlewares { - if reflect.ValueOf(mws) != reflect.ValueOf(c.expectedAction.middlewares[i]) { - t.Errorf("actualActionsMiddleware: %v expectedActionsMiddleware: %v\n", mws, c.expectedAction.middlewares[i]) - } - } + for i, mws := range actualAction.middlewares { + if reflect.ValueOf(mws) != reflect.ValueOf(c.expectedAction.middlewares[i]) { + t.Errorf("actualActionsMiddleware: %v expectedActionsMiddleware: %v\n", mws, c.expectedAction.middlewares[i]) + } + } - if len(actualParams) != len(c.expectedParams) { - t.Errorf("actualParams: %v expectedParams: %v\n", len(actualParams), len(c.expectedParams)) - } + if len(actualParams) != len(c.expectedParams) { + t.Errorf("actualParams: %v expectedParams: %v\n", len(actualParams), len(c.expectedParams)) + } - for i, Param := range actualParams { - if !reflect.DeepEqual(Param, c.expectedParams[i]) { - t.Errorf("actualParam: %v expectedParam: %v\n", Param, c.expectedParams[i]) + for i, Param := range actualParams { + if !reflect.DeepEqual(Param, c.expectedParams[i]) { + t.Errorf("actualParam: %v expectedParam: %v\n", Param, c.expectedParams[i]) + } + } } - } + }) } } @@ -1077,142 +1091,175 @@ func TestGetReg(t *testing.T) { func TestGetPattern(t *testing.T) { cases := []struct { + name string actual string expected string }{ { + name: "valid pattern", actual: getPattern(`:id[^\d+$]`), expected: `^\d+$`, }, { + name: "invalid pattern one", actual: getPattern(`:id[`), expected: "", }, { + name: "invalid pattern two", actual: getPattern(`:id]`), expected: "", }, { + name: "missing pattern", actual: getPattern(`:id`), expected: "", }, } for _, c := range cases { - if c.actual != c.expected { - t.Errorf("actual:%v expected:%v", c.actual, c.expected) - } + t.Run(c.name, func(t *testing.T) { + if c.actual != c.expected { + t.Errorf("actual:%v expected:%v", c.actual, c.expected) + } + }) } } func TestGetParamName(t *testing.T) { cases := []struct { + name string actual string expected string }{ { + name: "valid pattern", actual: getParamName(`:id[^\d+$]`), expected: "id", }, { + name: "invalid pattern one", actual: getParamName(`:id[`), expected: "id", }, { + name: "invalid pattern two", actual: getParamName(`:id]`), expected: "id]", }, { + name: "missing pattern", actual: getParamName(`:id`), expected: "id", }, } for _, c := range cases { - if c.actual != c.expected { - t.Errorf("actual:%v expected:%v", c.actual, c.expected) - } + t.Run(c.name, func(t *testing.T) { + if c.actual != c.expected { + t.Errorf("actual:%v expected:%v", c.actual, c.expected) + } + }) } } func TestCleanPath(t *testing.T) { cases := []struct { + name string path string expected string }{ { + name: "missing root", path: "", expected: "/", }, { + name: "one extra trailing slash", path: "//", expected: "/", }, { + name: "two extra trailing slashes", path: "///", expected: "/", }, { + name: "missing leading slash", path: "path", expected: "/path", }, { + name: "valid root", path: "/", expected: "/", }, { + name: "valid path with trailing slash", path: "/path/trailingslash/", expected: "/path/trailingslash/", }, { + name: "valid path with extra trailing slash", path: "path/trailingslash//", expected: "/path/trailingslash/", }, } for _, c := range cases { - actual := cleanPath(c.path) - if actual != c.expected { - t.Errorf("actual: %v expected: %v\n", actual, c.expected) - } + t.Run(c.name, func(t *testing.T) { + actual := cleanPath(c.path) + if actual != c.expected { + t.Errorf("actual: %v expected: %v\n", actual, c.expected) + } + }) } } func TestRemoveTrailingSlash(t *testing.T) { cases := []struct { + name string path string expected string }{ { + name: "one extra trailing slash", path: "//", expected: "/", }, { + name: "two extra trailing slashes", path: "///", expected: "//", }, { + name: "plain path", path: "path", expected: "path", }, { + name: "root", path: "/", expected: "", }, { + name: "path with trailing slash", path: "/path/trailingslash/", expected: "/path/trailingslash", }, { + name: "path with extra trailing slash", path: "/path/trailingslash//", expected: "/path/trailingslash/", }, } for _, c := range cases { - actual := removeTrailingSlash(c.path) - if actual != c.expected { - t.Errorf("actual: %v expected: %v\n", actual, c.expected) - } + t.Run(c.name, func(t *testing.T) { + actual := removeTrailingSlash(c.path) + if actual != c.expected { + t.Errorf("actual: %v expected: %v\n", actual, c.expected) + } + }) } }