Skip to content

Commit ef0060e

Browse files
committed
go-require: require-error: readme & tests
1 parent ff5c42a commit ef0060e

File tree

5 files changed

+77
-6
lines changed

5 files changed

+77
-6
lines changed

README.md

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -368,7 +368,13 @@ Also a bad solution would be to simply replace all `require` in goroutines with
368368

369369
The checker is enabled by default, because `testinggoroutine` is enabled by default in `go vet`.
370370

371-
P.S. Related `testify`'s [thread](https://github.com/stretchr/testify/issues/772).
371+
In addition, the checker warns about `require` in HTTP handlers (functions and methods whose signature matches
372+
[http.HandlerFunc](https://pkg.go.dev/net/http#HandlerFunc)), because handlers run in a separate
373+
[service goroutine](https://cs.opensource.google/go/go/+/refs/tags/go1.22.3:src/net/http/server.go;l=2782;drc=1d45a7ef560a76318ed59dfdb178cecd58caf948) that
374+
services the HTTP connection. Terminating these goroutines can lead to undefined behaviour and difficulty debugging tests.
375+
You can turn off the check using the `--go-require.ignore-http-handlers` flag.
376+
377+
P.S. Look at [testify's issue](https://github.com/stretchr/testify/issues/772), related to assertions in the goroutines.
372378

373379
---
374380

analyzer/analyzer_test.go

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,16 @@ func TestTestifyLint(t *testing.T) {
4949
"expected-actual.pattern": "goldenValue",
5050
},
5151
},
52-
{dir: "ginkgo"},
52+
{
53+
dir: "ginkgo",
54+
},
55+
{
56+
dir: "go-require-http-handlers",
57+
flags: map[string]string{
58+
"enable": checkers.NewGoRequire().Name() + "," + // https://github.com/Antonboom/testifylint/issues/66
59+
checkers.NewRequireError().Name(), // https://github.com/Antonboom/testifylint/issues/73
60+
},
61+
},
5362
{
5463
dir: "go-require-ignore-http-handlers",
5564
flags: map[string]string{
@@ -58,10 +67,6 @@ func TestTestifyLint(t *testing.T) {
5867
"go-require.ignore-http-handlers": "true",
5968
},
6069
},
61-
{
62-
dir: "go-require-issue66-issue73",
63-
flags: map[string]string{"enable-all": "true"},
64-
},
6570
{
6671
dir: "go-require-issue67",
6772
flags: map[string]string{"disable-all": "true", "enable": checkers.NewGoRequire().Name()},

analyzer/testdata/src/go-require-issue66-issue73/go_require_http_handler_test.go renamed to analyzer/testdata/src/go-require-http-handlers/go_require_http_handlers_test.go

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -90,6 +90,25 @@ func TestServerSuite(t *testing.T) {
9090

9191
func (s *ServerSuite) TestServer() {
9292
httptest.NewServer(http.HandlerFunc(s.handler))
93+
httptest.NewServer(s)
94+
}
95+
96+
func (s *ServerSuite) ServeHTTP(w http.ResponseWriter, _ *http.Request) {
97+
s.T().Helper()
98+
99+
file, err := os.Open("some file.json")
100+
s.Require().NoError(err) // want "go-require: do not use require in http handlers"
101+
102+
data, err := io.ReadAll(file)
103+
if !s.NoError(err) {
104+
return
105+
}
106+
107+
w.Header().Set("Content-Type", "application/json")
108+
_, err = w.Write(data)
109+
if !s.NoError(err) {
110+
s.FailNow(err.Error()) // want "go-require: do not use s\\.FailNow in http handlers"
111+
}
93112
}
94113

95114
func (s *ServerSuite) handler(w http.ResponseWriter, _ *http.Request) {

analyzer/testdata/src/go-require-ignore-http-handlers/go_require_ignore_http_handlers_test.go

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
package gorequireignorehttphandlers_test
22

33
import (
4+
"encoding/json"
45
"io"
56
"net/http"
67
"net/http/httptest"
@@ -10,6 +11,7 @@ import (
1011

1112
"github.com/stretchr/testify/assert"
1213
"github.com/stretchr/testify/require"
14+
"github.com/stretchr/testify/suite"
1315
)
1416

1517
func TestServer_Require(t *testing.T) {
@@ -46,3 +48,34 @@ func TestServer_Require(t *testing.T) {
4648

4749
require.Equal(t, http.StatusOK, <-statusCode)
4850
}
51+
52+
type SomeServerSuite struct {
53+
suite.Suite
54+
}
55+
56+
func TestSomeServerSuite(t *testing.T) {
57+
suite.Run(t, &SomeServerSuite{})
58+
}
59+
60+
func (s *SomeServerSuite) TestServer() {
61+
httptest.NewServer(http.HandlerFunc(s.handler))
62+
httptest.NewServer(s)
63+
}
64+
65+
func (s *SomeServerSuite) ServeHTTP(hres http.ResponseWriter, hreq *http.Request) {
66+
var req MyRequest
67+
err := json.NewDecoder(hreq.Body).Decode(&req)
68+
s.Require().NoError(err)
69+
s.Equal("42", req.ID)
70+
}
71+
72+
func (s *SomeServerSuite) handler(hres http.ResponseWriter, hreq *http.Request) {
73+
var req MyRequest
74+
err := json.NewDecoder(hreq.Body).Decode(&req)
75+
s.Require().NoError(err)
76+
s.Equal("42", req.ID)
77+
}
78+
79+
type MyRequest struct {
80+
ID string
81+
}

analyzer/testdata/src/require-error-skip-logic/issue73_test.go

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -73,6 +73,14 @@ func TestSomeServerSuite(t *testing.T) {
7373

7474
func (s *SomeServerSuite) TestServer() {
7575
httptest.NewServer(http.HandlerFunc(s.handler))
76+
httptest.NewServer(s)
77+
}
78+
79+
func (s *SomeServerSuite) ServeHTTP(hres http.ResponseWriter, hreq *http.Request) {
80+
var req MyRequest
81+
err := json.NewDecoder(hreq.Body).Decode(&req)
82+
s.Require().NoError(err)
83+
s.Equal("42", req.ID)
7684
}
7785

7886
func (s *SomeServerSuite) handler(hres http.ResponseWriter, hreq *http.Request) {

0 commit comments

Comments
 (0)