NSubstitute icon indicating copy to clipboard operation
NSubstitute copied to clipboard

Ability to forward all calls on a given interface to a given instance

Open jnm2 opened this issue 6 years ago • 15 comments

I'd like to be able to do something like DelegateCalls in this example:

var items = new List<Foo> { /* ... */ };

var list = Substitute.For<IMultiSelectList, IFocusTrackingList, IList>();
list.DelegateCalls<IList>(instance: items);

testUiComponent.DataSource = list;
// ...

Is there existing functionality that does this or can I make this a proposal?

As far as I know the only alternative is this kind of thing, repeated for all 15 IList members:

 ((IList)list)[Arg.Any<int>()].Returns(i => items[i.Arg<int>()]);

jnm2 avatar Oct 08 '17 01:10 jnm2

Hi @jnm2, There is no existing functionality to do this. I can see it could be handy for cases where multiple interfaces are implemented.

Is this something you have any time to work on implementing?

dtchepak avatar Oct 09 '17 01:10 dtchepak

That's a possibility, though a slim one right now till I get some existing backlogs down. I'll come back and chase this up if no one beats me to it.

What do you think from an API design perspective? Should it be an object extension method or just a static method if you think it's a specialized use case?

jnm2 avatar Oct 09 '17 04:10 jnm2

I'd like to avoid putting more extension methods on object where ever possible. I've been having "opt-in" extensions here which I think makes it a bit more palatable. We can always start with a static method and then expose an extension later if required.

Ideally I think I'd like a Substitute.With(/* configuration */).For<T>() that lets us specify defaults, delegation etc, but I am not sure there would be enough demand for that to make it worth implementing.

dtchepak avatar Oct 09 '17 05:10 dtchepak

Ooh, I like the fluent idea. But that's a lot more design.

jnm2 avatar Oct 09 '17 13:10 jnm2

@jnm2 For delegation feature itself, would you expect .Received() to still work for delegated calls? And overriding with .Returns?

For example, delegating IList calls to a real list, can we check if .Add calls are made? Or can we say sub.Count.Returns(50) when the delegated-to list only has 1 item?

The way I'm imagining it at the moment is it would be just like manually substituting all the calls on the interface to call/return from the target object, but it could lead to some confusing scenarios like the above case of an almost-real-list that has an invalid Count value.

I guess another options would be to try to only expose delegation as part of sub creation and then not allow any NSubstitute interaction with delegated type after that ("this is a substitute for IFoo, but it also works as a real list whose behaviour we don't substitute for at all").

(If this is something required only rarely, we could also avoid NSubstitute changes and just drop a method into your project that uses reflection to set up the delegation, automated what we would have manually done calling When..Do/Returns.)

dtchepak avatar Oct 09 '17 23:10 dtchepak

For delegation feature itself, would you expect .Received() to still work for delegated calls? And overriding with .Returns?

I would expect everything you describe, including the confusing scenarios if you set up contradictory return values. There might be a use case for overriding just one method out of a dozen.

jnm2 avatar Oct 09 '17 23:10 jnm2

I think the feature is too specific, complex and confusing to put in NSubstitute. I prefer to keep NSubstitute simple and intuitive to use. I think the better way to solve such cases is to implement an extension/helper method inside your test library (if you need this often). Something like this for IList<>:

[Test]
public void DelegateCallsExample()
{
            
    var list = new List<object>();
    var sub = Substitute.For<IMultiSelectList<object>>();
    DelegateCalls(sub, list);

    var item = new object();
    sub.Add(item);
    Assert.AreEqual(item, sub[0]);
}
public interface IMultiSelectList<T> : IList<T>{}
public static void DelegateCalls<T>(IList<T> sub, List<T> list)
{
//...
    sub[Arg.Any<int>()].Returns(x => list[x.Arg<int>()]);
    sub.When(x => x.Add(Arg.Any<T>())).Do(x => list.Add(x.Arg<T>()));
//...
}

alexandrnikitin avatar Oct 10 '17 16:10 alexandrnikitin

This seems like just the sort of helper which would be a perfect match for NSubstitute, both in usage and in philosophy. It's less confusing and less complex than faithfully and accurately translating it to the fifteen manual forwarding calls.

As to being to narrow a use case, I'm happy to close and see if others come along if you can't imagine it being discoverable and useful as proposed.

jnm2 avatar Oct 10 '17 17:10 jnm2

By no means I want to discourage you. If you have a real-world use case, that you think is worth to cover, please, share it. What I want to avoid: confusing and nonintuitive logic; complex logic and scenarios (which is always a warning for me).

alexandrnikitin avatar Oct 10 '17 19:10 alexandrnikitin

Sure, I would agree with that. The initial example I provided is straight out of and motivated by real-world code. It's not the only time I've been in this scenario, but it's the only one I remember off the top of my head. I'll keep my eyes open and report back if I run into it in more real-world scenarios.

jnm2 avatar Oct 10 '17 19:10 jnm2

I've been trying to understand it but failed. Could you share more details about the problem please. Does the problem appear because of inheritance? Will partial substitutes (for few types) help you here? Do you inject IList into the type you try to substitute and it acts as a decorator?

alexandrnikitin avatar Oct 10 '17 20:10 alexandrnikitin

To answer your questions:

  • There's no inheritance.
  • How would I accomplish this with partial substitutes?
  • No, I'm not trying to inject IList into a concrete type. I'm asking NSubstitute to generate a single in-memory implementation of the three interfaces, which I need. I am not testing with my advanced list classes which implement all three interfaces; if I was, this would be an integration test rather than a unit test. I'm testing the UI control in isolation from my advanced list classes.

The sole problem is that I don't want to maintain all fifteen of these by hand, not even counting property setters. I do want to delegate all fifteen to any old implementation of IList such as List<>– I don't really care which implementation, List<> is perfect– while still being able to use .Returns and .Do on the methods in the other two interfaces.

    sub[Arg.Any<int>()].Returns(x => list[x.Arg<int>()]);
    sub.When(x => x.Add(Arg.Any<T>())).Do(x => list.Add(x.Arg<T>()));

Again, it's working property and that's not the problem. The problem is that it's pure boilerplate, time-consuming to write and verify, where subtle errors are easy to overlook. This is where APIs usually shine.

jnm2 avatar Oct 10 '17 21:10 jnm2

Oh, by partial substitutes, do you mean I would write the substitute implementation myself, like this?

var items = new List<Foo> { /* ... */ };
var list = Substitute.ForPartsOf<SubstituteImplementation>(items);
// Use NSubstitute to fluently set up IMultiSelectList and IFocusTrackingList members
// Use NSubstitute to fluently raise IBindingList.ListChanged when I need it

private abstract class SubstituteImplementation : IMultiSelectList, IFocusTrackingList, IBindingList
{
    private readonly IList listDelegatee;

    protected SubstituteImplementation(IList listDelegatee)
    {
        this.listDelegatee = listDelegatee;
    }

    // I need these for consistency for the UI control to be able to treat this like a normal IList
    public IEnumerator GetEnumerator() => listDelegatee.GetEnumerator();

    public void CopyTo(Array array, int index) => listDelegatee.CopyTo(array, index);

    public int Count => listDelegatee.Count;

    public object SyncRoot => listDelegatee.SyncRoot;

    public bool IsSynchronized => listDelegatee.IsSynchronized;

    public int Add(object value) => listDelegatee.Add(value);

    public bool Contains(object value) => listDelegatee.Contains(value);

    public void Clear() => listDelegatee.Clear();

    public int IndexOf(object value) => listDelegatee.IndexOf(value);

    public void Insert(int index, object value) => listDelegatee.Insert(index, value);

    public void Remove(object value) => listDelegatee.Remove(value);

    public void RemoveAt(int index) => listDelegatee.RemoveAt(index);

    public object this[int index]
    {
        get => listDelegatee[index];
        set => listDelegatee[index] = value;
    }

    public bool IsReadOnly => listDelegatee.IsReadOnly;

    public bool IsFixedSize => listDelegatee.IsFixedSize;

    // I need these for testing the UI control's reactions to IMultiSelectList and IFocusTrackingList
    public abstract event EventHandler SelectionChanged;

    public abstract IEnumerable SelectedItems { get; set; }

    public abstract event EventHandler FocusedRowChanged;

    public abstract FocusedRowType FocusedRowType { get; set; }

    public abstract object FocusedItem { get; set; }

    // As well as this event from IBindingList
    public abstract event ListChangedEventHandler ListChanged;

    // The rest of these I do not need to worry about
    public abstract object AddNew();

    public abstract void AddIndex(PropertyDescriptor property);

    public abstract void ApplySort(PropertyDescriptor property, ListSortDirection direction);

    public abstract int Find(PropertyDescriptor property, object key);

    public abstract void RemoveIndex(PropertyDescriptor property);

    public abstract void RemoveSort();

    public abstract bool AllowNew { get; }

    public abstract bool AllowEdit { get; }

    public abstract bool AllowRemove { get; }

    public abstract bool SupportsChangeNotification { get; }

    public abstract bool SupportsSearching { get; }

    public abstract bool SupportsSorting { get; }

    public abstract bool IsSorted { get; }

    public abstract PropertyDescriptor SortProperty { get; }

    public abstract ListSortDirection SortDirection { get; }
}

The only reason I use NSubstitute is because it allows me to not have to write such pure boilerplate substitute implementations myself, but rather configure an autogenerated implementation via fluent API.

jnm2 avatar Oct 10 '17 21:10 jnm2

I think it would be handy to have the ability to forward to another object. Maybe it could replace partial subs? (or partial subs could be rewired to use this mechanism?) I'm thinking we could forward all calls to a known object (i.e. call base), then selectively override members.

Definitely agree with Alexandr we want to keep NSub lean and as simple as possible, but if this potentially makes the partial sub thing simpler then that would be a big win.

dtchepak avatar Oct 11 '17 01:10 dtchepak

hmm not sure if it helps, but im currently using this work-around on 3.1.0


    public static class NSubstituteHelpers
    {
        public class RedirectAllHandler : ICallHandler
        {
            public RedirectAllHandler(object instance)
            {
                Instance = instance;
            }

            public object Instance { get; }

            public RouteAction Handle(ICall call)
            {
                return RouteAction.Return(call.GetMethodInfo().Invoke(Instance, call.GetArguments()));
            }
        }
 

        public static T RedirectCalls<T, TRedirect>(this T substitute, TRedirect obj)
            where TRedirect : T
            where T : class
        {
   
            SubstitutionContext.Current.GetCallRouterFor(substitute)
                .RegisterCustomCallHandlerFactory(factory: state =>
                {
                    return new RedirectAllHandler(obj);
                } );
 
            return substitute;

        }
    }

then you just use it like this


            var myInstance = new Instance();
            var myMock = Substitute.For<IMyInstance>();

            myMock.RedirectCalls(myInstance);

            // to override a specific method, must not call base when overriding.

            myMock.When(x => x.MyMethod(Arg.Any<int>(), Arg.Any<int>(), Arg.Any<string>())).DoNotCallBase();
            myMock.MyMethod(Arg.Any<int>(), Arg.Any<int>(), Arg.Any<string>()).Returns("OVERRULED");

avilv avatar Oct 29 '18 10:10 avilv