-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
Add integration tests for transports #2194
Comments
Ideally, more than 1 MB. 1 MB still might fit in the initial flow control window, depending on the transport. 20 MB sounds like a safer choice, while still being reasonably fast.
That's 2 tests: The sender resetting the write side and the receiver resetting the read side.
Not a WIP any more after today's work: #2200. go-libp2p/p2p/test/connectiongating/gating_test.go Lines 23 to 29 in 03a37d3
I'm particularly worried about WebSocket here: The documentation says:
And as far as I can tell, we're doing nothing to work around this. If that's the case, this doesn't need to block the PR. We can disable the test for WebSocket and create an issue to fix this in a follow-up PR. Adding to the list in the original post (feel free to copy-paste):
The second test maybe doesn't need to be in the first iteration, if it's adds too much complexity. |
Yes, I'm waiting for reviews there and open questions. Right now this isn't blocking anything and there are other things this week that have a higher priority (e.g. internal perf reviews), so I probably won't get to this this week. |
This is done! |
We should have some integration tests for transports in general to:
The tests should at least exercise:
N = 1000
?). Or alternatively, setting a lower rcmgr limit on the receiver side. The client would need to back off on stream resets then (wait for 50ms, then try again). Eventually, both server and client should have handled exactlyN
streams. transport tests: many streams and lots of data #2296The text was updated successfully, but these errors were encountered: