NSubstitute icon indicating copy to clipboard operation
NSubstitute copied to clipboard

Setting default return values

Open Lobosque opened this issue 4 years ago • 11 comments

The default return value for string is "" and for integers it is 0. These are very reasonable defaults, but is it possible to change it for recursive mocks without having to mock each and every return possibility? Eg once a mock is created I want that every string returned for this mock, and any mock recursively created, to be "abc" instead "".

Lobosque avatar Mar 05 '21 15:03 Lobosque

It is possible to return a string (for example) for every member in a mock using ReturnsForAll. This will not apply to recursively created mocks though.

While there is no built-in support for applying this to an entire mock, you can try experimenting with the container in NSubstituteDefaultFactory to override the default auto-values provider if you are keen, but be aware you are in very uncharted (undocumented) territory there! 😅 🐉

dtchepak avatar Mar 07 '21 22:03 dtchepak

@dtchepak what about non-primitives? I noticed that by default, when a instance should be returned, the mock just returns null... I kind of expected to return an instance with its simplest constructor. Is there any way around that to avoind manually mocking thousands of types?

Lobosque avatar Mar 08 '21 12:03 Lobosque

@dtchepak I'm also having trouble to mock even very simple cases:

s.ExecuteQueryAsync(new GetCustomerQuery("userId")).Returns(Task.FromResult(new CustomerDto()));
var res = s.ExecuteQueryAsync(new GetCustomerQuery("userId"));
res.Result // Returns null

In this example, I expected res to be a CustomerDto instance, but is is null.

ExecuteQueryAsync signature is:

Task<TResult> ExecuteQueryAsync<TResult>(IQuery<TResult> query);

Lobosque avatar Mar 08 '21 13:03 Lobosque

Sadly I cannot see the code for IQuery<T> and the GetCustomerQuery class, also not where this method resides.

NSubstitute is primary designed for architectures using interfaces as contract definitions.

So if your "s" is a substitute for an interface with this ExecuteQueryAsync method it should work.

There is one problem left and that is GetCustomerQuery("userid"). You new this class 2 times, so if they have no correctly implemented equality, you setup the substitute to return a specific query for query A nd then call the method with query B. It does not matter that they look equal in our human eyes, they must be equal in a machine understandable way.

Basically the Task<TResult> is the problem. NSubstitute only creates classes with a public parameterless constructor. So you have to tell NSubstitute how to create a Task<TResult>.

Ergamon avatar Mar 11 '21 22:03 Ergamon

@Lobosque :

what about non-primitives? ... I kind of expected to return an instance with its simplest constructor.

We try to avoid calling constructors for classes unless all its public members are virtual/abstract and it has a default or parameterless constructor. This is to try to avoid inadvertently running real (potentially destructive) code in your tests. Using the auto-values provider code should let you override this behaviour.

In this example, I expected res to be a CustomerDto instance, but is is null.

I think this is because you are stubbing out a return value for a different query instance. If GetCustomerQuery does not have equality defined then NSubstitute (and .NET in general) will not be able to tell they are meant to be the same. If you can not implement equality for this type, then you can try explicitly matching the argument, using reference equality, or accepting any argument:

// Match argument details:
s.ExecuteQueryAsync(Arg.Is<GetCustomerQuery>(x => id == "userId").Returns(Task.FromResult(new CustomerDto()));
var res = s.ExecuteQueryAsync(new GetCustomerQuery("userId"));

// Reference equality:
val query = new GetCustomerQuery("userId");
s.ExecuteQueryAsync(query).Returns(Task.FromResult(new CustomerDto()));
var res = s.ExecuteQueryAsync(query);

// Accept any arg:
s.ExecuteQueryAsync(null).ReturnsForAnyArg(Task.FromResult(new CustomerDto()));
var res = s.ExecuteQueryAsync(new GetCustomerQuery("userId"));

dtchepak avatar Mar 11 '21 23:03 dtchepak

Why is the default for string the empty string? Why deviate from the defaults the clr uses? Maybe in a nullable context it makes sense to use "" for string and null for string? but when nullable is not enabled?

qsdfplkj avatar Jan 20 '23 15:01 qsdfplkj

Why is the default for string the empty string? Why deviate from the defaults the clr uses? Maybe in a nullable context it makes sense to use "" for string and null for string? but when nullable is not enabled?

This default predates nullable/non-nullable. The idea was to increase the odds of an unconfigured (default) substitute working without throwing when pushed in as a dependency to any class under test. If a particular value is required, it should always be configured on the substitute.

dtchepak avatar Jan 24 '23 08:01 dtchepak

It deviates from the Clr default. My code checks for null. It's like saying the default int == 1 because of the same reasoning (working without throwing errors).

Besides why not fix it now we have nullable.

qsdfplkj avatar Jan 24 '23 10:01 qsdfplkj

It deviates from the Clr default. My code checks for null. It's like saying the default int == 1 because of the same reasoning (working without throwing errors).

We also don't follow clr defaults for arrays, tasks, queryables, observables, substitutable interfaces etc. :) The aim is to make testing more convenient. If it is causing issues for you it is possible to change the defaults.

Besides why not fix it now we have nullable.

I haven't done much with nullable. Do you know how feasible it is to do this in a non-breaking way?

dtchepak avatar Jan 25 '23 02:01 dtchepak

I might have a look if/when I have time... I think it should be straight forward given then link you posted to setup that uses clr a defaults provider.

Breaking changes would be tests that might suddenly fail (given the different defaults) but the code shouldn't break as the nullable doesn't change the code (you still have to do your null checks).

qsdfplkj avatar Jan 25 '23 11:01 qsdfplkj

I don't think you can get the nullable attribute from the Type we need a MemberInfo/PropertyInfo.

This would work for me:

 public class DefaultValueProvidersFactory : IAutoValueProvidersFactory
 {
    public IReadOnlyCollection<IAutoValueProvider> CreateProviders(ISubstituteFactory substituteFactory) => new[] { new DefaultValueProvider() };

    private class DefaultValueProvider : IAutoValueProvider
    {
        public bool CanProvideValueFor(Type type) => true;

        public object GetValue(Type type) => type.IsValueType ? Activator.CreateInstance(type) : null;
    }
}

One question though:

The setup:

        var customizedContainer = NSubstituteDefaultFactory.DefaultContainer.Customize().Decorate<IAutoValueProvidersFactory>((_, _) => new DefaultValueProvidersFactory());
        SubstitutionContext.Current = customizedContainer.Resolve<ISubstitutionContext>();

Why is this SubstitutionContext.Current set while the container is a new instances? Does the current remain set or is this something with a scope like a thread or something?

This doesn't work always correctly, now I get:

NSubstitute.Exceptions.CouldNotSetReturnDueToNoLastCallException: Could not find a call to return from.

qsdfplkj avatar Jan 25 '23 12:01 qsdfplkj