Skip to content

Commit

Permalink
🩹 Fix: Fix app.Test() auto-failing when a connection is closed early (#…
Browse files Browse the repository at this point in the history
…3279)

* ♻️ Refactor: Extract testConn err to variable

* ♻️ Refactor: Extract ErrTestGotEmptyResponse from app.Test()

* 🩹 Fix: Fix `app.Test()` auto-failing when testConn is closed

* 🩹 Fix(app_test.go): Use tab for indent instead of spaces

* 🩹 Fix(app_test.go): Fix to respect gofmt linter

* ♻️ Refactor: Update Drop tests to verify error type
  • Loading branch information
grivera64 authored Jan 13, 2025
1 parent bc37f20 commit 4e5fea1
Show file tree
Hide file tree
Showing 5 changed files with 27 additions and 8 deletions.
6 changes: 4 additions & 2 deletions app.go
Original file line number Diff line number Diff line change
Expand Up @@ -941,6 +941,8 @@ func (app *App) Hooks() *Hooks {
return app.hooks
}

var ErrTestGotEmptyResponse = errors.New("test: got empty response")

// TestConfig is a struct holding Test settings
type TestConfig struct {
// Timeout defines the maximum duration a
Expand Down Expand Up @@ -1022,7 +1024,7 @@ func (app *App) Test(req *http.Request, config ...TestConfig) (*http.Response, e
}

// Check for errors
if err != nil && !errors.Is(err, fasthttp.ErrGetOnly) {
if err != nil && !errors.Is(err, fasthttp.ErrGetOnly) && !errors.Is(err, errTestConnClosed) {
return nil, err
}

Expand All @@ -1033,7 +1035,7 @@ func (app *App) Test(req *http.Request, config ...TestConfig) (*http.Response, e
res, err := http.ReadResponse(buffer, req)
if err != nil {
if errors.Is(err, io.ErrUnexpectedEOF) {
return nil, errors.New("test: got empty response")
return nil, ErrTestGotEmptyResponse
}
return nil, fmt.Errorf("failed to read response: %w", err)
}
Expand Down
19 changes: 17 additions & 2 deletions app_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1491,7 +1491,7 @@ func Test_App_Test_timeout(t *testing.T) {
Timeout: 100 * time.Millisecond,
FailOnTimeout: true,
})
require.Equal(t, os.ErrDeadlineExceeded, err)
require.ErrorIs(t, err, os.ErrDeadlineExceeded)
}

func Test_App_Test_timeout_empty_response(t *testing.T) {
Expand All @@ -1507,7 +1507,22 @@ func Test_App_Test_timeout_empty_response(t *testing.T) {
Timeout: 100 * time.Millisecond,
FailOnTimeout: false,
})
require.Equal(t, errors.New("test: got empty response"), err)
require.ErrorIs(t, err, ErrTestGotEmptyResponse)
}

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

app := New()
app.Get("/", func(c Ctx) error {
return c.Drop()
})

_, err := app.Test(httptest.NewRequest(MethodGet, "/", nil), TestConfig{
Timeout: 0,
FailOnTimeout: false,
})
require.ErrorIs(t, err, ErrTestGotEmptyResponse)
}

func Test_App_SetTLSHandler(t *testing.T) {
Expand Down
4 changes: 2 additions & 2 deletions ctx_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -5896,7 +5896,7 @@ func Test_Ctx_Drop(t *testing.T) {

// Test the Drop method
resp, err := app.Test(httptest.NewRequest(MethodGet, "/block-me", nil))
require.Error(t, err)
require.ErrorIs(t, err, ErrTestGotEmptyResponse)
require.Nil(t, resp)

// Test the no-response handler
Expand Down Expand Up @@ -5927,7 +5927,7 @@ func Test_Ctx_DropWithMiddleware(t *testing.T) {

// Test the Drop method
resp, err := app.Test(httptest.NewRequest(MethodGet, "/block-me", nil))
require.Error(t, err)
require.ErrorIs(t, err, ErrTestGotEmptyResponse)
require.Nil(t, resp)
}

Expand Down
4 changes: 3 additions & 1 deletion helpers.go
Original file line number Diff line number Diff line change
Expand Up @@ -612,6 +612,8 @@ func isNoCache(cacheControl string) bool {
return true
}

var errTestConnClosed = errors.New("testConn is closed")

type testConn struct {
r bytes.Buffer
w bytes.Buffer
Expand All @@ -631,7 +633,7 @@ func (c *testConn) Write(b []byte) (int, error) {
defer c.Unlock()

if c.isClosed {
return 0, errors.New("testConn is closed")
return 0, errTestConnClosed
}
return c.w.Write(b) //nolint:wrapcheck // This must not be wrapped
}
Expand Down
2 changes: 1 addition & 1 deletion helpers_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -548,7 +548,7 @@ func Test_Utils_TestConn_Closed_Write(t *testing.T) {
// Close early, write should fail
conn.Close() //nolint:errcheck, revive // It is fine to ignore the error here
_, err = conn.Write([]byte("Response 2\n"))
require.Error(t, err)
require.ErrorIs(t, err, errTestConnClosed)

res := make([]byte, 11)
_, err = conn.w.Read(res)
Expand Down

1 comment on commit 4e5fea1

@github-actions
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Performance Alert ⚠️

Possible performance regression was detected for benchmark.
Benchmark result of this commit is worse than the previous benchmark result exceeding threshold 1.50.

Benchmark suite Current: 4e5fea1 Previous: e04f815 Ratio
Benchmark_Ctx_Send 6.535 ns/op 0 B/op 0 allocs/op 4.343 ns/op 0 B/op 0 allocs/op 1.50
Benchmark_Ctx_Send - ns/op 6.535 ns/op 4.343 ns/op 1.50
Benchmark_Utils_GetOffer/1_parameter 216 ns/op 0 B/op 0 allocs/op 136.5 ns/op 0 B/op 0 allocs/op 1.58
Benchmark_Utils_GetOffer/1_parameter - ns/op 216 ns/op 136.5 ns/op 1.58
`Benchmark_RoutePatternMatch//api/:param/fixedEnd_ not_match _/api/abc/def/fixedEnd - allocs/op` 14 allocs/op
Benchmark_Middleware_BasicAuth - B/op 80 B/op 48 B/op 1.67
Benchmark_Middleware_BasicAuth - allocs/op 5 allocs/op 3 allocs/op 1.67
Benchmark_Middleware_BasicAuth_Upper - B/op 80 B/op 48 B/op 1.67
Benchmark_Middleware_BasicAuth_Upper - allocs/op 5 allocs/op 3 allocs/op 1.67
Benchmark_CORS_NewHandler - B/op 16 B/op 0 B/op +∞
Benchmark_CORS_NewHandler - allocs/op 1 allocs/op 0 allocs/op +∞
Benchmark_CORS_NewHandlerSingleOrigin - B/op 16 B/op 0 B/op +∞
Benchmark_CORS_NewHandlerSingleOrigin - allocs/op 1 allocs/op 0 allocs/op +∞
Benchmark_CORS_NewHandlerPreflight - B/op 104 B/op 0 B/op +∞
Benchmark_CORS_NewHandlerPreflight - allocs/op 5 allocs/op 0 allocs/op +∞
Benchmark_CORS_NewHandlerPreflightSingleOrigin - B/op 104 B/op 0 B/op +∞
Benchmark_CORS_NewHandlerPreflightSingleOrigin - allocs/op 5 allocs/op 0 allocs/op +∞
Benchmark_CORS_NewHandlerPreflightWildcard - B/op 104 B/op 0 B/op +∞
Benchmark_CORS_NewHandlerPreflightWildcard - allocs/op 5 allocs/op 0 allocs/op +∞
Benchmark_Middleware_CSRF_GenerateToken - B/op 514 B/op 341 B/op 1.51
Benchmark_Middleware_CSRF_GenerateToken - allocs/op 10 allocs/op 6 allocs/op 1.67

This comment was automatically generated by workflow using github-action-benchmark.

Please sign in to comment.