The object is returned the second time
Closed this issue · 3 comments
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);
}
}
}
I tested on
.Net 8
Nsubstitute 5.1.0
xunit 2.8.1
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
I assume the questions are answered, thus closing this one.