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

ReturnsForAnyArgs and WhenForAnyArgs and generic arguments #152

Closed
alexandrnikitin opened this issue May 14, 2014 · 10 comments
Closed

ReturnsForAnyArgs and WhenForAnyArgs and generic arguments #152

alexandrnikitin opened this issue May 14, 2014 · 10 comments
Labels
question Question on NSubstitute behaviour

Comments

@alexandrnikitin
Copy link
Member

This is more a question rather than a bug report. Inspired by a question on SO
Suppose we have an interface with a generic method:

public interface IInterfaceWithGenericMethod
{
    int GenericMethod<T>(T arg);
}

Then when we specify the sub to return a value for any argument then it fails on the call with another generic type.

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

const int expectedInt = 1;
sub.GenericMethod(Arg.Any<int>()).ReturnsForAnyArgs(expectedInt); // Specifying for int

Assert.AreEqual(expectedInt, sub.GenericMethod(0)); // Works for int
Assert.AreEqual(expectedInt, sub.GenericMethod("")); // Fails for string

The same is right for WhenForAnyArgs().
Imo this isn't obvious behavior. Thoughts?

alexandrnikitin added a commit to alexandrnikitin/NSubstitute that referenced this issue May 14, 2014
@dtchepak
Copy link
Member

I think whether it is obvious or not depends on how we think about generic methods. AFAICT when we use a generic method it will be compiled as a specific, non-generic instance of that generic method. I think we tend to approximate this to "calling a generic method" with a varying type parameter, in which case we'd conclude that we should be able to setup calls to sub.GenericMethod<T>(). Dropping the approximation leads us to the opposite conclusion: sub.GenericMethod<int>() must be configured separately to sub.GenericMethod<string>() because they are different methods. So I think the current behaviour makes sense.

It could still be useful to come up with a way off configuring all instances of GenericMethod<T>, but I think it will need a different syntax.

@alexandrnikitin
Copy link
Member Author

I think it's more a question of how we treat ReturnsForAnyArgs() method and "ForAnyArgs" suffix there. Is it more "Any" or more "Returns"? Does it really mean "Any"?

Your position is fine for me but for Returns() method with argument specifications. E.g. sub.GenericMethod(Arg.Any<int>()).Returns(expectedInt); Is fine to return expectedInt only for args of int type. There generic specification could have a different syntax e.g. sub.GenericMethod(Arg.Any<int>().OrAnyOtherType()).Returns(expectedInt);

But it's not so obvious for ReturnsForAnyArgs() which imho a special case. And meant to cover any args regardless of their specification, types and so on. I'm trying to think from end-user perspective here.

@dtchepak
Copy link
Member

I take your point. I've had this cause confusion for people before. I think a good example could be WhenForAnyArgs because it doesn't set a return type to constrain the generic.

sub.WhenForAnyArgs(x => x.GenericMethod(0)).Do(...) sets up GenericMethod<int>, but if we think of type arguments as regular arguments then it gets confusing. We could be trying to set up GenericMethod<T> for all T.

To me the former still makes sense provided we're thinking of generics accurately (i.e. "ForAnyArgs" means any arguments passed to the closed generic GenericMethod<int>). The latter could still be a useful thing to do, but NSub doesn't currently support it.

@alexfoxgill
Copy link

Came here via #185, this would be useful for a generic repository pattern. I think it would need its own syntax though; .ReturnsForAnyArgs() is strongly typed.

The syntax could look something like

var x = Substitute.For<IInterfaceWithGenericMethod>();
x.GenericMethod<object>().ReturnsForAnyGeneric(call => Activator.CreateInstance(call.GenericArgs[0]))

@dtchepak
Copy link
Member

Closed #185 so we can continue discussion here.

How frequently do you need something like this? In my tests I haven't run in to a case where I've had to stub numerous generic instances of calls with the same logic.

I think we could come up with a way of stubbing/getting callbacks for multiple generic instances, but I'm not sure it is worth the complexity (compared to, say, hand-rolling a fake IRepository).

@smirnfil
Copy link

I think this is a nice functionality to have.

I want this behaviour for my repo stubs - if someone somewhere asks for some IQueryable just give an empty one to him. I'll probably be happy with just #67(should look into it as it seems much easier to implement).

However, I understands the concerns about complexity. Faking doesn't looks as nice as mocking (one of repos that we use have 13 methods and I really care about 1 or 2 generic ones) but I could live with it.

@dtchepak
Copy link
Member

Would be great to have #67 sorted! :)

@julealgon
Copy link

Stumbled upon this while attempting to validate a call into Microsoft.Extensions.Logging.ILogger.

The logger interface takes a T state, but the "public" APIs (i.e. common extension methods) do not expose this directly.

For example, consumer code calls it like this:

logger.Critical("A critical problem has occurred!");

This is translated into a call to the interface's method, which has this signature:

public void Log<TState>(
            LogLevel logLevel,
            EventId eventId,
            TState state,
            Exception exception,
            Func<TState, Exception, string> formatter)

The string parameter that represents the message in the original call is not passed directly into this method, but instead is wrapped in an internal type and passed as state. Thus, it is actually impossible to perform an assertion on ILogger, since you cannot specify the internal type without getting a compilation error.

I actually solved this before by creating an abstract TestableLogger that exposes a non-generic version of the base interface method and then redirecting the implementation to it, and then using .Received on top of that new method. Obviously, this is extremely convoluted.

I'd really appreciate if NSubstitute provided a way to specify Any<AnyType>() that would check for any generic argument on a given parameter.

@dtchepak
Copy link
Member

dtchepak commented Dec 5, 2020

@julealgon Thanks for the good example. I've created #634 for this feature request.

@304NotModified
Copy link
Contributor

I assume the questions has been answered in this issue, if not, please let us know!

@304NotModified 304NotModified added the question Question on NSubstitute behaviour label Apr 30, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
question Question on NSubstitute behaviour
Projects
None yet
Development

No branches or pull requests

6 participants