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

Does NSubstitute Support Strict Mode or going to? #219

Closed
rafidka opened this Issue Jan 15, 2016 · 8 comments

Comments

Projects
None yet
3 participants
@rafidka
Copy link

rafidka commented Jan 15, 2016

@dtchepak I saw your answer here:

https://groups.google.com/forum/#!topic/nsubstitute/AdGTfszewtA

But it is a bit old and I wasn't sure whether you were going to implement it or not. But then I found your comment on this issue:

#211 (comment)

and it seems as if you want to support strict mode, no?

So just creating this issue to formalize this issue, and to know whether NSubstitute is going to support this or not, as searching on Google didn't show (at least for me) any clear answer to this.

Thanks

@dtchepak

This comment has been minimized.

Copy link
Member

dtchepak commented Jan 16, 2016

We never ended up making a decision on this. I take it you would like to use this feature? @grzesiek-galezowski has the feature working so maybe you can try that out? I'd love to get some feedback - if you both find that works for your cases then that would make a nice case for getting it into NSubstitute.

@rafidka

This comment has been minimized.

Copy link

rafidka commented Jan 16, 2016

By never ended up making a decision I understand you had different opinions on it? If that is the case, why not make it configurable like FakeItEasy:

FakeItEasy - Strict Fake

And yes, I would like to have such a feature. Actually, I really like NSubstitute and prefer it over the other mocking frameworks, however, it is necessary in my application (at least in some cases) to ensure that the developer writing unit tests set all his/her expectations rather than depending on some default values which might hide some issues. In other words, if some call is being made, I want the developer to be aware of that so he/she could then decide what to set it to.

I saw the feature working above, but the problem with verifyNoMoreInteractions-like solution is that it enforces the user to add extra code for each unit test. Additionally, in some cases, I don't necessarily want to verify that no other interaction happened, I just want to be aware of them to fill some values for them. Just like how you write a unit test and make it fail, and then start fixing it. Finally, when you are writing tests for existing code base, you might have more than one mock, so you will start adding more verifyNoMoreInteractions-like methods at the end of your unit test.

@rafidka

This comment has been minimized.

Copy link

rafidka commented Jan 16, 2016

What I hoped I could do is to have a generic substitution for all values followed by special ones. The generic one throws an exception, and the specific one returns the value I care about. I tried to do this:

      var ext = Substitute.For<IStringExt>();
      ext.ToLower(Arg.Any<string>()).Throws<Exception>();
      ext.ToLower("A").Returns("a");
      ext.ToLower("B").Returns("b");
      Console.WriteLine("Output of ToLower(A) is: " + ext.ToLower("A"));
      Console.WriteLine("Output of ToLower(B) is: " + ext.ToLower("B"));

This way I can implement strict mode by having a generic substitution for any value which is not overridden.

@grzesiek-galezowski

This comment has been minimized.

Copy link

grzesiek-galezowski commented Jan 16, 2016

Hi, it's an interesting discussion, so allow me to chime in about the verifyNoMoreInteractions() style. I don't want to gain anything by commenting, just share my opinion. While verifyNoMoreInteractions() is not perfect, it's still interesting why it may be more appealing that full "strict mocks".

About the redundant code - the way I see strict mocks, there are very few ways to use them without awkward pieces of code. Let's take the typical ways of using strict mocks - an expectation based API (which I think is best suited for strict mocks). A structure of a test using expectations is like this:

  1. Create mocks
  2. Set allowed and expected calls on them (these calls will not throw exception)
  3. Interact with the SUT (on illegal call, an exception will be thrown)
  4. If we are here, then none of the illegal calls were made.
  5. Verify that all expected calls were made (this is usually a single line of code, as the expectations are already specified earlier).

The reason for point 5 is that while we know at that point that none of the unexpected calls were made, we still don't know whether all the expected calls were made. So here we still need to do an assertion whether all the expected calls were received.

So there is still the last step - step 5. Some frameworks do some things to exclude this last step doing different tricks, e.g. JMock in Java uses JUnit runners and @RunWith annotation to automatically achieve the step 5 (see https://github.com/jmock-developers/jmock-library/blob/jmock2/jmock-example/src/main/org/jmock/example/gettingstarted/GettingStartedJUnit4.java).

Anyway, using NSubstitute this way would probably require to introduce some methods for setting expectations.

When dealing with verification-based API and having verifyNoMoreInteractions(), the structure usually looks like this:

  1. Create mocks
  2. Optionally set return values for some calls
  3. Interact with the SUT
  4. Verify that all expected calls were made
  5. Verify that unexpected calls were not made

So in each case, there is still some additional code. Note that for FakeItEasy, this issue of additional code still exists: FakeItEasy/FakeItEasy#198 (comment) .

Another point - specifying that no interactions happened on several mocks can be simplified to a single line with helper method, e.g.:

Received.NothingOn(mock1, mock2, mock3);

It would be difficult, I think, to say "specify no interactions happened on any of the mocks expect for the calls that I verified in the test" - this would most probably require some sort of "mock repository" (as in some early mock libraries) to hold all the mocks created for a particular test to be able to assert on them without specifying them explicitly. Some time ago, I tried to create a similar functionality for JustMock to try what it takes, and it led me to creating such "repository" - you can view the rough code at: https://github.com/grzesiek-galezowski/TrainingExamples/blob/master/StrictSubstitute/StrictSubstituteSpecification/StrictSubstituteSpecification.cs (there is a class called Context in this file).

@dtchepak

This comment has been minimized.

Copy link
Member

dtchepak commented Jan 19, 2016

Hi @rafidka,
Thanks for the feedback on the verifyNoMoreInteractions behaviour.

We can't really support a conventional strict mock due to our choice of syntax. For example:

sub.ToLower("A")   // <-- this executes first
   .Returns("a");  // <-- this stubs a result for the previous call

I think we can hack around the current implementation to get the behaviour you want. From your ToLower example:

var ext = Substitute.For<IStringExt>();
Func<string> defaultReturn = () => "";
ext.ToLower(Arg.Any<string>()).Returns(x => defaultReturn());
ext.ToLower("A").Returns("a");
ext.ToLower("B").Returns("b");
defaultReturn = () => { throw new Exception(); };
Console.WriteLine("Output of ToLower(A) is: " + ext.ToLower("A"));
Console.WriteLine("Output of ToLower(B) is: " + ext.ToLower("B"));

You might be able to wrap this up into a nice(r) helper if you use this pattern a lot:

var ext = Substitute.For<IStringExt>();
using (var returner = Helper.ThrowAfterUse(() => "")) {
    ext.ToLower(Arg.Any<string>()).Returns(x => returner.Return());
    ext.ToLower("A").Returns("a");
    ext.ToLower("B").Returns("b");
} // helper's Dispose changes `Return()` to throw an exception
Console.WriteLine("Output of ToLower(A) is: " + ext.ToLower("A"));
Console.WriteLine("Output of ToLower(B) is: " + ext.ToLower("B"));

Or use a builder for the return value itself, something like:

var ext = Substitute.For<IStringExt>();
ext.ToLower(Arg.Any<string>()).Returns(x => Helper.For(x).If("A", "a").ElseIf("B","b").ElseThrow());

If you write another Returns extension method you could make this terser and probably more readable.

@grzesiek-galezowski

This comment has been minimized.

Copy link

grzesiek-galezowski commented Jan 21, 2016

@dtchepak unfortunately, the same rule for overwriting does not seem to work with When().Do() which seems to rule out using this approach to void methods.

For example, the following test fails:

var sub = Substitute.For<ITested>();

Action a = () => { };

sub.When(m => m.VoidMethod(Arg.Any<int>(), Arg.Any<int>(),Arg.Any<string>()))
        .Do(c => a());
sub.When(m => m.VoidMethod(1,2,"")).Do(c => { });

a = () => { throw new Exception();};

sub.VoidMethod(1,2,"");
@dtchepak

This comment has been minimized.

Copy link
Member

dtchepak commented Jan 24, 2016

@grzesiek-galezowski Yes, When..Do calls don't override each other, they are cumulative. So in this case you'd need to make sure the set of calls matched by each When..Do don't overlap at all.

@rafidka

This comment has been minimized.

Copy link

rafidka commented Feb 9, 2016

Thanks @dtchepak , @grzesiek-galezowski for your feedback, and sorry for the late comment. I understand then the answer to my question is that NSubstitute doesn't support strict mode and it will not support it, but we can work around it for non-void methods as explained in this comment. Thanks. Closing the issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment