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

Bugfix/async event handlers return instantly #808

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

jmartschinke
Copy link

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.

@@ -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}"
Copy link
Member

Choose a reason for hiding this comment

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

question: does this need to be a new project, or can the tests be incorporated into NSubstitute.Acceptance.Specs?

Copy link
Author

Choose a reason for hiding this comment

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

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
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants