Skip to content

Commit

Permalink
Fix non-constant format strings in t.Fatalf (#972)
Browse files Browse the repository at this point in the history 
This is a new go vet analysis pass that fires under go 1.24:

  $ go1.24rc2 vet ./...
  # github.com/go-chi/chi/v5
  # [github.com/go-chi/chi/v5]
  ./mux_test.go:236:12: non-constant format string in call to (*testing.common).Fatalf
  ...

The strings would be parsed as format strings which is not intended.
See golang/go#60529 for details.
  • Loading branch information
@JRaspass
JRaspass authored Feb 5, 2025
1 parent 877e876 commit 72fbe46
Show file tree
Hide file tree
Showing 3 changed files with 68 additions and 68 deletions.
12 changes: 6 additions & 6 deletions middleware/get_head_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -38,27 +38,27 @@ func TestGetHead(t *testing.T) {
defer ts.Close()

if _, body := testRequest(t, ts, "GET", "/hi", nil); body != "bye" {
t.Fatalf(body)
t.Fatal(body)
}
if req, body := testRequest(t, ts, "HEAD", "/hi", nil); body != "" || req.Header.Get("X-Test") != "yes" {
t.Fatalf(body)
t.Fatal(body)
}
if _, body := testRequest(t, ts, "GET", "/", nil); body != "404 page not found\n" {
t.Fatalf(body)
t.Fatal(body)
}
if req, body := testRequest(t, ts, "HEAD", "/", nil); body != "" || req.StatusCode != 404 {
t.Fatalf(body)
t.Fatal(body)
}

if _, body := testRequest(t, ts, "GET", "/articles/5", nil); body != "article:5" {
t.Fatalf(body)
t.Fatal(body)
}
if req, body := testRequest(t, ts, "HEAD", "/articles/5", nil); body != "" || req.Header.Get("X-Article") != "5" {
t.Fatalf("expecting X-Article header '5' but got '%s'", req.Header.Get("X-Article"))
}

if _, body := testRequest(t, ts, "GET", "/users/1", nil); body != "user:1" {
t.Fatalf(body)
t.Fatal(body)
}
if req, body := testRequest(t, ts, "HEAD", "/users/1", nil); body != "" || req.Header.Get("X-User") != "-" {
t.Fatalf("expecting X-User header '-' but got '%s'", req.Header.Get("X-User"))
Expand Down
10 changes: 5 additions & 5 deletions middleware/url_format_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -36,16 +36,16 @@ func TestURLFormat(t *testing.T) {
defer ts.Close()

if _, resp := testRequest(t, ts, "GET", "/articles/1.json", nil); resp != "1" {
t.Fatalf(resp)
t.Fatal(resp)
}
if _, resp := testRequest(t, ts, "GET", "/articles/1.xml", nil); resp != "1" {
t.Fatalf(resp)
t.Fatal(resp)
}
if _, resp := testRequest(t, ts, "GET", "/samples/articles/samples.1.json", nil); resp != "1" {
t.Fatalf(resp)
t.Fatal(resp)
}
if _, resp := testRequest(t, ts, "GET", "/samples/articles/samples.1.xml", nil); resp != "1" {
t.Fatalf(resp)
t.Fatal(resp)
}
}

Expand All @@ -64,6 +64,6 @@ func TestURLFormatInSubRouter(t *testing.T) {
defer ts.Close()

if _, resp := testRequest(t, ts, "GET", "/articles/1/subroute.json", nil); resp != "1" {
t.Fatalf(resp)
t.Fatal(resp)
}
}
114 changes: 57 additions & 57 deletions mux_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -233,13 +233,13 @@ func TestMuxMounts(t *testing.T) {
defer ts.Close()

if _, body := testRequest(t, ts, "GET", "/sharing/aBc", nil); body != "/aBc" {
t.Fatalf(body)
t.Fatal(body)
}
if _, body := testRequest(t, ts, "GET", "/sharing/aBc/share", nil); body != "/aBc/share" {
t.Fatalf(body)
t.Fatal(body)
}
if _, body := testRequest(t, ts, "GET", "/sharing/aBc/share/twitter", nil); body != "/aBc/share/twitter" {
t.Fatalf(body)
t.Fatal(body)
}
}

Expand All @@ -257,10 +257,10 @@ func TestMuxPlain(t *testing.T) {
defer ts.Close()

if _, body := testRequest(t, ts, "GET", "/hi", nil); body != "bye" {
t.Fatalf(body)
t.Fatal(body)
}
if _, body := testRequest(t, ts, "GET", "/nothing-here", nil); body != "nothing here" {
t.Fatalf(body)
t.Fatal(body)
}
}

Expand All @@ -273,11 +273,11 @@ func TestMuxEmptyRoutes(t *testing.T) {
mux.Handle("/api*", apiRouter)

if _, body := testHandler(t, mux, "GET", "/", nil); body != "404 page not found\n" {
t.Fatalf(body)
t.Fatal(body)
}

if _, body := testHandler(t, apiRouter, "GET", "/", nil); body != "404 page not found\n" {
t.Fatalf(body)
t.Fatal(body)
}
}

Expand All @@ -304,13 +304,13 @@ func TestMuxTrailingSlash(t *testing.T) {
defer ts.Close()

if _, body := testRequest(t, ts, "GET", "/accounts/admin", nil); body != "admin" {
t.Fatalf(body)
t.Fatal(body)
}
if _, body := testRequest(t, ts, "GET", "/accounts/admin/", nil); body != "admin" {
t.Fatalf(body)
t.Fatal(body)
}
if _, body := testRequest(t, ts, "GET", "/nothing-here", nil); body != "nothing here" {
t.Fatalf(body)
t.Fatal(body)
}
}

Expand Down Expand Up @@ -371,24 +371,24 @@ func TestMuxNestedNotFound(t *testing.T) {
defer ts.Close()

if _, body := testRequest(t, ts, "GET", "/hi", nil); body != "bye" {
t.Fatalf(body)
t.Fatal(body)
}
if _, body := testRequest(t, ts, "GET", "/nothing-here", nil); body != "root 404 mw with" {
t.Fatalf(body)
t.Fatal(body)
}
if _, body := testRequest(t, ts, "GET", "/admin1/sub", nil); body != "sub" {
t.Fatalf(body)
t.Fatal(body)
}
if _, body := testRequest(t, ts, "GET", "/admin1/nope", nil); body != "sub 404 mw2" {
t.Fatalf(body)
t.Fatal(body)
}
if _, body := testRequest(t, ts, "GET", "/admin2/sub", nil); body != "sub2" {
t.Fatalf(body)
t.Fatal(body)
}

// Not found pages should bubble up to the root.
if _, body := testRequest(t, ts, "GET", "/admin2/nope", nil); body != "root 404 mw with" {
t.Fatalf(body)
t.Fatal(body)
}
}

Expand Down Expand Up @@ -470,28 +470,28 @@ func TestMuxNestedMethodNotAllowed(t *testing.T) {
defer ts.Close()

if _, body := testRequest(t, ts, "GET", "/root", nil); body != "root" {
t.Fatalf(body)
t.Fatal(body)
}
if _, body := testRequest(t, ts, "PUT", "/root", nil); body != "root 405" {
t.Fatalf(body)
t.Fatal(body)
}
if _, body := testRequest(t, ts, "GET", "/prefix1/sub1", nil); body != "sub1" {
t.Fatalf(body)
t.Fatal(body)
}
if _, body := testRequest(t, ts, "PUT", "/prefix1/sub1", nil); body != "sub1 405" {
t.Fatalf(body)
t.Fatal(body)
}
if _, body := testRequest(t, ts, "GET", "/prefix2/sub2", nil); body != "sub2" {
t.Fatalf(body)
t.Fatal(body)
}
if _, body := testRequest(t, ts, "PUT", "/prefix2/sub2", nil); body != "root 405" {
t.Fatalf(body)
t.Fatal(body)
}
if _, body := testRequest(t, ts, "GET", "/pathVar/myvar", nil); body != "pv" {
t.Fatalf(body)
t.Fatal(body)
}
if _, body := testRequest(t, ts, "DELETE", "/pathVar/myvar", nil); body != "pv 405" {
t.Fatalf(body)
t.Fatal(body)
}
}

Expand Down Expand Up @@ -532,39 +532,39 @@ func TestMuxComplicatedNotFound(t *testing.T) {

// check that we didn't break correct routes
if _, body := testRequest(t, ts, "GET", "/auth", nil); body != "auth get" {
t.Fatalf(body)
t.Fatal(body)
}
if _, body := testRequest(t, ts, "GET", "/public", nil); body != "public get" {
t.Fatalf(body)
t.Fatal(body)
}
if _, body := testRequest(t, ts, "GET", "/public/", nil); body != "public get" {
t.Fatalf(body)
t.Fatal(body)
}
if _, body := testRequest(t, ts, "GET", "/private/resource", nil); body != "private get" {
t.Fatalf(body)
t.Fatal(body)
}
// check custom not-found on all levels
if _, body := testRequest(t, ts, "GET", "/nope", nil); body != "custom not-found" {
t.Fatalf(body)
t.Fatal(body)
}
if _, body := testRequest(t, ts, "GET", "/public/nope", nil); body != "custom not-found" {
t.Fatalf(body)
t.Fatal(body)
}
if _, body := testRequest(t, ts, "GET", "/private/nope", nil); body != "custom not-found" {
t.Fatalf(body)
t.Fatal(body)
}
if _, body := testRequest(t, ts, "GET", "/private/resource/nope", nil); body != "custom not-found" {
t.Fatalf(body)
t.Fatal(body)
}
if _, body := testRequest(t, ts, "GET", "/private_mw/nope", nil); body != "custom not-found" {
t.Fatalf(body)
t.Fatal(body)
}
if _, body := testRequest(t, ts, "GET", "/private_mw/resource/nope", nil); body != "custom not-found" {
t.Fatalf(body)
t.Fatal(body)
}
// check custom not-found on trailing slash routes
if _, body := testRequest(t, ts, "GET", "/auth/", nil); body != "custom not-found" {
t.Fatalf(body)
t.Fatal(body)
}
}

Expand Down Expand Up @@ -621,10 +621,10 @@ func TestMuxWith(t *testing.T) {
defer ts.Close()

if _, body := testRequest(t, ts, "GET", "/hi", nil); body != "bye" {
t.Fatalf(body)
t.Fatal(body)
}
if _, body := testRequest(t, ts, "GET", "/inline", nil); body != "inline yes yes" {
t.Fatalf(body)
t.Fatal(body)
}
if cmwInit1 != 1 {
t.Fatalf("expecting cmwInit1 to be 1, got %d", cmwInit1)
Expand Down Expand Up @@ -1497,7 +1497,7 @@ func TestMountingSimilarPattern(t *testing.T) {
defer ts.Close()

if _, body := testRequest(t, ts, "GET", "/hi", nil); body != "bye" {
t.Fatalf(body)
t.Fatal(body)
}
}

Expand All @@ -1514,10 +1514,10 @@ func TestMuxEmptyParams(t *testing.T) {
defer ts.Close()

if _, body := testRequest(t, ts, "GET", "/users/a/b/c", nil); body != "a-b-c" {
t.Fatalf(body)
t.Fatal(body)
}
if _, body := testRequest(t, ts, "GET", "/users///c", nil); body != "--c" {
t.Fatalf(body)
t.Fatal(body)
}
}

Expand All @@ -1536,10 +1536,10 @@ func TestMuxMissingParams(t *testing.T) {
defer ts.Close()

if _, body := testRequest(t, ts, "GET", "/user/123", nil); body != "userId = '123'" {
t.Fatalf(body)
t.Fatal(body)
}
if _, body := testRequest(t, ts, "GET", "/user/", nil); body != "nothing here" {
t.Fatalf(body)
t.Fatal(body)
}
}

Expand Down Expand Up @@ -1581,7 +1581,7 @@ func TestMuxRegexp(t *testing.T) {
defer ts.Close()

if _, body := testRequest(t, ts, "GET", "//test", nil); body != "Hi: " {
t.Fatalf(body)
t.Fatal(body)
}
}

Expand All @@ -1594,10 +1594,10 @@ func TestMuxRegexp2(t *testing.T) {
defer ts.Close()

if _, body := testRequest(t, ts, "GET", "/foo-.json", nil); body != "" {
t.Fatalf(body)
t.Fatal(body)
}
if _, body := testRequest(t, ts, "GET", "/foo-abc.json", nil); body != "abc" {
t.Fatalf(body)
t.Fatal(body)
}
}

Expand Down Expand Up @@ -1629,16 +1629,16 @@ func TestMuxRegexp3(t *testing.T) {
defer ts.Close()

if _, body := testRequest(t, ts, "GET", "/one/hello/peter/first", nil); body != "first" {
t.Fatalf(body)
t.Fatal(body)
}
if _, body := testRequest(t, ts, "GET", "/one/hithere/123/second", nil); body != "second" {
t.Fatalf(body)
t.Fatal(body)
}
if _, body := testRequest(t, ts, "DELETE", "/one/hithere/123/second", nil); body != "third" {
t.Fatalf(body)
t.Fatal(body)
}
if _, body := testRequest(t, ts, "DELETE", "/one/123", nil); body != "forth" {
t.Fatalf(body)
t.Fatal(body)
}
}

Expand All @@ -1661,16 +1661,16 @@ func TestMuxSubrouterWildcardParam(t *testing.T) {
defer ts.Close()

if _, body := testRequest(t, ts, "GET", "/bare/hi", nil); body != "param:hi *:" {
t.Fatalf(body)
t.Fatal(body)
}
if _, body := testRequest(t, ts, "GET", "/bare/hi/yes", nil); body != "param:hi *:yes" {
t.Fatalf(body)
t.Fatal(body)
}
if _, body := testRequest(t, ts, "GET", "/case0/hi", nil); body != "param:hi *:" {
t.Fatalf(body)
t.Fatal(body)
}
if _, body := testRequest(t, ts, "GET", "/case0/hi/yes", nil); body != "param:hi *:yes" {
t.Fatalf(body)
t.Fatal(body)
}
}

Expand Down Expand Up @@ -1746,7 +1746,7 @@ func TestEscapedURLParams(t *testing.T) {
defer ts.Close()

if _, body := testRequest(t, ts, "GET", "/api/http:%2f%2fexample.com%2fimage.png/full/max/0/color.png", nil); body != "success" {
t.Fatalf(body)
t.Fatal(body)
}
}

Expand All @@ -1769,10 +1769,10 @@ func TestCustomHTTPMethod(t *testing.T) {
defer ts.Close()

if _, body := testRequest(t, ts, "GET", "/", nil); body != "." {
t.Fatalf(body)
t.Fatal(body)
}
if _, body := testRequest(t, ts, "BOO", "/hi", nil); body != "custom method" {
t.Fatalf(body)
t.Fatal(body)
}
}

Expand Down Expand Up @@ -1948,7 +1948,7 @@ func TestServerBaseContext(t *testing.T) {
defer ts.Close()

if _, body := testRequest(t, ts, "GET", "/", nil); body != "yes" {
t.Fatalf(body)
t.Fatal(body)
}
}

Expand Down

0 comments on commit 72fbe46

Please sign in to comment.