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

fixed - InternalExpectAsync does not await actionAsync() - causing ac… #5538

Merged
merged 3 commits into from
Feb 3, 2022
Merged

fixed - InternalExpectAsync does not await actionAsync() - causing ac… #5538

merged 3 commits into from
Feb 3, 2022

Conversation

brah-mcdude
Copy link
Contributor

…tionAsync to run as a detached task #5537

Fixes #
InternalExpectAsync does not await actionAsync() - causing actionAsync to run as a detached task #5537

Changes

  1. added a test ExpectAsync_should_await_actionAsync that reproduced the issue by failing and stopped failing when fix was applied.
  2. the actual fix: added async and await.

Please provide a brief description of the changes here.

  1. the issue was that a test that should have failed, was not failing.
  2. so I created test that reproduces this issue - a test that fails - when a failed assertion was not failing (a false positive).
  3. once I created the test (RED), I fixed the issue (GREEN).
  4. done

Include data from before / after this change here.

Before:

 Akka.TestKit.Tests.Xunit2.TestEventListenerTests.CustomEventFilterWarningTests.ExpectAsync_should_await_actionAsync
   Source: AllTestForEventFilterBase.cs line180Duration:3.6 sec

  Message:Assert.Throws() Failure
Expected: typeof(Akka.TestKit.Xunit2.Internals.AkkaEqualException)
Actual:   (No exception was thrown)

  Stack Trace: 
AllTestForEventFilterBase`1.ExpectAsync_should_await_actionAsync() line 183
--- End of stack trace from previous location ---

After:

 Akka.TestKit.Tests.Xunit2.TestEventListenerTests.CustomEventFilterWarningTests.ExpectAsync_should_await_actionAsync
   Source: AllTestForEventFilterBase.cs line180Duration:450 ms

@Aaronontheweb Aaronontheweb self-requested a review January 25, 2022 19:36
Copy link
Member

@Aaronontheweb Aaronontheweb left a comment

Choose a reason for hiding this comment

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

LGTM

@Aaronontheweb Aaronontheweb merged commit 60a7b40 into akkadotnet:dev Feb 3, 2022
@brah-mcdude brah-mcdude deleted the InternalExpectAsyncDoesNotAwaitActionAsync branch February 3, 2022 16:35
@brah-mcdude
Copy link
Contributor Author

Just for the record, here is the original pull request where I described and reproduced the issue:
https://github.com/akkadotnet/akka.net/pull/5529/files

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants