NullReferenceException thrown when `Do` method is setup using async/await

[kensykora]

Normally in Moq, when I want to validate that a method was called with a complex object that has a certain property matched, I would do something like:

object.Verify(x => x.Method(It.Is(x => x.Property == "someValueIExpect")), Times.Once)

In this library, it seems like the only way to achieve this type of behavior is with the .Do() method and setting a local boolean there. However, when I try to do this in a method that uses async/await, I get a NullReferenceException thrown.

Consider the example library code:

    public interface ITestClass
    {
        Task<ApiResult<ApiResponse>> RegisterAsync(ApiRequest apiRequestData);

        ApiResult<ApiResponse> Register(ApiRequest apiRequestData);
    }

    public class ApiRequest
    {
        public Guid Id { get; set; }
    }

    public class ApiResponse
    {
        public PocoObject[] SomeArray { get; set; }
    }

    public class PocoObject
    {
        public string SomeString { get; set; }
    }

    public class ApiResult<TResponseType>
    {

        public HttpStatusCode HttpStatusCode { get; set; }

        public ApiCallStatus Status { get; set; }

        public TResponseType ResultData { get; set; }

    }

    public enum ApiCallStatus
    {
        Success,
        Failure
    }

And the following Mock and XUnit Tests:

    public class TestClassMock : MockBase<ITestClass>, ITestClass
    {
        public TestClassMock(MockBehavior behavior = MockBehavior.Loose)
            : base(behavior)
        {
        }

        public async Task<ApiResult<ApiResponse>> RegisterAsync(ApiRequest apiRequestData)
        {
            return await this.Apply(x => x.RegisterAsync(apiRequestData)); //Null Reference Exception thrown here in the first test
        }

        public ApiResult<ApiResponse> Register(ApiRequest apiRequestData)
        {
            return this.Apply(x => x.Register(apiRequestData));
        }
    }

public class UnitTests
    {
        [Fact]
        public async Task TestRegisterAsync() //Fails :(
        {
            var mock = new TestClassMock();
            var id = Guid.NewGuid();
            var calledWithId = false;

            mock.When(x => x.RegisterAsync(It.IsAny<ApiRequest>())).Do((ApiRequest req) => calledWithId = req.Id == id);

            var result = await mock.RegisterAsync(new ApiRequest { Id = id });

            mock.Verify(x => x.RegisterAsync(It.IsAny<ApiRequest>()));
            Assert.True(calledWithId);
        }

        [Fact]
        public async Task TestRegisterAsync_WithoutTest() //Success!
        {
            var mock = new TestClassMock();
            var id = Guid.NewGuid();

            mock.When(x => x.RegisterAsync(It.IsAny<ApiRequest>())).Return(
                Task.FromResult(new ApiResult<ApiResponse>
                                        {
                                            HttpStatusCode = HttpStatusCode.OK,
                                            ResultData = new ApiResponse
                                                            {
                                                                SomeArray = new[]
                                                                                {
                                                                                    new PocoObject() { SomeString = "ABC123" } 
                                                                                }
                                                            }
                                        }));

            var result = await mock.RegisterAsync(new ApiRequest { Id = id });

            mock.Verify(x => x.RegisterAsync(It.IsAny<ApiRequest>()));
            Assert.Equal("ABC123", result.ResultData.SomeArray.First().SomeString);
        }

        [Fact]
        public async Task TestRegister() //Success!
        {
            var mock = new TestClassMock();
            var id = Guid.NewGuid();
            var calledWithId = false;

            mock.When(x => x.Register(It.IsAny<ApiRequest>()))
                .Do((ApiRequest req) => calledWithId = req.Id == id);

            mock.Register(new ApiRequest { Id = id });

            mock.Verify(x => x.Register(It.IsAny<ApiRequest>()));
            Assert.True(calledWithId);
        }
    }

When I run this in Xamarin.iOS test runner, The first test that has a .Do() (async/await) fails while second test without a .Do() (async) is successful, and the third test with a .Do() is also successful.

It seems that there is a bug with async/await and registering a .Do() in that this null reference exception is thrown.

I created a minimal reproduction of the issue here, requires Xamarin (tested on visual studio): http://cl.ly/2T2P3u3n361t

[kentcb]

Have you tried Verify with It.Matches? Something like:

mock
    .Verify(x => x.SomeMethod(It.Matches(y => y.SomeProperty == "foo")))
    .WasCalledExactlyOnce();

[kensykora]

That does actually solve my use case! Thanks!

However, I think there is still a bug in here somewhere related to Loose behavior and .Do method with async/await. I think it may have to do with the lack of .Returns() setup

[kentcb]

@kensykora can you provide a repro?

[kensykora]

I did -- see link above: http://cl.ly/2T2P3u3n361t

[kentcb]

Apologies - I misunderstood what you were saying.

It sounds like loose behavior is not quite what you're expecting. Loose behavior in PCLMock simply means that if a method returns a value of type T, default(T) will be returned. Therefore, if your method returns a Task or Task<T>, null will be returned unless you specify otherwise. Of course, if you await null, you'll get an exception.

You should specify return values for such methods when either:

1. Your mock is created with a loose behavior OR
2. You're providing your own specifications that override your defaults

Example of 1

In your TestClassMock you would have this:

public TestClassMock(MockBehavior behavior = MockBehavior.Loose)
    : base(behavior)
{
    if (behavior == MockBehavior.Loose)
    {
        this
            .When(x => x.RegisterAsync(It.IsAny<ApiRequest>()))
            .Return(Task.FromResult<ApiResult<ApiResponse>>(null));
    }
}

This ensures that, by default, the RegisterAsync method returns a Task that contains null, rather than returning null itself.

If you're using the code generator, the partial classes will include a partial method called ConfigureLooseBehavior that you would use to provide these specifications. I suspect you're not using codegen because you're using Xamarin Studio/mono and it's not working there yet. Hopefully this will be fixed when Roslyn is incorporated into their tool chain (see http://tirania.org/blog/archive/2015/Jul-21.html). If not, I will be looking into the issue further at that point.

Example of 2

Even if you have provided relevant specifications for loose behavior (always a good idea), overriding those specifications can catch you out. In your TestRegisterAsync method, you should ensure you specify a return value:

mock.When(x => x.RegisterAsync(It.IsAny<ApiRequest>()))
    .Do((ApiRequest req) => calledWithId = req.Id == id)
    .Return(Task.FromResult<ApiResult<ApiResponse>>(null));

Can PCLMock do better?

Maybe. Probably. Perhaps.

It may seem "obvious" that loose behavior should detect for methods that return Task or Task<T> and automatically return an appropriate Task rather than null. OK. But then what about other types? What about observables? Should PCLMock automatically return Observable.FromResult(null) instead of null?

Maybe if PCLMock allowed consuming code to participate in decisions around loose behavior (such as being able to make the decision that any method returning Task<T> should return Task.FromResult(default(T)))? shrugs I simply haven't thought about this enough to know. I'll open a separate issue so it gets due consideration. In the meantime, please close this issue if it answers your question.

[kensykora]

example no. 2 hits on the crux of the issue I'm encountering. Loose behavior expecting for async/await that you specify a task needs to return. I'll post opinions in #19 -- Thanks for your help, this library is a savior!