gofiber / fiber

⚡️ Express inspired web framework written in Go
https://gofiber.io
MIT License
32.12k stars 1.6k forks source link

🐛 [Bug]: Enable go-require for testify-lint #2891

Closed gaby closed 4 months ago

gaby commented 4 months ago

Bug Description

The latest testify release added a notice against using require inside a goroutine. This is something covered by testify-lint through the go-require rule which is enabled by default. This rule was disabled during golangci-lint upgrade in https://github.com/gofiber/fiber/pull/2842

The following tests are failing:

--- FAIL: Test_Ctx_BodyStreamWriter (0.00s)
--- FAIL: Test_Client_Agent_Dest (0.00s)
    --- FAIL: Test_Client_Agent_Dest/small_dest (0.00s)
    --- FAIL: Test_Client_Agent_Dest/enough_dest (0.02s)
--- FAIL: Test_App_New_Test_Parallel (0.00s)
--- FAIL: Test_Client_UserAgent (0.00s)
--- FAIL: Test_App_ReadTimeout (0.57s)
--- FAIL: Test_App_BadRequest (0.58s)
--- FAIL: Test_App_Shutdown (0.00s)
--- FAIL: Test_App_SmallReadBuffer (0.59s)
--- FAIL: Test_Client_Agent_TLS (2.87s)
--- FAIL: Test_App_ShutdownWithContext (3.01s)
--- FAIL: Test_App_ShutdownWithTimeout (3.02s)
--- FAIL: FuzzUtilsGetOffer (0.00s)
FAIL
FAIL    github.com/gofiber/fiber/v3 13.404s
--- FAIL: Test_Proxy_Do_WithRealURL (1.00s)
FAIL
FAIL    github.com/gofiber/fiber/v3/middleware/proxy    8.126s
FAIL

Partly fixed by https://github.com/gofiber/fiber/pull/2874 Testify Notice: https://github.com/stretchr/testify/pull/1392

How to Reproduce

N/A

Expected Behavior

N/A

Fiber Version

v3.0.0

Code Snippet (optional)

No response

Checklist:

nickajacks1 commented 4 months ago

A secondary benefit of this is that it forces you to handle goroutines in a more deterministic way (eg using channels). I've seen people incorrectly assume that their goroutines are finishing by the time the test ends when in reality the test is just exiting before the assertion is run.

gaby commented 4 months ago

@nickajacks1 Yeah, definitely. I'm trying to fix the failing Client test using a channel. The proxy test failing was caused because it was using the default Timeout value for app.Test().

gaby commented 4 months ago

Related to https://github.com/gofiber/fiber/issues/2871