Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Received.InOrder executes return override on mocks and fails test #357

Open
trinith opened this issue Feb 13, 2018 · 5 comments
Open

Received.InOrder executes return override on mocks and fails test #357

trinith opened this issue Feb 13, 2018 · 5 comments
Labels
bug Reported problem with NSubstitute behaviour help wanted Core team needs help with this! If you've got some time, please volunteer to take a look.

Comments

@trinith
Copy link

trinith commented Feb 13, 2018

Hey :)

So lets say I have a factory that creates objects and I'm mocking that to make sure that it returns a mocked object with specific properties. In my [TestInitialize], I might do something like...

ISomeFactory mockFactory = Substitute.For<ISomeFactory>();
mockFactory.CreateThing().Returns(
  x =>
  {
    IThing thing = Substitute.For<IThing>();
    thing.SomeBool.Returns(true);
    return true;
  }

... Now lets say I have a method that I want to test on some other object that utilizes ISomeFactory to create things, and I want to make sure it makes the appropriate calls. I might write a test that contains the following....

WorkingObject sut = new WorkingObject(mockFactory);

sut.DoStuff(); // Results in a call to mockFactory.CreateThing

mockFactory.Received(1).CreateThing();

This works fine and the test passes; however, if I instead write the test as follows...

WorkingObject sut = new WorkingObject(mockFactory);

sut.DoStuff(); // Results in a call to mockFactory.CreateThing

Received.InOrder(
  x =>
  {
    mockFactory.CreateThing();
  }
);

... the test will fail with the following error...

Message: Test method ArbitraryPixel.CodeLogic.Common_Tests.Layers.CodeInputLayer_Tests.ConstructorShouldCreateBackgroundBorderEntity threw exception:
NSubstitute.Exceptions.CouldNotSetReturnDueToMissingInfoAboutLastCallException: Could not find information about the last call to return from.

Make sure you called Returns() after calling your substitute (for example: mySub.SomeMethod().Returns(value)),
and that you are not configuring other substitutes within Returns() (for example, avoid this: mySub.SomeMethod().Returns(ConfigOtherSub())).

If you substituted for a class rather than an interface, check that the call to your substitute was on a virtual/abstract member.
Return values cannot be configured for non-virtual/non-abstract members.

Correct use:
mySub.SomeMethod().Returns(returnValue);

Potentially problematic use:
mySub.SomeMethod().Returns(ConfigOtherSub());
Instead try:
var returnValue = ConfigOtherSub();
mySub.SomeMethod().Returns(returnValue);

The error text seems to suggest that what I'm doing on the factory returns method isn't advisable, but it seems quite reasonable to me and it works fine when we use the method that involves mock.Received().Method(). Additionally, if I put a breakpoint in the code for Returns, it is hit when using Received.InOrder(...), but not when using mock.Recevied().Method().

I dunno, hopefully that makes sense :)

@dtchepak
Copy link
Member

Full repro:

    using Xunit;
    using NSubstitute;
    using System.Collections.Generic;
    using System.Linq;

    public interface IThing {
        bool SomeBool { get; set; }
    }
    public interface ISomeFactory {
        IThing CreateThing();
    }
    public class WorkingObject {
        private ISomeFactory Factory { get; }

        public WorkingObject(ISomeFactory factory) {
            Factory = factory;
        }

        public void DoStuff() {
            var thing = Factory.CreateThing();
        }
    }

    public class UnitTest1 {
        [Fact]
        public void DoStuff() {
            var factory = Substitute.For<ISomeFactory>();
            factory.CreateThing().Returns(x => {
                var thing = Substitute.For<IThing>();
                thing.SomeBool.Returns(true);
                return thing;
            });

            var sut = new WorkingObject(factory);
            sut.DoStuff();

            // Passes:
            factory.Received().CreateThing();

            // Fails:
            Received.InOrder(() => {
                factory.CreateThing();
            });
        }
    }

@dtchepak
Copy link
Member

Hi @trinith,

Thanks for raising this. This seems related to the nested configuration for CreateThing() (where thing.SomeBool.Returns(...) is called within CreateThing().Returns(...)).

One workaround is to remove that nesting:

       [Fact]
        public void DoStuff() {
            var thing = Substitute.For<IThing>();
            thing.SomeBool.Returns(true);
            var factory = Substitute.For<ISomeFactory>();
            factory.CreateThing().Returns(thing);

            var sut = new WorkingObject(factory);
            sut.DoStuff();

            // Passes:
            factory.Received().CreateThing();

            // Fails:
            Received.InOrder(() => {
                factory.CreateThing();
            });
        }

That may not be possible for your specific case, but it works for the case above so thought I should mention it.

@dtchepak dtchepak added the bug Reported problem with NSubstitute behaviour label Feb 14, 2018
@trinith
Copy link
Author

trinith commented Feb 14, 2018

Hey, thanks for the response, and the confirmation!

The workaround can certainly work in a lot of cases. In other ones, Received.InOrder wasn't as critical as I thought at the time and was able to change to using a single .Received().Method check. I'll make it work ;)

@dtchepak dtchepak added the help wanted Core team needs help with this! If you've got some time, please volunteer to take a look. label Feb 17, 2018
@zvirja
Copy link
Contributor

zvirja commented Feb 28, 2018

@dtchepak Is this somehow related to #26? Looks like a duplicate.

@alexandrnikitin
Copy link
Member

@zvirja yes, it is. It seems we need to have a closer look at Roslyn analyzers. We can catch most of NSub syntax abuses.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Reported problem with NSubstitute behaviour help wanted Core team needs help with this! If you've got some time, please volunteer to take a look.
Projects
None yet
Development

No branches or pull requests

4 participants