-
-
Notifications
You must be signed in to change notification settings - Fork 811
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
[Feature request] Loose mode for commands and strict for queries. #1056
Comments
You should be able to do that by implementimg a custom
Same. You should be able to produce some default return value for |
Doesn't it contradict to the unconditional throwing for any returned value? |
@stakx It looks like DefaultValueProvider does not know for which method it is called. I'd be nice to tell in the exception message which call needs a setup. Yet I can extract it from stack frame, but it's not so performant. |
@voroninp, that's right, while In your case, you might fare well enough by producing some sensible (I didn't mean to provide you with the perfect solution, just take my above post as a starting point for your own experiments.) |
Code, if anyone needs it later sealed class AlwaysThrowingDefaultValueProvider : DefaultValueProvider
{
protected override object GetDefaultValue(Type type, Mock mock)
{
// these two are equivalents of voids for async operations.
// Formally we need to check the type is awaitable and result of operation is void.
// But it's to much effort.
if (type == typeof(Task))
{
return Task.CompletedTask;
}
if (type == typeof(ValueTask))
{
return new ValueTask();
}
var mockedType = mock.Object.GetType();
var callLocation = new StackTrace()
.GetFrames()
.Select(_ => new { Method = _.GetMethod(), File = _.GetFileName(), Line = _.GetFileLineNumber() })
.First(_ => _.Method.DeclaringType.IsAssignableFrom(mockedType));
throw new QueryNotSetupException($"Call to {callLocation.Method.Name} must have setup.");
}
}
sealed class QueryNotSetupException : Exception
{
public QueryNotSetupException(string message) : base(message) { }
}
/// <summary>
/// Queries are methods without side effects but returning values. Commands are void methods with side effects.
/// Thus Queries should be strictly mocked, whereas Commands should be loosely mocked, that is be just verifiable without
/// requiring explicit setup.<br/><br/>
///
/// See: https://blog.ploeh.dk/2013/10/23/mocks-for-commands-stubs-for-queries/
/// </summary>
public static class StrictQueries
{
/// <summary>
/// Creates mock with loosely mocked Commands and strictly mocked Queries</c>.
/// </summary>
public static void Mock<T>(out Mock<T> mock) where T : class =>
mock = new Mock<T>(MockBehavior.Loose)
{
DefaultValueProvider = new AlwaysThrowingDefaultValueProvider()
};
/// <summary>
/// Creates mock with loosely mocked Commands and strictly mocked Queries</c>.
/// </summary>
public static void Mock<T>(out Mock<T> mock, Fixture fixture) where T : class
{
Mock(out mock);
var obj = mock.Object;
fixture.Register(() => obj);
}
} |
@stakx TBH, I'd like native Moq support for this mode. Though it works, as you suggested, failure messages are not that friendly compared to the ones generated by Moq itself. |
I understand. However, this requirement is too specific to fit well into the core library. Two things:
|
Do you mean loose commands, but strict queries? Or the part with Task and ValueTask? |
Both. Concepts like queries and commands belong in a layer above Moq, they're too specific for a general mocking library. And while Moq has special support for tasks, your requirements are again too specific when combined to warrant a solution in the library. |
I agree it's a bit more abstract level, yet the tool could promote better practices so to say. ;-) I see people using loose mocks not to bother with mocking voids - that is commands. But then it makes tests be a hell when one gets |
That's what
No. Moq isn't anyone's nanny. It has a specific purpose: providing you with mock objects, mostly for use in tests. That's it. Promoting or even enforcing specific architectural patterns simply is out of scope. |
Is it possible to add one more mocking mode, so void methods do not require setup and are just verified, yet methods which return value require setup?
Also for the methods which break CQS principle like
Task<int> SaveChangesAsync()
it would be nice to explicitly tell, that returned value does not matter:So setup behaves as if it were a lose mock, but developer clearly sees that returned value does not matter.
The text was updated successfully, but these errors were encountered: