How to keep unit tests passing when adding an optional argument to an interface

[CrispyDrone]

Question

How can I ensure that verifying calls does not break when adding optional arguments to an interface?

Adding an optional argument to an interface is one way of adding additional behavior while being backwards compatible. However, I do not want to have to update dozens of unit tests that don't care about this additional behavior. Right now I'm seemingly forced to add an Arg.Any argument for all my existing verifications (Received), otherwise tests will fail due to a non-default argument being provided.

Is there any way to achieve this backwards compatible behavior in NSubstitute without breaking existing unit tests? Thanks!

[Test]
public void OptionalArgumentsBackwardsCompatible()
{
    var pleaser = Substitute.For<IWantToBeBackwardsCompatible>();
    var sut = new INeedSomeoneWhoCanProvideCompatibility(pleaser);

    sut.Beg();

    pleaser
        .Received()
        // the `true` here is important, so we cannot use `ReceivedWithAnyArgs`
        .Please(true);
}

public class INeedSomeoneWhoCanProvideCompatibility
{
    private readonly IWantToBeBackwardsCompatible _wantToBeBackwardsCompatible;

    public INeedSomeoneWhoCanProvideCompatibility(IWantToBeBackwardsCompatible wantToBeBackwardsCompatible)
    {
        _wantToBeBackwardsCompatible = wantToBeBackwardsCompatible;
    }

    public void Beg()
    {
        _wantToBeBackwardsCompatible.Please(true);
    }
}

public interface IWantToBeBackwardsCompatible
{
    void Please(bool offerBribe);
}

Add optional argument to the IWantToBeBackwardsCompatible interface. Now, Received will fail since it expects to receive null as it is marked as the default argument on the interface.

public class INeedSomeoneWhoCanProvideCompatibility
{
    private readonly IWantToBeBackwardsCompatible _wantToBeBackwardsCompatible;

    public INeedSomeoneWhoCanProvideCompatibility(IWantToBeBackwardsCompatible wantToBeBackwardsCompatible)
    {
        _wantToBeBackwardsCompatible = wantToBeBackwardsCompatible;
    }

    public void Beg()
    {
       // one usage of the interface will pass in false for the new argument. 
      // Other usages will not provide anything, or will provide true.
        _wantToBeBackwardsCompatible.Please(true, false);
    }
}

public interface IWantToBeBackwardsCompatible
{
   // add new nullable optional argument
    void Please(bool offerBribe, bool? kneel = null);
}

[dtchepak]

Hi @CrispyDrone , thanks for the good example.

If I understand correctly, I don't think there is a neat way of doing this with NSubstitute. Received is meant to check that the arguments match, so if they don't after the interface change then the tests will fail.

The only thing I can think of that may make updating the tests less arduous is to try an extension method:

    public static class CompatibilityExtensions
    {
        public static void ReceivedBribeCall(this IWantToBeBackwardsCompatible c, bool offerBribe) {
            c.Received().Please(offerBribe, Arg.Any<bool?>());
        }
    }

Depending on how your tests are formatted, you may be able to get away with Find & Replace .Received().Please( with .ReceivedBribeCall(. Not sure if you could use regex options or similar to handle whitespace differences (like newlines in your original example).

Sorry I don't have a better answer for you.

[CrispyDrone]

Hi @dtchepak

Thank you for your quick reply. No worries!

Do you think it could be a possibility to allow a substitute or a call to be configured such that any argument that matches one of its default values, would instead be replaced by Arg.Any?

I would imagine something like this:

var pleaser = Substitute.For<IWantToBeBackwardsCompatible>(Behavior.Loose);

Or something with Configure:

    pleaser
        .Configure(Behavior.Loose)
        .Received()
        .Please(true);

Or something at the global level:

Substitute.Behavior = Behavior.Loose;

Although, I can see that this is approaching a rather involved solution for a rare scenario. I would completely understand if you think this does not fit with the library 😄

[dtchepak]

@CrispyDrone From memory the information about default parameters disappears after compilation (the compiler inserts the constant at any call sites), so I don't think NSubstitute would be able to distinguish the default value from an explicitly passed value.

[CrispyDrone]

Normally this information is encoded in the metadata of the parameter definition. You can get the required information through reflection using the ParameterInfo.IsOptional and ParameterInfo.DefaultValue apis.

One thing that worries me is the note at the bottom of the page that compilers are not required to emit this information in the metadata. However, it would be a surprise to me if the C# compiler (and VB compiler) don't do this.

NSubstitute would not be able to distinguish whether the value, if the default, is provided by the compiler or the programmer. Any call verification with a non-default value should still be able to be identified by NSubstitute as such.

I've been exploring the code base the past few days, and have been meaning to contribute. I want to learn more about dynamic code generation, and metaprogramming in general. I would like to see if I could implement this. Even if you prefer not to integrate it into the library, it would still be useful learning for me 😃

[dtchepak]

Even if you prefer not to integrate it into the library, it would still be useful learning for me

At present I think this is a bit niche to include in NSubstitute. If you want to implement as an exercise I'm happy to try to help if you need info about any of the code in NSub. Alternatively if want to try to implement it as an extension (maybe in NSubstitute.Community.*?), and need any additional extensibility points in NSub in order to do so, let us know and we can try to accomodate that!

[CrispyDrone]

@dtchepak

Thanks for all the help!

I will see if I can create an NSubstitute.Community.* package.

In the meantime, do you want me to close this issue and open a new one if I need those extensibility points, or do we keep this one open for that purpose?

[dtchepak]

do you want me to close this issue and open a new one if I need those extensibility points, or do we keep this one open for that purpose?

Whatever works best for you. :)

[CrispyDrone]

Hey again @dtchepak

I temporarily forked NSubstitute and had a go at trying to implement it in the real library first. My thoughts were to first see if I could get it to work at all when I have full freedom. Good news, as it seems the change was relatively trivial as far as I can tell.

I think I will be able to implement the same behavior in a community package in the exact same way, but I think I would need a modification of the IParameterInfo interface (addition of DefaultValue), and perhaps something with MatchArgs (however I think I should be able to just add a static instance through an extension method in my own MatchArgsExtension class). If you want, you can have a look at how I implemented it over here.

I was also hoping you could give me some feedback regarding the API design. I currently have a method called ReceivedWithSomeArgs that will display the behavior of ignoring the arguments passed to optional parameters for which no explicit value, different from the default, was provided in the verification call.

I think this approach is better than the initial idea of being able to specify some sort of "loose" behavior in the mock. The main problems with that are the following in my opinion:

1. It would require a lot more changes throughout the code base
2. Would force people to always go look at how the substitutes are being created, as they will display different behavior depending on the configuration

Also, do you happen to know how I can include all NSubstitute tests in my own package? I could copy paste all of them, or perhaps use a git submodule?

[dtchepak]

Hi @CrispyDrone ! I'll try to get you feedback on this over the weekend if that's ok. From a quick glance, I think there is logic for determine default values for arg types already, but I'll try to confirm and get back to you.

By the way, a lot of the tests are obsolete. The AcceptanceTests are the only tests that are runnable at present (and these test the main functionality, the other tests were an experiment in driving out design.)

[dtchepak]

@CrispyDrone I've had a better look now. It looks pretty neat. I can probably add the DefaultValue if you need it (that's pretty minimal impact).

I'm not sure how do-able it is, but it might be possible to create your own custom route to do this without additional modifications to NSub (other than the DefaultValue one). You could duplicate the current Received route and match any args for optional args. @zvirja is flat out busy at the moment, but might be able to advise more on the best way to do this in conjunction with the minimal container implementation in NSub. If you give it a go and get stuck ping me and I'll try to help out.

[MNF]

@CrispyDrone did you have any progress with this? I’ve tried to open your link https://github.com/nsubstitute/NSubstitute/compare/master...CrispyDrone:feat/add-received-for-some-args, but it’s broken.