-
Notifications
You must be signed in to change notification settings - Fork 1k
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
Add ReceiveAsync feature to Akka.TestKit TestActorRef #6281
Add ReceiveAsync feature to Akka.TestKit TestActorRef #6281
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM but not sure if we need an API approval here
context.CheckReceiveTimeout(); | ||
} | ||
else | ||
{ | ||
context.Self.AsInstanceOf<IInternalActorRef>().SendSystemMessage(new ActorTaskSchedulerMessage(exception, actorScheduler.CurrentMessage)); | ||
} | ||
|
||
// Used by TestActorRef to intercept async execution result | ||
if(actorScheduler is IAsyncResultInterceptor interceptor) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd prefer not to do this on the TaskScheduler
that all actors use, but I don't really see a way around it - this is probably the best call we can make in the short run, without redesigining the mailbox system to use Task
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, other alternatives would require a more radical change to implement, this is the shortest solution I can find without scrambling codes all over the place
/// </summary> | ||
/// <param name="message">The message.</param> | ||
/// <param name="sender">The sender.</param> | ||
public Task ReceiveAsync(object message, IActorRef sender = null) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
won't this require API approvals @Arkatufus ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It doesn't look like it, at least not locally
Yep, did not require API approval |
Co-authored-by: Aaron Stannard <[email protected]> (cherry picked from commit 5605d83)
* Add ReceiveAsync feature to Akka.TestKit TestActorRef (#6281) Co-authored-by: Aaron Stannard <[email protected]> (cherry picked from commit 5605d83) * Post-merge fix
Fixes #6265
Changes