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

Unit testing async pessimistic TimeoutPolicy #623

Closed
utkarsh5k opened this issue Mar 21, 2019 · 4 comments
Closed

Unit testing async pessimistic TimeoutPolicy #623

utkarsh5k opened this issue Mar 21, 2019 · 4 comments

Comments

@utkarsh5k
Copy link

Summary

I am trying to test whether my pessimistic timeout policies work as expected. For this, I tried to mock the function on which I have a timeout to just suspend the thread for a time interval > specified timeout. I expected that this should throw a TimeoutRejectedException which should then be handled in whatever way specified. However, I am seeing that after the given time interval, the task completes and the Timeout is never executed.

Approach/Pseudo-code

// Lets assume I have a timeout policy which is: 
Policy perAttemptTimeoutPolicy = Policy.TimeoutAsync(2, TimeoutStrategy.Pessimistic, OnTimeout);
var retryPolicy = Policy.Handle<Exception>().WaitAndRetryAsync(2, retryAttempt => TimeSpan.FromSeconds(1));
Policy finalPolicy = retryPolicy.WrapAsync(perAttemptTimeoutPolicy);
...
...
// I am now executing a certain function Func() which is supposed to timeout. 
ExecuteWithPolicyAsync(finalPolicy, Func);
...
...
// Now when I am unit testing this code, I mock this function in the following manner
// Assuming that whenever this flow is tested by the UT, the return value is returnValue and the function suspends
// for 5 seconds (ideally initiating a timeout and the outer retry policy)
someObject.Func().Returns(returnValue).AndDoes(x => Thread.Sleep(5000));

However, this does not work quite as expected. The Timeout is never executed and the function suspends for whatever interval I provide, and then the execution continues without failure. However, the timeout policy actually works, as I have seen in my local runs.

// Interestingly, adding the following line to the actual body of Func() does not change anything: 
Func()
{
    Thread.Sleep(5000);
    ...
    ...
    return returnValue;
}

I am fairly confident in my use of the Timeout policy, and it works as expected in my flows. However, the unit tests are not. Is there anything I am doing wrong, or is Timeout policy difficult to capture in unit tests?

Thanks.

@reisenberger
Copy link
Member

reisenberger commented Mar 21, 2019

Hi @utkarsh5k . Compare the following two issues which I believe describe the same scenario, albeit with different test/mock harnesses:

Summary: Async pessimistic timeout policy intentionally relies on delegates executed through the policy conforming to the normal async pattern; that is, returning a Task representing ongoing work. Configuring .AndDoes(x => Thread.Sleep(5000) in the test obviously configures an entirely synchronous delegate which doesn't conform to that normal async pattern.

This follow up comment explains why I elected that async pessimistic timeout policy should not handle this case. Summary: Having async pessimistic timeout policy handle that case (async code which is badly behaved anyway in being purely synchronous) would impose an unnecessary performance hit on the majority use case.

We document this in the timeout wiki here:

Note that pessimistic timeout for async executions will not timeout purely synchronous delegates which happen to be labelled async. It expects that the executed async code conforms to the standard async pattern, returning a Task representing the continuing execution of that async work (for example when the executed delegate hits the first internal await statement).


Let me know if that makes sense.

If you can re-cast your unit test/any manual testing to use an async callback instead of .AndDoes(...) - and to use await Task.Delay(5000); instead of Thread.Sleep(5000); - then you should be able to construct your own test to prove it works. See here for how I fixed up a similar test which used Moq.

If you're interested to see Polly's own tests on the policy, they are here.

EDIT: For future people landing here, we also publish a wiki page exploring different dimensions of unit-testing with Polly.

@reisenberger reisenberger changed the title Unit testing TimeoutPolicy Unit testing async TimeoutPolicy Mar 21, 2019
@reisenberger reisenberger changed the title Unit testing async TimeoutPolicy Unit testing async pessimistic TimeoutPolicy Mar 21, 2019
@reisenberger
Copy link
Member

reisenberger commented Mar 26, 2019

Hi @utkarsh5k . Did you have any further questions around this or anything further we could help with? I had a quick look at applying the same approach I used with Moq (move the async part to the .Returns(), not the callback - to work around Moq/NSubstitute not providing async callback configuration). It looks like the same can be done with NSubstitute:

    using System;
    using System.Threading.Tasks;
    using FluentAssertions;
    using NSubstitute;
    using Polly;
    using Polly.Timeout;
    using Xunit;

    public class UnitTest1
    {
        [Fact]
        public async Task TestAsyncPessimisticTimeoutPolicyWithNSubstitute()
        {
            var timeoutPolicy = Policy.TimeoutAsync(2, TimeoutStrategy.Pessimistic);

            var someObject = Substitute.For<SomeClass>();
            someObject.Func().Returns(async _ => { await Task.Delay(TimeSpan.FromSeconds(30)); });

            timeoutPolicy.Awaiting(p => p.ExecuteAsync(() => someObject.Func())).Should().Throw<TimeoutRejectedException>();

            // This should really be two separate tests - just putting it all in one test for speed/brevity.
            someObject.Func().Returns(async _ => { await Task.Delay(TimeSpan.FromSeconds(0.1)); });
            timeoutPolicy.Awaiting(p => p.ExecuteAsync(() => someObject.Func())).Should().NotThrow<TimeoutRejectedException>();
        }

        public class SomeClass
        {
            public virtual async Task Func() { }
        }
    }

@utkarsh5k
Copy link
Author

Hi @reisenberger , apologies for the delay in my response, I was away from work for a while. I have used a similar approach to what you suggested in your answer, and it works perfectly for me now. Once again, thank you for the explanation!

@reisenberger
Copy link
Member

No problem. Great. Glad you got it sorted! 👍

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

No branches or pull requests

2 participants