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

p2p: fixed flakey TestWithSendTimeout #2830

Merged
merged 1 commit into from
Jan 26, 2024

Conversation

pinebit
Copy link
Contributor

@pinebit pinebit commented Jan 26, 2024

Fixed flakey TestWithSendTimeout.

category: bug
ticket: none

Copy link

sonarcloud bot commented Jan 26, 2024

Quality Gate Passed Quality Gate passed

Kudos, no new issues were introduced!

0 New issues
0 Security Hotspots
No data about Coverage
0.0% Duplication on New Code

See analysis details on SonarCloud

p2p.RegisterHandler("test", server, protocolID, func() proto.Message { return new(pbv1.Duty) },
func(ctx context.Context, peerID peer.ID, req proto.Message) (proto.Message, bool, error) {
// The delay must be much greater than the send timeout to trigger the deadline error.
Copy link
Contributor

Choose a reason for hiding this comment

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

What's the send timeout?

Copy link
Contributor Author

@pinebit pinebit Jan 26, 2024

Choose a reason for hiding this comment

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

See github.com/libp2p/go-yamux/v4@v4.0.1/stream.go for details.
It creates a timer in "future" and then select takes either message or a timer signal. If the test runs fast (the reason for being flakey) select chooses the message and not the timer event in the first pass. Then it closes the stream. That's how I understood the problem..

Copy link

codecov bot commented Jan 26, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (4aae1fb) 53.21% compared to head (5860dc0) 53.22%.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #2830   +/-   ##
=======================================
  Coverage   53.21%   53.22%           
=======================================
  Files         200      200           
  Lines       27705    27705           
=======================================
+ Hits        14744    14745    +1     
  Misses      11130    11130           
+ Partials     1831     1830    -1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@pinebit pinebit added the merge when ready Indicates bulldozer bot may merge when all checks pass label Jan 26, 2024
@obol-bulldozer obol-bulldozer bot merged commit a44f4dc into main Jan 26, 2024
12 checks passed
@obol-bulldozer obol-bulldozer bot deleted the pinebit/fixing-flakey-test-p2p branch January 26, 2024 07:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
merge when ready Indicates bulldozer bot may merge when all checks pass
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants