Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
3 tasks done
gaby opened this issue Mar 3, 2024 · 3 comments · Fixed by #2892
Closed
3 tasks done

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

gaby opened this issue Mar 3, 2024 · 3 comments · Fixed by #2892

Comments

@gaby
Copy link
Member

gaby commented Mar 3, 2024

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 #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 #2874
Testify Notice: stretchr/testify#1392

How to Reproduce

N/A

Expected Behavior

N/A

Fiber Version

v3.0.0

Code Snippet (optional)

No response

Checklist:

  • I agree to follow Fiber's Code of Conduct.
  • I have checked for existing issues that describe my problem prior to opening this one.
  • I understand that improperly formatted bug reports may be closed without explanation.
@gaby gaby added the ☢️ Bug label Mar 3, 2024
@nickajacks1
Copy link
Member

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
Copy link
Member Author

gaby commented Mar 3, 2024

@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
Copy link
Member Author

gaby commented Mar 3, 2024

Related to #2871

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants