Add timeout support on send #148
Conversation
Add internal/errorgrp package to support cancellable error groups Add tests for push/pull timeout
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## main #148 +/- ##
==========================================
- Coverage 68.51% 68.07% -0.44%
==========================================
Files 29 30 +1
Lines 2560 2600 +40
==========================================
+ Hits 1754 1770 +16
- Misses 704 727 +23
- Partials 102 103 +1 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
thanks for putting this together.
according to codecov:
https://app.codecov.io/gh/go-zeromq/zmq4/pull/148/blob/internal/errorgrp/errorgrp.go
we don't exercize the whole errorgrp.Group2.Go code.
we could probably try to make sure we still get the context error when it expires during the execution of f.
(probably with a 2-level handshake where the cancel() happens when we are sure f is being executed)
This is sort of odd - I had impression that only block execution of f if context is already cancelled (i.e. if the eg has limits on number of executable functions and parent context got cancelled prior next f in eg started). update: Test case was missing |
Increase test coverage. Minor correction according to PR review.
|
I think its ready for next review cycle. Please comment |
Unfortunatelly the errgroup.Group is not copyable so had to handle implicity initialization in all methods.
|
Please take a look. Still not sure how well it is on implicit errgroup.Group initialization |
|
Dough - original init() was nicer for codecov. Will wait for your comments |
Should be coverage friendly
The fix is similar to one of previous commit with deadlock fix
Add internal/errorgrp package to support cancellable error groups
Add tests for push/pull timeout