-
Notifications
You must be signed in to change notification settings - Fork 856
ported timeoutTicker fix from 6.2.0 #2458
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
Conversation
Codecov Report❌ Patch coverage is
❌ Your project status has failed because the head coverage (47.23%) is below the target coverage (60.00%). You can increase the head coverage or adjust the target coverage. Additional details and impacted files@@ Coverage Diff @@
## main #2458 +/- ##
=======================================
Coverage 51.56% 51.56%
=======================================
Files 1543 1543
Lines 156818 156828 +10
=======================================
+ Hits 80856 80865 +9
- Misses 69861 69862 +1
Partials 6101 6101
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
| } | ||
| }) | ||
| if err != nil { | ||
| t.Fatal(err) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
use testify since the repo depends on it already and other tests use it?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
there are 2 problems with testify:
- it is weakly typed (I've provided strongly typed wrapper in sei-tendermint/libs/utils/require)
- require package works only from the test's main goroutine (same as t.Fatal though), assert package is a wrapper on t.Errorf which reports errors lazily (the test actually continues after t.Errorf call, which obfuscates error - extra logs, potential deadlocks, cascading errors).
Hence, I prefer regular error passing (or optionally panics for test helpers, for which error passing gets too verbose). The final t.Fatal call is just for reporting the error back to the testing harness.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK thanks for explaining.
Would be nice to maintain less custom code and converge on using existing popular enough libraries maintained by someone other than us 😅
But both approaches work.
Ported sei-protocol/sei-tendermint#331