Inconsistent behavior of subsitutes

[cshu]

using System;
using NSubstitute;

	public interface ITest1{
		Test1 Get();
	}
	public class Test1{
	}
	public interface ITest2{
		Test2 Get();
	}
	public class Test2{
		public int X;
	}
	public interface ITest3{
		Test3 Get();
	}
	public class Test3{
		public int X {get;set;}
	}
	class Program
	{
		static void Main(string[] args)
		{
			var subs1 = Substitute.For<ITest1>();
			Console.WriteLine(null == subs1.Get());
			var subs2 = Substitute.For<ITest2>();
			Console.WriteLine(null == subs2.Get());
			var subs3 = Substitute.For<ITest3>();
			Console.WriteLine(null == subs3.Get());
		}
	}

This prints False False True on my Linux with NSubstitute 4.0.0 and .NET Core 2.2.104

It does not feel intuitive.

I came across this because I was confused when running some tests I wrote for a project. Eventually I found out the behaviour above. How about making them all return False or all return True?

[dtchepak]

Hi @cshu,

Thanks for raising this. I understand that this is a bit confusing. Here is how auto mocks work:

Once a substitute has been created some properties and methods will automatically return non-null values. For example, any properties or methods that return an interface, delegate, or purely virtual class* will automatically return substitutes themselves. This is commonly referred to as recursive mocking, and can be useful because you can avoid explicitly creating each substitute, which means less code. Other types, like String and Array, will default to returning empty values rather than nulls.

* Note: A pure virtual class is defined as one with all its public methods and properties defined as virtual or abstract and with a default, parameterless constructor defined as public or protected.

The footnote on "pure virtual classes" describes the source of the inconsistency, but doesn't really explain why.

Explanation

The rationale behind this was to avoid people accidentally running real code without knowing. Say we have a class with some destructive behaviour and another interface that exposes that:

public interface IStorageCleaner {
    void DeleteEverything();
}
public class StorageCleaner : IStorageCleaner {
    /// Deletes everything. Non-virtual, so cannot be overridden in a sub-class/proxy/substitute.
    public void DeleteEverything() {
        /* ... actually delete everything on the device ... */
    }
}
public interface IStorage {
    StorageCleaner Cleaner { get; }
    void Save(...);
}

Now we test a class that depends on this:

class SomeClass {
    IStorage storage;
    public SomeClass(IStorage storage) { this.storage = storage; }

    public void Clean() { 
        storage.Cleaner.DeleteEverything(); 
    }
}

// In Test.cs:
[Test]
public void TestClean() {
  var storage = Substitute.For<IStorage>();
  var subject = new SomeClass(storage);
  
  subject.Clean(); // (1)

  storage.Cleaner.Received().DeleteEverything();
}

At the subject.Clean() call (marked (1))), if we automatically created an instance of StorageCleaner for IStorage.Cleaner then what appears to be a completely reasonable test made using a substitute IStorage has run actual code and accidentally deleted everything! (Note that StorageCleaner.DeleteEverything() is non-virtual, so NSubstitute will not be able to intercept this on a concrete type.)

For this reason, if there is any chance of running actual code (i.e. non-virtual members are present), we make sure people explicitly set a return value rather than auto-creating one behind the scenes.

In the example above, if IStorage.Cleaner had a return type of IStorageCleaner instead of StorageCleaner then NSubstitute knows it is safe to return a substitute for that case.

Conclusion

The main goal for auto-mocking with NSubstitute is to guess helpful return values for substitute members so a substitute will behave reasonably in a test with minimal configuration. For interfaces, a substitute for that interface will be returned. For things like string and Task<T>, empty values will be given as defaults. NSubstitute will err on the side of safety for return types that could produce destructive side-effects, with the view that "why is this defaulting to null?" is a better confusion than "why did this substitute delete everything from my disk?". 😄

I find the most hassle free way to work with this is to explicitly configure a substitute with any values we rely on for our test. Un-configured values will default to something, but by not specifying them we're stating that, for this test, we don't mind what value is returned.

Hope this helps.