Bugfix/async event handlers return instantly #808
Conversation
This file contains 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
…the test TestSubstituteWithAsynchronousHandler does not work because of the bug
dtchepak
reviewed
May 14, 2024
| @@ -29,6 +29,8 @@ Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "NSubstitute.Acceptance.Spec | |||
| EndProject | |||
| Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "NSubstitute.Benchmarks", "tests\NSubstitute.Benchmarks\NSubstitute.Benchmarks.csproj", "{D2D162D4-EF1D-4B40-8736-9228C2FEA16C}" | |||
| EndProject | |||
| Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "TestAsyncEventHandlersWithNSubstiute", "tests\Nsubstitute.TestAsyncEventHandlersWithNSubstiute\TestAsyncEventHandlersWithNSubstiute.csproj", "{F9393821-D811-433C-8642-584F94701F1D}" | |||
There was a problem hiding this comment.
question: does this need to be a new project, or can the tests be incorporated into NSubstitute.Acceptance.Specs?
There was a problem hiding this comment.
Good idea, I see no problem moving it to there.
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.
When trying to test an asynchronous event handler with Raise.Event, the Raise.Event immediately returns on the first thread change in the asynchronous event handler.
This makes testing the behaviour of the event handler almost impossible as you cannot be sure that the eventhandler is finished. That leads to bad test code, for example just waiting for a set amount of time in hopes the event handler has finished.
I fixed this bug by waiting for the event handler to finish with GetAwaiter().GetResult(); which is the safest way to synchronously wait for an asnychronous method to finish.
As handler.DynamicInvoke(eventArguments) already returned a Task independent of the return value of the event handler, it made this bugfix really small.