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

Akka.TestKit: EventFilter.Expect_Async waits full expect-msg-after period even if internal method has already finished #7173

Open
Aaronontheweb opened this issue Apr 26, 2024 · 5 comments
Labels
akka-testkit Akka.NET Testkit issues won't fix
Milestone

Comments

@Aaronontheweb
Copy link
Member

Version Information
Version of Akka.NET? v1.5.19 and later
Which Akka.NET Modules? Akka.TestKit

Describe the bug

Consider the following reproduction test:

public class TimerStartupCrashBugFixSpec : AkkaSpec
{
    public TimerStartupCrashBugFixSpec(ITestOutputHelper output) : base(output: output, Akka.Configuration.Config.Empty)
    {
        Sys.Log.Info("Starting TimerStartupCrashBugFixSpec");
    }
    
    private class TimerActor : UntypedActor, IWithTimers
    {
        public sealed class Check
        {
            public static Check Instance { get; } = new Check();
            private Check() { }
        }
        
        public sealed class Hit
        {
            public static Hit Instance { get; } = new Hit();
            private Hit() { }
        }
        
        private readonly ILoggingAdapter _log = Context.GetLogger();
        private int _counter = 0;
        public ITimerScheduler? Timers { get; set; } = null;

        protected override void PreStart()
        {
            Timers?.StartPeriodicTimer("key", Hit.Instance, TimeSpan.FromMilliseconds(1));
        }

        protected override void OnReceive(object message)
        {
            switch (message)
            {
                case Check _:
                    _log.Info("Check received");
                    Sender.Tell(_counter);
                    break;
                case Hit _:
                    _log.Info("Hit received");
                    _counter++;
                    break;
            }
        }

        protected override void PreRestart(Exception reason, object message)
        {
            _log.Error(reason, "Not restarting - shutting down");
            Context.Stop(Self);
        }
    }
    
    [Fact]
    public async Task TimerActor_should_not_crash_on_startup()
    {
        var actor = Sys.ActorOf(Props.Create(() => new TimerActor()).WithRouter(new RoundRobinPool(10)));
        
        // bug: EventFilter waits full 3 seconds even if the underlying operation completes earlier
        await EventFilter.Exception(typeof(ActorInitializationException)).ExpectAsync(0, async () =>
        {
            var i = 0;
            while (i == 0)
            {
                // guarantee that the actor has started and processed a message from scheduler
                i = await actor.Ask<int>(TimerActor.Check.Instance);
            }
        });
    }
}

i = await actor.Ask<int>(TimerActor.Check.Instance); completes in under 100ms. The test still waits a full 3 seconds to exit.

Expected behavior

The way all of the assertions in the TestKit should be designed is exit as soon as:

  1. The thing being tested has completed successfully OR
  2. The time period has elapsed.

Actual behavior

Right now it looks like we're waiting FOR BOTH of these things to complete. This might be adding significant execution time to the entire Akka.NET test suite.

@Aaronontheweb
Copy link
Member Author

Are there any other async TestKit methods that behave like this @Arkatufus ?

@Aaronontheweb Aaronontheweb modified the milestones: 1.5.20, 1.5.21 Apr 29, 2024
@Arkatufus
Copy link
Contributor

This is by design, when you set expectedCount in ExpectAsync() to 0, the ExpectAsync() behaves exactly like ExpectNoMsg() because the filter does not know if the async Func<Task<T>> have a side effect that happens in the future.

var expected = expectedOccurrences.GetValueOrDefault();
if (expected > 0)
{
await _testkit.AwaitConditionNoThrowAsync(() => Task.FromResult(matchedEventHandler.ReceivedCount >= expected), timeout, cancellationToken: cancellationToken);
return matchedEventHandler.ReceivedCount == expected;
}
else
{
// if expecting no events to arrive - assert that given condition will never match
var foundEvent = await _testkit.AwaitConditionNoThrowAsync(() => Task.FromResult(matchedEventHandler.ReceivedCount > 0), timeout, cancellationToken: cancellationToken);
return foundEvent == false;
}

To shorten the time, set the timeout value to less than 3 seconds.

@Arkatufus
Copy link
Contributor

Arkatufus commented Apr 30, 2024

An improvement to this is to disallow setting expectedCount to 0 and create another event filter method called ExpectNoneAsync() to make this clearer to the user.

@Arkatufus
Copy link
Contributor

Arkatufus commented Apr 30, 2024

The reproduction code is slightly suspect, if the unit test is supposed to check that the actor did not crash during startup, then the actor creation itself should be inside the filter.
What this unit test testing is checking that the Ask() operation did not time out.

@Aaronontheweb
Copy link
Member Author

The reproduction code is slightly suspect, if the unit test is supposed to check that the actor did not crash during startup, then the actor creation itself should be inside the filter. What this unit test testing is checking that the Ask() operation did not time out.

The Ask operation will time out if the actor fails during startup as it can't process the message when this occurs.

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 won't fix
Projects
None yet
Development

No branches or pull requests

2 participants