Moq return setup returning wrong data on second execution

Viewed 217

I am having an issue setting up the return value in my unit test causing unexpected data to be returned on second execution.

I am testing a service I have created and I have the following code to setup the return data from an interface used within the service.

         _dataService.Setup(x => x.GetJsonFromApi(It.IsAny<string>(), API_KEY, ENTITY_A))
            .Returns(Task.FromResult(new List<Entity>() {
                new Entity {
                    Forename = "first_name",
                    Surname = "last_name",
                    EntityType = ENTITY_A
                }
            }));
        _dataService.Setup(x => x.GetJsonFromApi(It.IsAny<string>(), API_KEY, ENTITY_B))
            .Returns(Task.FromResult(new List<Entity>() {
                    new Entity {
                        Forename = "first_name",
                        Surname = "last_name",
                        EntityType = ENTITY_B
                    }
            }));
        _dataService.Setup(x => x.GetJsonFromApi(It.IsAny<string>(), API_KEY, ENTITY_C))
            .Returns(Task.FromResult(new List<Entity>() {
                    new Entity {
                        Forename = "first_name",
                        Surname = "last_name",
                        EntityType = ENTITY_C
                    }
            }));

The above works fine on the first execution of my service in the unit test with each call returning one object. The code in the service is:

            var data = await _apiDataService.GetJsonFromApi(apiUrl, apiKey, ENTITY_A);
            data.AddRange(await _apiDataService.GetJsonFromApi(apiUrl, apiKey, ENTITY_B));
            data.AddRange(await _apiDataService.GetJsonFromApi(apiUrl, apiKey, ENTITY_C));

On the second execution; the first call (e.g. _apiDataService.GetJsonFromApi(apiUrl, apiKey, ENTITY_A)) is returning a list of three objects instead of the expected one. I allowed the debugger to continue execute of the following two calls with data.AddRange() and the expected single object is returned from both of those on the second execution and added to the list so I end up with five objects.

Is anyone able to tell me what I am doing wrong please.

Some additional info:

The code is executed twice in the unit test method as:

    apiCheckerService.AddEntitiesHash(client.Id).GetAwaiter().GetResult();
    apiCheckerService.AddEntitiesHash(client.Id).GetAwaiter().GetResult();

The properties passed to the GetJsonFromApi method are all strings and the ENTITY_A, ENTITY_B and ENTITY_C are constants, so on the second execution all parameters passed to the function should be exactly the same. I am testing another part of the service that should be locked down on the second run, but need this to run properly for the test to progress.

1 Answers

Most probably (like in 99% of the cases) that's because you are using an overload of .Returns() that takes a ready-to-use value. Please note what are the parameters of .Returns() in your code: it's a Task. Already constructed. Constructed from a value. Value that is already constructed. And it's a List.

This means that the mock will remember this very List<> object and will reuse it later. Anytime anything wants to GetJsonFromApi with given parameters, they will get the very same object instance. Your mock will not give them a new list with similar contents (like a normal HTTP/etc client would behave), but will always return the same object instance.

Now, what will happen if your code elsewhere obtains that list and appends new items to it? Your mock wouldn't notice. It would still happily return the same list instance. Now with more items.

I bet that this is what happens.

For that reason, the .Returns method also has an overload that takes a delegate:

instead of: foo.Returns(new List<int>{ 1, 2, 3 })
try using: foo.Returns(() => new List<int>{ 1, 2, 3 })

instead of: foo.Returns(Task.FromResult(new List<int>{ 1, 2, 3 }))
try using: foo.Returns(() => Task.FromResult(new List<int>{ 1, 2, 3 }))
or even: foo.ReturnsAsync(() => new List<int>{ 1, 2, 3 })

That way the only thing cached by the mock is the lambda, and the lambda is not executed until mocked method is invoked. Then, every time a mocked method is invoked, lambda is executed again, and returns a freshly constructed object. If any code later modifies that object, it doesn't matter, as next invocation of the mocked method will build another fresh response.

Related