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

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

Closed
brah-mcdude opened this issue Jan 25, 2022 · 5 comments
Labels
akka-testkit Akka.NET Testkit issues confirmed bug
Milestone

Comments

@brah-mcdude
Copy link
Contributor

brah-mcdude commented Jan 25, 2022

why does this pass:

/// <summary>
/// this test is a false positive
/// </summary>
/// <returns></returns>
[Fact]
public async Task ReplyingToProbeMessagesFunc()
{
    await EventFilter.Error().ExpectAsync(0, actionAsync: async () =>
    {
        var probe = CreateTestProbe();
        probe.Tell("hello");
        probe.ExpectMsg("hello");
        probe.Reply("world");
        await Task.Run(() => { ExpectMsg("world2"); });
        Assert.Equal(probe.Ref, LastSender);
    });
}

Looks to me like we forgot to.... you know, await the Task returned inside here:

/// <summary>
/// Async version of <see cref="InternalExpect"/>
/// </summary>
private async Task InternalExpectAsync(Func<Task> actionAsync, ActorSystem actorSystem, int expectedCount, TimeSpan? timeout = null)
{
await InterceptAsync<object>(() => { actionAsync(); return Task.FromResult<object>(null); }, actorSystem, timeout, expectedCount);
}

So I suspect the await Task.Run(() => { ExpectMsg("world2"); }); is running as a detatched task, which is incredibly annoying. We just need to update the TestKit to await those tasks internally and then this should pass.

Originally posted by @Aaronontheweb in #5529 (comment)

@Aaronontheweb Aaronontheweb added akka-testkit Akka.NET Testkit issues confirmed bug labels Jan 25, 2022
@Aaronontheweb Aaronontheweb added this to the 1.4.33 milestone Jan 25, 2022
@Aaronontheweb
Copy link
Member

Just need to await actionAsync() and delete the Task.FromResult call there I think.

@brah-mcdude
Copy link
Contributor Author

brah-mcdude commented Jan 25, 2022

For the record - this is the exception thrown by - ExpectMsg("world2"):

Akka.TestKit.Xunit2.Internals.AkkaEqualException
  HResult=0x80131500
  Message=Assert.Equal() Failure
Expected: world2
Actual:   world
  Source=Akka.TestKit.Xunit2
  StackTrace:
   at Akka.TestKit.Xunit2.XunitAssertions.AssertEqual[T](T expected, T actual, String format, Object[] args) in C:\Users\brahm\source\repos\akka.net\src\contrib\testkits\Akka.TestKit.Xunit2\XunitAssertions.cs:line 68
   at Akka.TestKit.TestKitBase.<>c__DisplayClass108_0`1.<ExpectMsg>b__0(T m) in C:\Users\brahm\source\repos\akka.net\src\core\Akka.TestKit\TestKitBase_Expect.cs:line 54
   at Akka.TestKit.TestKitBase.<>c__DisplayClass119_0`1.<InternalExpectMsgEnvelope>b__2(T m, IActorRef sender) in C:\Users\brahm\source\repos\akka.net\src\core\Akka.TestKit\TestKitBase_Expect.cs:line 222
   at Akka.TestKit.TestKitBase.InternalExpectMsgEnvelope[T](Nullable`1 timeout, Action`2 assert, String hint, Boolean shouldLog) in C:\Users\brahm\source\repos\akka.net\src\core\Akka.TestKit\TestKitBase_Expect.cs:line 252
   at Akka.TestKit.TestKitBase.InternalExpectMsgEnvelope[T](Nullable`1 timeout, Action`1 msgAssert, Action`1 senderAssert, String hint) in C:\Users\brahm\source\repos\akka.net\src\core\Akka.TestKit\TestKitBase_Expect.cs:line 224
   at Akka.TestKit.TestKitBase.InternalExpectMsg[T](Nullable`1 timeout, Action`1 msgAssert, String hint) in C:\Users\brahm\source\repos\akka.net\src\core\Akka.TestKit\TestKitBase_Expect.cs:line 199
   at Akka.TestKit.TestKitBase.ExpectMsg[T](T message, Nullable`1 timeout, String hint) in C:\Users\brahm\source\repos\akka.net\src\core\Akka.TestKit\TestKitBase_Expect.cs:line 54
   at Akka.TestKit.Tests.Xunit2.TestEventListenerTests.AllTestForEventFilterBase`1.<ExpectAsync_should_await_actionAsync>b__22_1() in C:\Users\brahm\source\repos\akka.net\src\core\Akka.TestKit.Tests\TestEventListenerTests\AllTestForEventFilterBase.cs:line 183
   at System.Threading.Tasks.Task.InnerInvoke()
   at System.Threading.Tasks.Task.<>c.<.cctor>b__272_0(Object obj)
   at System.Threading.ExecutionContext.RunFromThreadPoolDispatchLoop(Thread threadPoolThread, ExecutionContext executionContext, ContextCallback callback, Object state)

@brah-mcdude
Copy link
Contributor Author

brah-mcdude commented Jan 25, 2022

Just need to await actionAsync() and delete the Task.FromResult call there I think.

@Aaronontheweb:
I could not get rid of:

return Task.FromResult<object>(null); 

Maybe you can?
Also, I had to add an "async" keyword.

Aaronontheweb added a commit that referenced this issue Feb 3, 2022
…tionAsync to run as a detached task #5537 (#5538)

Co-authored-by: Aaron Stannard <[email protected]>
@Aaronontheweb
Copy link
Member

resolved via #5538

@brah-mcdude
Copy link
Contributor Author

Issue seems to appear in another scenario:

This code:

        protected async Task ExpectLogNoWarningsNorErrorsAsync(Func<Task> action)
        {
            await CreateEventFilter(Sys)
                .Custom(CustomErrorOrWarningFilter)
                .ExpectAsync(0, TimeSpan.FromSeconds(3),
                async () =>
                {
                    await action();
                });
        }

Invokes this code:

        /// <summary>
        /// Async version of <see cref="Intercept{T}"/>
        /// </summary>
        protected Task<T> InterceptAsync<T>(Func<T> func, ActorSystem system, TimeSpan? timeout, int? expectedOccurrences, MatchedEventHandler matchedEventHandler = null)
        {
            return InterceptAsync(() => Task.FromResult(func()), system, timeout, expectedOccurrences, matchedEventHandler);
        }

Which seems to invoke:
func()
without awaiting it.

Arkatufus added a commit that referenced this issue Feb 10, 2022
)

* simplified test. i checked that it properly reproduces the issue.

* I added "actionAsync:" parameter name in order to reproduce issue #5537

Co-authored-by: Gregorius Soedharmo <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
akka-testkit Akka.NET Testkit issues confirmed bug
Projects
None yet
Development

No branches or pull requests

2 participants