nsubstitute / NSubstitute

A friendly substitute for .NET mocking libraries.
https://nsubstitute.github.io
Other
2.65k stars 260 forks source link

The object is returned the second time #832

Open IlyaZanegin opened 1 week ago

IlyaZanegin commented 1 week ago

Hello everybody!

why is an empty object returned the first time? Please explain to me


using NSubstitute;
using System.Net;
using System.Reflection;
using Xunit;
using Assert = Xunit.Assert;

namespace RepositoryServiceTests
{
    public class ApiResult<T>
    {
        public HttpStatusCode StatusCode { get; set; }
        public T? Value { get; set; }
    }

    public abstract class BaseClass
    {
        protected virtual async Task<ApiResult<string>> DownloadFile(string path1, string path2)
        {
            var result = new ApiResult<string>();
            await Task.CompletedTask;
            return result;
        }
    }

    public class SomeClass : BaseClass
    {
        public async Task<string?> Download(string path1, string path2)
        {
            var result1 = await DownloadFile(path1, path2);   //return null. why???????????????????????????????
            var result2 = await DownloadFile(path1, path2);   //return object

            return result2.Value;
        }
    }

    public class TestClass
    {
        [Fact]
        public async Task TestMethod()
        {
            // Arrange
            var someClass = Substitute.For<SomeClass>();

            var model = Task.FromResult(
                new ApiResult<string>
                {
                    StatusCode = HttpStatusCode.OK,
                    Value = "path to file"
                });

            var method = someClass.GetType().GetMethod("DownloadFile", BindingFlags.NonPublic | BindingFlags.Instance);
            method!.Invoke(someClass, [Arg.Any<string>(), Arg.Any<string>()]).Returns(model);

            // Act
            var result = await someClass.Download(Arg.Any<string>(), Arg.Any<string>());

            // Assert
            Assert.NotNull(result);
        }
    }
}
IlyaZanegin commented 1 week ago

I tested on .Net 8 Nsubstitute 5.1.0 xunit 2.8.1

Ergamon commented 6 days ago

Seems you are quite new to NSubstitute. There are some problems in your code.

You should always use the Analyzers package to get the fitting warnings.

First of all for NSubstitute to work, your code must be visible to NSubstitute. Your protected method is not visible. You try to hack your way with reflection, but this will not work.

The easiest solution is to make the DownloadFile not protected but public. But that is probably an ugly change. The next solution is to make the method internal protected not protected. For using a test project you must work with InternalsVisibleTo.

If all is just one project, like in the simple code, this can look like this:

public class ApiResult<T>
{
  public HttpStatusCode StatusCode { get; set; }
  public T? Value { get; set; }
}

public abstract class BaseClass
{
  internal protected virtual async Task<ApiResult<string>> DownloadFile(string path1, string path2)
  {
    var result = new ApiResult<string>();
    await Task.CompletedTask;
    return result;
  }
}

public class SomeClass : BaseClass
{
  public async Task<string?> Download(string path1, string path2)
  {
    var result1 = await DownloadFile(path1, path2);   //return null. why???????????????????????????????
    var result2 = await DownloadFile(path1, path2);   //return object

    return result2.Value;
  }
}

public class TestClass
{
  [Fact]
  public async Task TestMethod()
  {
    // Arrange
    var someClass = Substitute.For<SomeClass>();

    var model = Task.FromResult(
      new ApiResult<string>
      {
        StatusCode = HttpStatusCode.OK,
        Value = "path to file"
      });

    someClass.DownloadFile(default!, default!).ReturnsForAnyArgs(model);

    // Act
    var result = await someClass.Download("path1", "path2");

    // Assert
    Assert.NotNull(result);
  }
}

You can also see the second mistake. You are passing Arg.Any to a call not to a NSubstitute definition. The intention of your code is clear. You want to say that the parameters dont matter. For this you can use libraries like AutoFixture. But a misplaced Arg.Any can lead to very strange results.

If you dont like the fact that you have to change your production code just for a test library, you can have third class just in your test project. This class subclasses SomeClass and "opens" DownloadFile just for your test code.

The good thing about starting unit testing, is having these problems. In the end they always show some design problems. So in my opinion the best solution would be to get rid of BaseClass, introduce an interface containing the DownloadFile method. Then SomeClass has the interface as dependency. Composition over inheritance. Rewriting the code in this manner not only leads to a better design, but also makes your code easier testable.

But as written, with some modification or an helper class, your original code can also work