Is it possible to fake a protected virtual method?

[falcu]

I think this has already been asked but I m double checking because I don't find much info neither in the doc nor in google.
It is very useful to fake protected methods when using the Extract and Override for unit testing legacy code. I could of course create a manual fake, inherit from the base class and override the method I want to fake, but I m using an isolation framework to avoid creating manual fakes.
If this is not possible, are there any plans for including this functionality in the API?

Thanks

[dtchepak]

The NSubstitute API relies on calling a method then configuring it with Returns, which means for standard use we can only configure calls that can be invoked via the public API (which excludes protected and private members).

That said, I think NSubstitute might work ok if you find another way to invoke that method. For example, you could write a Protected(this object target, string methodName, params object[] arg) method to invoke a protected virtual member on a substitute via reflection, then Returns should work on that last call as expected.

Here is a hacky example (I'd need to clarify the exact binding flags to use and how to match overloads. I'd also want to check that the member was virtual to ensure I can set a return value on it):

    // DISCLAIMER: sample code only.
    using NSubstitute;
    using System.Reflection;

    public class Foo {
        protected virtual String DoStuff(int i) { return "default"; } 
        public String ReallyDoStuff(int i) { return DoStuff(i) + "!"; }
    }

    public static class Reflect {
        public static object Protected(this object target, string name, params object[] args) {
            var type = target.GetType();
            var method = type.GetMethods(BindingFlags.NonPublic | BindingFlags.Instance)
                             .Where(x => x.Name == name).Single();
            return method.Invoke(target, args);
        }
    }

    public class Tests {
        [Test]
        public void TestFoo() {
            var sub = Substitute.For<Foo>();
            sub.Protected("DoStuff", Arg.Is<int>(x => x < 10)).Returns(x => x.Arg<int>().ToString());

            Assert.AreEqual("1!", sub.ReallyDoStuff(1));
            Assert.AreEqual("!", sub.ReallyDoStuff(100));
        }
    }

I'm not sure if there are any cases where this would cause major problems. If you end up trying it please let me know how you go.

Regards,
David

[dtchepak]

Testing Received() passes as well:

        [Test]
        public void TestReceived() {
            var sub = Substitute.For<Foo>();

            sub.ReallyDoStuff(5);

            sub.Received().Protected("DoStuff", 5);
            sub.DidNotReceive().Protected("DoStuff", 2);
        }

[falcu]

Thanks!, I m checking it.

Check this behaviour:

    public class Order
    {
        private User user;

        public Order() { }

        public bool Add(User aUser)
        {
            if (CheckValidUser(aUser))
            {
                //Do Stuff
                this.user = aUser;
                return true;
            }
            else
            {
                return false;
            }
        }

        protected virtual bool CheckValidUser(User aUser)
        {
            return aUser.IsValidUser();
        }

    }


    public class User
    {
        public bool IsValidUser()
        {
            return true;
        }
    }

        [TestMethod]
        public void Add_ValidUser_ReturnsTrue()
        {
            Order aOrder = Substitute.For<Order>();

            bool result = aOrder.Add(new User());

            Assert.IsTrue(result);
        }

The test fails. Is that expected? if I remove the virtual from the CheckValidUser method or create a substitute with ForPartsOf, the real implementation of CheckValidUser is called and the test passes. It's weird that in the original case, NSubstitute fakes the method with a default implementation that cannot be changed.

[falcu]

I think your code is fine, I just added a check for virtual:

        public static object Protected(this object target, string name, params object[] args)
        {
            var type = target.GetType();
            var method = type.GetMethods(BindingFlags.NonPublic | BindingFlags.Instance)
                             .Where(x => x.Name == name && x.IsVirtual).Single();
            return method.Invoke(target, args);
        }

[dtchepak]

It's weird that in the original case, NSubstitute fakes the method with a default implementation that cannot be changed.

Oh right, that is odd. In this case it allows us to hack in this ability to stub protected methods. I'm not sure if that behaviour should be changed. Working with classes has always been a bit dicey with NSub.

[falcu]

In Protected(), how can I find the protected virtual method that matches the args passed, when using the NSubstitute Arg class? I could easily get the types from the args and call GetMethod with the array of types rather than GetMethods. The issue arises when the Arg class is used. In this case, I get null in the args so I cannot find the types. The only solution I can think of is adding a Types[] that matches the types of the args passed to the Protected() method . Any other idea?

If the issue is not clear, consider the following example

    public class Order
    {
        private User user;

        public Order() { }

        public bool Add(User aUser)
        {
            if (CheckValidUser(aUser))
            {
                //Do Stuff
                this.user = aUser;
                return true;
            }
            else
            {
                return false;
            }
        }

        protected virtual bool CheckValidUser(User aUser)
        {
            return aUser.IsValidUser();
        }

        protected virtual bool CheckValidUser(string id)
        {
            //check id
            return true;
        }

    }

        [TestMethod]
        public void Add_InvalidUser_ReturnsFalse()
        {
            Order aOrder = Substitute.For<Order>();
            aOrder.Protected("CheckValidUser", Arg.Any<User>()).Returns(false);

            bool result = aOrder.Add(new User());

            Assert.IsFalse(result);
        }

So as Protected() receives an array of args with 1 null object, I cannot find the correct method, which in this case corresponds to protected virtual bool CheckValidUser(User aUser)

[dtchepak]

You could try peeking at the queued argument specs (you would need to implement peekAll rather than dequeueAll.)

NSub has some logic for matching argument specs up to arguments, but it is a very messy part of the code base so I'm not sure if it is possible to reuse that code. (I've wanted to re-write it for ages, but the current implementation works so I decided to leave it alone.)

[falcu]

The extension method to fake protected virtual methods seems to be working fine. I did several tests.
However, it would be really cool if you can add this feature to the API. I know it makes the API a little bit dirty but it is really useful to fake protected methods when testing legacy code as I said in the first comment. I'm recommending this isolation framework for all of the apps my team develop, but the inability of facking protected method makes some people reluctant.

Please consider including this as a new feature

[dtchepak]

@falcu: thanks for the feedback. I'll keep this feature in mind but I'm really reluctant to add it. NSub already has a number of "error-prone" features (e.g. inability to reliably detect non-virtual calls and raise an error) and I'm not keen on introducing another one. And it isn't a lot of code for people to include it directly if they really want it.

[falcu]

Hey I have some dependency among my tests when using the peekAll you suggested. It seems that the SubstitutionContext.Current is not clean when some tests start.
If I include the following TestCleanup, they all pass, but I want to avoid having the clean up method on every test class. I would like to handle the clean internally

        [TestCleanup]
        public void CleanUp()
        {
            SubstitutionContext.Current.DequeueAllArgumentSpecifications();
        }

Is there anything I can do in my Protected() extension method to ensure that SubstitutionContext.Current is clean? Some sort of event ?

Thanks!!

[dtchepak]

So DequeueAllArgumentSpecifications() is still returning arg specs? This should not happen -- whenever NSub processes a call it dequeues all the arg specs. So this may indicate that you are calling a non-virtual protected method in one of your tests. I would recommend trying to locate that spot rather than trying to cover it up with a cleaning step.

At the end of your Protected() method you could try checking there are no outstanding arg specs (using DequeueAll), and if there are raising an exception to warn that the call did not seem to go through NSubstitute's routing correctly.

[falcu]

You were absolutely right. I'm testing my extension method and I'm asserting that an exception is thrown when I attemp to fake a method that is non protected or non virtual. As I'm testing the method, I do need to DequeueAllArgumentSpecifications()at the end of tests that are asserting this exception.
Thanks!!

One more thing, can you think of a better way to PeekAllArgumentSpecifications:

option 1

  private static IEnumerable<IArgumentSpecification> PeekAllArgumentSpecifications(this ISubstitutionContext substitutionContext)
        {
            object allArgumentSpecificationsParent = new PrivateObject(substitutionContext).GetField("_argumentSpecifications");
            IEnumerable<IArgumentSpecification> allArgumentSpecifications =
                (IEnumerable<IArgumentSpecification>)
                    new PrivateObject(allArgumentSpecificationsParent).GetProperty("Value");

            return allArgumentSpecifications;
        }

option 2

        private static IEnumerable<IArgumentSpecification> PeekAllArgumentSpecifications(this ISubstitutionContext substitutionContext)
        {
            IEnumerable<IArgumentSpecification> allArgumentSpecifications =
                substitutionContext.DequeueAllArgumentSpecifications();

            foreach (IArgumentSpecification argumentSpecification in allArgumentSpecifications)
            {
                substitutionContext.EnqueueArgumentSpecification(argumentSpecification);
            }

            return allArgumentSpecifications;
        }

[falcu]

And yes I will do both when identifying an invalid method, DequeueAllArgumentSpecifications() and throw the exception. By doing so, I will avoid having a second test, which fakes correctly a protected virtual method, to fail.

[dtchepak]

I think the best way to implement peek is to add it to SubstituteContext. :)
I'm a bit worried that both option 1 and 2 may cause problems with anything multi-threaded. (running tests in parallel, or testing multi-threaded code)

[falcu]

haha but that's not an option right? Or you plan to add it :)?

[dtchepak]

Sure it's an option! :)
Do you have time to add it and send a pull request through?

[dtchepak]

Closing as part of a general cleanup of blocked issues. Please re-open if more information comes to light which will let us proceed with this.

[CreepyGnome]

I don't understand why it has to be hacked around when the docs state:

NSubstitute can only work with virtual members of the class that are overridable
Then defined overridable as
By overridable we mean public virtual, protected virtual, protected internal virtual, or internal virtual with InternalsVisibleTo attribute applied.

This implies that NSubstitute can handle mock protected virtual members. Yet while those statements are in a few places in their doc's there are no docs that actually show how to mock anything but public virtual members.

It seems that Moq handles this without issues, and I am not sure why any Mocking/Substitution framework would exclude the ability to mock any virtual member of all visibilities that can be used with the virtual keyword.

So if NSubstitute doesn't support these they need to make it very clear in all the doc pages where it is applicable information. If however, they do support it they need to add docs to show how to mock other visibility for virtual members.

In many cases, when this occurs I just want to check if a protected method was called to ensure code is working as expected don't' really need to mock the method itself. I have to use Moq in these situations even though I prefer NSubstitute.

[dtchepak]

Thanks for pointing this out @CreepyGnome. I agree and have raised #599 to fix the documentation.

when this occurs I just want to check if a protected method was called to ensure code is working as expected don't' really need to mock the method itself.

Could you provide a good example of a case like this? (maybe in a new issue as a feature request?) It should be possible to implement if we can get a good use case and come up with a reasonable syntax .

[CreepyGnome]

@dtchepak What do you mean a good example case? Just try to mock or check if a virtual protected or internal method was called you cannot.

If you want to feature requests written for how to implement it, feel free to write one that starts with how Moq does it and then figure out a way to make it easier and better than that. You know like how everything else about NSubstitute is easier and better than Moq with this one exception currently to my knowledge. :) I mean Moq requires you to pass the string name of the method which sucks if you could figure out a way to provide a type-safe way to call the method, maybe behind Configure.Protected and Configure.Internal or if that is expensive to do for all types, then only do that if during the Substitute.For<T>call require an optional setting to be provided to say to protected and or internal and then do it only then. This is not hopefully a common occurrence for all tests but when you need it you need it and the easier and type-safest way it can be done would be amazing.

Currently, whenever I need to check if a virtual protected or internal method on someone else library was called I have to use Moq which is annoying and it confuses some developers who ask why are you use NSubstitute and Moq. Then I have to explain why and they are like "oh, okay that sucks" and we move on. So would love not to have to mix Moq and NSubstitute and I am really trying hard not to go back to Moq as I've been using NSubstitute for a while now and I really love it.

Just this one li

[dtchepak]

Hi @CreepyGnome ,

What do you mean a good example case? Just try to mock or check if a virtual protected or internal method was called you cannot.

I mean an example of when you would want to check a protected/internal method, based (loosely) on a real-life case. I can make up one up based on a guess, but sometimes having a realistic, motivating example helps us get a better design, or raises alternative features that can address the underlying need from another direction.

Currently, whenever I need to check if a virtual protected or internal method on someone else library was called I have to use Moq which is annoying and it confuses some developers who ask why are you use NSubstitute and Moq.

I've started spiking this out. If you are happy with hacking something in the meantime, you can write your own extension method to invoke the member you're after by reflection as per the above comments, then use it where ever you'd normally call a method on a substitute:

