Overriding a .Returns value results in original handler being called

[Niall]

I've hit a problem where a test class is setting up a generic handler for a method, where a return value is constructed based on the arguments to the method. In a specifc test, I want to override this and return a different canned value.

The problem is that the Func the original setup is using actually gets called when I'm trying to setup the second handler. See the boiled down example below.

Put a breakpoint on the "return 5" line, and you will see that on the line where we setup the .Returns(6), the original delegate is executed as if DoIt has actually been called. Is there something I'm doing wrong, or is this a bug?

        [TestMethod]

        public void Foo()

        {

            var foo = Substitute.For<IFoo>();

            foo.DoIt(Arg.Any<string>())

                .Returns(ci =>

            {

                return 5;

            });

            foo.DoIt(Arg.Any<string>()).Returns(6);

        }

   public interface IFoo

   {

       int DoIt(string blah);

   }

Thanks

[David Tchepak]

Hi Niall,

You are correct, the original delegate will always get called. 

The first `Returns` tells NSubstitute to return 5 (via a delegate) every time `foo.DoIt()` is called, so when we get to `foo.DoIt(...).Returns(6)` it runs like this:

foo.DoIt(null) // <- NSub records it got a call to DoIt, & sees it needs to return 5 (by running the delegate).

   .Returns(6) // <- Tells NSub to update the return value for the last call.

               //    NSub removes the last DoIt call from the list of received calls,

               //    and records that it needs to return 6 next time DoIt is called

This means that subsequent calls will now return 6, but I don't think there's a way or stopping the original delegate from running without hacking the delegate itself. (e.g. `Returns(x => active ? 5 : 0)`).

Regards,

David

--
You received this message because you are subscribed to the Google Groups "NSubstitute" group.
To unsubscribe from this group and stop receiving emails from it, send an email to nsubstitute...@googlegroups.com.
To post to this group, send email to nsubs...@googlegroups.com.
Visit this group at http://groups.google.com/group/nsubstitute.
For more options, visit https://groups.google.com/d/optout.

[Niall]

Hi David,

Yeah I guess I hadn't thought too much about the mechanics of it. To me, the foo.DoIt(Arg.Any<string>()).Returns(something) is a "setup" phase, as seen in other mocking frameworks. Intuitively to me there's no actual method call happening there but obviously something on the mock object must be being called during that call chain.

So it was a surprise to me that when calling the method with another Arg.Any<> parameter, the original handler is executed. In my head, this is a second setup phase, trying to replace the return delegate with a specific value, similar to this page, but where a delegate handler had originally been used: http://nsubstitute.github.io/help/replacing-return-values/.

Is there some other way for me to remove the original handler delegate off that method and then setup a new Returns() for the method? This is one thing that feels difficult with NSubstitute.

[David Tchepak]

Yeah NSubstitute does a fair amount of evil to get its syntax, the downside being some cases become painful (hopefully the majority of happy cases make up for the rare painful case -- it depends a lot on coding styles and how individuals use their mocks I think).

There is no way to "un-setup" a particular method. We could potentially add a nuke-all-configured-members option (similar to ClearReceivedCalls[1]), but that might be too indiscriminate for your case?

If you let me know a bit of the specifics about what you're trying to test I'm happy to try to help.

Otherwise I think your choices are:

* hack the delegate to only run if a `finishedSetup` flag or equivalent is true (yuck)

* tweak design to remove need for side-effects in `Returns` delegate

* hand-roll the mock, or try Moq, FakeItEasy, or RhinoMocks. That will give you more control over setup vs. act phases of your test.

Sorry it's causing you trouble.

Regards,

David

[1]: https://github.com/nsubstitute/NSubstitute/blob/ee77b59c6355f9159409f0d46f4e0370dc5c466b/Source/NSubstitute/SubstituteExtensions.cs#L182

[Niall]

What I've got is a test case where the mocked method is a service call to save a booking. The service call returns some values based on the booking, so the mock of the service call uses the Returns overload which takes a delegate to craft a response that is realistic. Here is a slightly simplified version of the original setup code:

            _tradeService.Trade(Arg.Any<NewOrderRequest>(), Arg.Any<IFxQuote>(), Arg.Any<DateTime>()).Returns(call =>

            {

                var order = call.Arg<NewOrderRequest>();

                var quote = call.Arg<IFxQuote>();

                return ExecutionReport.Success(order, quote);

            });

This is shared by the 10-15 test methods in the class and everything is fine.

But in this one test case, I want to test the scenario where the TradeService has a failure. The problem is that when I set a new .Returns value in this specific test, it calls the above delegate, and the .Success method ends up throwing because the parameters are null.

Obviously I could move this test to a new fixture, or pull the above out of the common setup, but either way it results in a more complex test, and I'm a big fan of tests being simpler to read than the code they're testing (NSubstitute helps here).

So yeah, I could get around it with a class variable flag to stop it doing that. I ended up going a route where I mocked the service throwing an exception instead of returning a failure result - it achieves the same ends for the purposes of my test, but it forces the test to catch the exception.

I haven't tried doing this with a .When style syntax - would that possibly avoid this issue? Could I use a When...Do to return a different value that overrides the original .Returns?

[David Tchepak]

Thanks a lot for the example. I don't think When..Do will work here; we can't set the return value from there and it will still get called all the time too.

I realise this may not fit in with the larger context, but maybe it would help to separate the details about how `_tradeServer.Trade()` works and just use the contract behind the interface? Something like, "when `Trade()` is called it will return a Success report or a Failure report". (Specific details like "the success report will include the order and quote passed as arguments" can possibly be deferred to the tests for the trade service, but the calling code may not need to know that detail)

The test would then look more like this:

    

    _tradeService.Trade(null, null, default(DateTime)).ReturnsForAnyArgs(_successfulReport);

where `_successfulReport` is a pre-canned result from `ExecutionReport.Success()`, or a directly constructed object, or a mocked successful report. I guess this depends on how the calling code needs to use the report, but hopefully it won't need to know anything about how trade service works, only how to handle a successful report.  (If it does need to know this detail, can we use a real TradeService here instead of a mocked one? Maybe we can mock the boundary a bit further out instead?)

Because we've conveniently ignored the detail of how the arguments are linked to the output, we can now override this `Returns` in the other test without a problem.

Another approach could be a variation on the closure-cheat-hack-yuck suggestion:

    //Field in test fixture:
    Func<ICallInfo,ExecutionReport> _getReport;

    //Test setup:

    _getReport = call => ExecutionReport.Success(call.Arg<NewOrderRequest>, call.Arg<IFxQuote>);
    _tradeService.Trade(null, null, default(DateTime)).ReturnsForAnyArgs(call => _getReport(call));

    //Other test that overrides the result:

    _getReport = call => ExecutionReport.Failure();

    ...

Now we can reassign the `_getReport` delegate to change what `_tradeService.Trade()` returns (without having to use NSub to alter the `Returns`). Bit yuck I admit, but should work ok here.

Alternatively we could make the initial delegate code from your example handle the null case (more hacking around the NSub limitation). But I'm already scraping the bottom of the barrel so I'll stop here. :)

Hope some of this rambling helps. If not let me know and I'll try and come up with some other ideas tomorrow once I've slept on it. :)

Regards,

David

[Niall]

Sorry, got distacted for a couple of days.

Yeah the problem with returning a single canned ExecutionReport is that the report has to be consistent with the request for the TradeService code to work properly. So the values in the Report depend on the values that were in the original request - hence the use of the delegate to return a report based on the inputs.

The closure-cheat-hack-yuck solution hadn't occurred to me. It's kind of subtle and yet hacky at the same time. Might end up going that way.

Thanks for the time you've taken to look into this!

[Emil Jonsson]

Hi guys, I was just wondering.. What is the reason for a testing framework that calls the original of every method before we can mock it? Am I not understanding something or do we really always need to call the original method we want to mock?

I mean I would like to use this framework for our tests alot, but...

One big reason for mocking methods is so we can avoid calling them.. I think I must be missing something here?

MvH

Emil

[David Tchepak]

Hi Emil,

I'm not sure what you mean? When we substitute for an interface there is not really a concept of an "original" method, there are just the signatures that need to be implemented. Substitutes implement these in ways that can be used for assertions or to configure results, and also as substitutes for production code. Substituting for classes with virtual members work similarly: all methods are replaced with proxied implementations (the catch being that non-virtual members can not be replaced).

NSubstitute's syntax for setting up a call involves calling a proxied method on the substitute:

   sub.Calculate().Returns(42);

This choice differs from some of the other excellent libraries like Moq (FakeItEasy is also well worth checking out) where the syntax is more like this:

  mock.SetUp(x => x.Calculate()).Returns(42);

The former is a bit shorter, while the latter permits more checks in the `SetUp` which provides more reliable/deterministic behaviour. The overall aims are the same, so I'd suggest picking whichever makes most sense to you. 

Hope this helps. :)

Regards,

David

To unsubscribe from this group and stop receiving emails from it, send an email to nsubstitute+unsubscribe@googlegroups.com.

To post to this group, send email to nsubs...@googlegroups.com.

Visit this group at https://groups.google.com/group/nsubstitute.

[Emil Jonsson]

Oh, so if I use it on a virtual method (or interface of course) the original is then not called during setup?

It's for database code, we want to avoid calling a database in our unit tests, pretty standard scenario I would think :)

Thanks for the answer!

Emil

[David Tchepak]

Oh, so if I use it on a virtual method (or interface of course) the original is then not called during setup?

Correct.

Thanks for the answer!

No problem, glad I could help. 😁

Regards,

David

To unsubscribe from this group and stop receiving emails from it, send an email to nsubstitute+unsubscribe@googlegroups.com.