Changes to get tests passing on .NET standard #65
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
terite
force-pushed
the
david/net-standard
branch
from
8a5844f to
a61d8f7
Compare
In my experience, DisconnectAndReconnect and DisconnectAndReconnectAsync fail inconsistently. When they fail, they often take 10 seconds to do so. With this change, flaky tests fail faster and more often.
StartReceive does not need to call ReceiveFromAsync more than once because Receive will call StartReceive when it's done.
ermau told me that this is an obsolete test that can be removed.
ConnectionFailed is not a test where repeated runs add more information.
terite
force-pushed
the
david/net-standard
branch
from
77caab2 to
e6f3432
Compare
These two test cases don't pass consistently for UDP tests. They mostly fail due to timeouts. This seems like lock contention issues, or maybe an inefficient UDP ack.
This test now takes less time to fail, and fails more often for invalid code. In addition, this test now asserts that the OnConnect and OnDisconnect events fire the correct number of times.
Calling Disconnect sends a DisconnectMessage. Receiving a DisconnectMessage calls Disconnect. The response to a DisconnectMessage is a DisconnectMessage. To avoid odd situations like OnDisconnected being fired twice, only allow one call to Disconnect at a time.
In stressful situations, Socket.ReceiveFromAsync can be "sync" many times in a row, enough to cause a stack overflow. Switching to a while loop and Customized "Completed" handling avoids these stack overflows. Some socket errors are transient, and should not trigger disposal of shared resources. Previously, any errors (or even empty payloads) were treated as fatal errors which ended the receive loop. This resulted in connections silently ignoring incoming messages.
This reverts commit 379a4ff.
|
@ermau I'm going to start splitting this up into smaller parts soon. Let me know if you have any big-picture feedback first though :) |
terite
changed the title
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The goal for this PR is to get tests passing, then pull out relevant commits into their own PRs.