// DISCLAIMER: incomplete code. Use at your own risk!
public static object Protected(this object substitute, string memberName, params object[] args)
{
    var method = substitute
        .GetType()
        .GetMethod(memberName, BindingFlags.Instance | BindingFlags.NonPublic);

    if (!method.IsVirtual) {
        throw new Exception("Must be a virtual member");
    }
    return method.Invoke(substitute, args);
}

To upgrade this from the hacky implementation we need:

• Overload resolution
• Error detection (not a substitute, descriptive non-virtual exception, descriptive exception trying to call generics, attempt to call public member(?) etc.)
• Handling for non-method members
• Look at putting sub into configuration mode (will this work with Received?)
• Lots of testing

Example use:

sub.Received().Protected("MyMethod", arg0, arg1);
sub.Protected("MyMethod", arg0, arg1).Returns(42);

If you'd like to help with this feature I can push up the spike and we can divvy up the work. I still need to run this by @zvirja and @alexandrnikitin to make sure they're happy with it before I can attempt to get this in though.

[CreepyGnome]

@dtchepak If you are going to copy Moq then this shouldn't need much effort on the "how to do" this. If you want to make it better I gave you some ideas to see if they are possible. That if they are would make it better and easier than Moq. I suggest if you copy moq or other implementation you limit them to be hidden behind the Configure and Received methods as we would only want to configure them not to actually call them by accident. So your example I would suggest it be sub.Configure.Protected(..).Returns(..)

I would hope people working on and contributing to a framework like this can see real-world use cases for this as their understanding of Unit Tests and their purposes would be better than the average developer.

So take ASP.NET core and their HttpMessageHandler and its DelegateHandler that you typically extend that requires you to override the protected virtual SendAsync method. This is not an easy thing to test because of this, so they created the HttpMessageInvoker class to help with this which is great. However if you you have to provide it a sub/mock inner handler and to truly know if you are doing things correctly you need to know at least if you call this method, and if you have logic than can call this method with different data then you want to know what is passed as well to help ensure your code you are testing performed as expected. You can read all the Microsoft .NET Core docs on the handler and its invoker if you are not familiar with them to get more details about this pattern.

I am sorry, however, I don't have time to contribute more than requesting that this is a critically missing feature and that, at its worse, it should be at least like Moq and if possible making it type-safe would be preferred and not passing strings around if possible.

I also don't want to use a hack in my tests, I want my tests to be as clean as possible and as maintainable as possible. That hack is incomplete and could be confusing to a developer who follows after me and definitely would be if they end up being a junior or less experienced mid-level. So I'd rather mix and match Moq and NSubstitute when this happens to keep it clean as the next developers can easily look at the docs for those frameworks to see what I am doing, or may already know one or both of them.

[dtchepak]

Thanks for the feedback @CreepyGnome.

If you want to make it better I gave you some ideas to see if they are possible. That if they are would make it better and easier than Moq. I suggest if you copy moq or other implementation you limit them to be hidden behind the Configure and Received methods as we would only want to configure them not to actually call them by accident.

I don't think the strongly typed member references to protected members is possible. The compiler hides these from outside callers by design. We could probably check the strings and possibly argument types at compile time with NSubstitute.Analyzers though. 🤔 (cc: @tpodolak ) Moq 4.8 lets people declare an interface for protected members to help with intellisense, although this still ends up being stringly typed with respect to the mocked class (does solve the problem invoking generics though).

I don't think we can limit Protected to just Configure or Received either, as we'd lose NSubstitute's API by having to wrap the substituted object (in which case it would be better to use Moq, as it already does this very successfully).

I also don't want to use a hack in my tests, I want my tests to be as clean as possible and as maintainable as possible. That hack is incomplete and could be confusing to a developer who follows after me and definitely would be if they end up being a junior or less experienced mid-level.

I think it's important to accept that accessing protected members from external code is always going to be a bit of a hack, because they are designed specifically to prevent this sort of access. Moq uses the same reflection-based approach as discussed here. If you are concerned about this (which is quite understandable) then it may be worth considering alternate designs that are more inherently testable.

So take ASP.NET core and their HttpMessageHandler and its DelegateHandler that you typically extend that requires you to override the protected virtual SendAsync method.

This sounds like a good example. From a quick search it sounds similar to what is described in this post?

I am sorry, however, I don't have time to contribute more

No worries, thanks for taking the time to provide your thoughts on this.

[tpodolak]

The compiler hides these from outside callers by design. We could probably check the strings and possibly argument types at compile time with NSubstitute.Analyzers though. thinking (cc: @tpodolak)

@dtchepak sorry for delayed response. Yes that would be possible with analyzers

[zvirja]

@dtchepak It was a really great explanation from your side - nice work!

@CreepyGnome I share David's reluctance to add this to the NSubstitute library. I see that sometimes testing goes wild due to external code which is out of control, but normally we test our own code and good designing principles should be a rescue. In spirit of TDD one should consider tests as first client of production code, so that latter should be designed appropriately. If it's difficult to test code, than it should be re-designed to make that simpler.

Mocking of protected members sounds questionable, while is still understandable in certain cases. For me it's very rare case to be streamlined and supported by the NSubstitute. If there is a demand on a market to have this feature, it's a very nice reason to create a library in NSubstitute.Community namespace (see example), which will provide this feature. If we find that this library in overwhelmingly popular or has strong limitations due to not having access to NSubstitute internal core, we could then decide how to proceed further.

For now I can see that this feature request is voted, and so it is not demanded broadly.

[CreepyGnome]

Well sometimes you are thrown into a legacy code base with no tests and you have to do your best to ensure you are testing the new things you do, but have no control over the interface you are given.

I don't disagree about Mocking protected members and I am not mocking them, I just want to verify they where called. That is all, I have never and never plan to mock them. But I do need to ensure I can verify they where called. So more of a Spy than a Mock.

However, I had to switch to Moq for this project and remove NSubstitute because this was a blocking issue for us.

So the demand I would say for this is feature parity with Moq. That when the rubber meets the road and you are in a situation out of your control and you need to be able to do one or two tests that will do something with a protected member then you just need to do it.

This is not about arguing about or over best practices. This is about the real world and existing old code that you cannot refactor old existing poorly designed bases classes and just need to work in So in those crappy situations when that occurs I have to switch to Moq. Otherwise, I can use NSubstitute pretty much for all the other projects I tend to work on.

Also please don't forget there will always be more ugly legacy code than any other type and it sucks, but its reality. If we want to help bring those codebases into the light we need tools that can help. Part of that is being able to build tests around it before you should even attempt to refactor it, and even then you may only be able to go so far and it may be slow going. But building tests that can verify existing functionality as best as possible to help make it safer to refactor is important. If you would like NSubstibute to be helpful in that then you have to do things that allow for doing things ugly and not-part-of best practices. You can hide them a little bit but not too much to make it so they are not abused too easily or used accidentally, but they need to be there. As the framework that is used during this phase will probably be the framework used going forward when they can start using best practices. As this is the stuff we have trained the legacy team how to use and would be too much to retrain them to another one after all this effort.

So if you don't want to help teams refactor their legacy codebases then fine, Moq can fill in there and probably stay thereafter. You just have to decide if you want to be the framework that only works with and forces you to follow your version of Best Practices or if you want to be the framework that can work in some of the most common situations in some of the largest codes bases out there and help bring them to best practices, but first allowing them to just test what they have in place without changes, and then they can start working from there towards best practices.

[zvirja]

Thanks for the wide explanation. Again, sounds like a good idea to create a community package for that. Will do that if I have free time. If not - feel free to do that yourself ;-)

[dtchepak]

@arijit1begins

How is Protected member of sub?

Protected in this case is an extension method (declared with this object target parameter).

[arijit1begins]

Thanks . I realised later :)

[bronumski]

@dtchepak and @zvirja - I came here for the same reason, to see if NSubstitute supported the ability to mock protected members the same way that you can with Moq.

Whilst it may not support the feature out of the box your work around is a perfect solution which means that we don't have to support two different frameworks. I am quite happy using an extension method in our code base to leverage something that doesn't come out of the box and it doesn't fill me with horror and force me to run two mocking frameworks side by side. I'm still unhappy that "format" is not an extension method for string but I am not running python code inside my .net apps to get that feature :) .

Again appreciate the effort you and the team put into providing a free open source library that provides great value to the TDD community. 👏