-
Notifications
You must be signed in to change notification settings - Fork 264
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
Improve output for expected argument for custom argument matchers on non-match or make it customizable #796
Comments
It might be adding this to the proxy classes solves this: (looking at code of
As an alternative, a new interface IDescribeMatches (?) could be added to test on and determine if an argument matcher provides a custom implementation to describe the expected argument. (instead of testing if object.ToString() is overridden and calling that for this purpose) |
Thanks for raising this @rbeurskens . This definitely needs to be improved.
class CustomMatcher : IArgumentMatcher, IDescribeNonMatches, IArgumentMatcher<int>
{
public string DescribeFor(object argument) => "failed";
public bool IsSatisfiedBy(object argument) => false;
public bool IsSatisfiedBy(int argument) => false;
public override string ToString() => "Custom match";
}
[Test]
public void CustomArgMatch() {
//ArgumentMatcher.Enqueue<int>(new CustomMatcher());
var spec = new ArgumentSpecification(typeof(int), new CustomMatcher());
SubstitutionContext.Current.ThreadContext.EnqueueArgumentSpecification(spec);
_something.Received().Add(23, 0);
}
/*
Failed CustomArgMatch [52 ms]
Error Message:
NSubstitute.Exceptions.ReceivedCallsException : Expected to receive a call matching:
Add(23, Custom match)
Actually received no matching calls.
*/ I think we can just delegate to the matcher for these proxies? private class GenericToNonGenericMatcherProxy<T> : IArgumentMatcher
{
protected readonly IArgumentMatcher<T> _matcher;
public GenericToNonGenericMatcherProxy(IArgumentMatcher<T> matcher)
{
_matcher = matcher;
}
public bool IsSatisfiedBy(object? argument) => _matcher.IsSatisfiedBy((T?)argument!);
public override string ToString() => _matcher?.ToString() ?? ""; // TODO: fix nullability stuff here
}
private class GenericToNonGenericMatcherProxyWithDescribe<T> : GenericToNonGenericMatcherProxy<T>, IDescribeNonMatches
{
public GenericToNonGenericMatcherProxyWithDescribe(IArgumentMatcher<T> matcher) : base(matcher)
{
if (matcher is not IDescribeNonMatches) throw new SubstituteInternalException("Should implement IDescribeNonMatches type.");
}
public string DescribeFor(object? argument) => ((IDescribeNonMatches)_matcher).DescribeFor(argument);
public override string ToString() => _matcher?.ToString() ?? ""; // TODO: fix nullability stuff here
} |
This would not work as like there would be no proxy if the custom matcher does not override The problem with using public class ArgumentFormatter : IArgumentFormatter
{
internal static IArgumentFormatter Default { get; } = new ArgumentFormatter();
public string Format(object? argument, bool highlight)
{
var formatted = Format(argument);
return highlight ? "*" + formatted + "*" : formatted;
}
private string Format(object? arg)
{
return arg switch
{
null => "<null>",
string str => $"\"{str}\"",
{ } obj when HasDefaultToString(obj) => arg.GetType().GetNonMangledTypeName(), // <===
_ => arg.ToString() ?? string.Empty
};
static bool HasDefaultToString(object obj)
=> obj.GetType().GetMethod(nameof(ToString), Type.EmptyTypes)!.DeclaringType == typeof(object); // <=== if obj is a proxy, we can't tell if it's overridden on the matcher itself
}
} Maybe this would be an option: public interface IFormatable
{
string Format(Func<object? ,string> format);
// Alternative if exposing the proxied object through this property is no problem:
// object? Arg {get;}
}
public class ArgumentFormatter : IArgumentFormatter
{
private string Format(object? arg)
{
return arg switch
{
IFormattable f => f.Format(Format),
// IFormattable f => Format(f.Arg),
null => "<null>",
string str => $"\"{str}\"",
{ } obj when HasDefaultToString(obj) => arg.GetType().GetNonMangledTypeName(), // <===
_ => arg.ToString() ?? string.Empty
};
// (...)
}
}
private class GenericToNonGenericMatcherProxy<T> : IArgumentMatcher, IFormatable
{
public string Format(Func<object? ,string> format) => format(_matcher);
// public object? Arg => _matcher;
// (...)
} It would still require custom |
On a second thought and looking a bit further, I see how the code ends up running public class ArgumentSpecification : IArgumentSpecification
{
public string FormatArgument(object? argument)
{
var isSatisfiedByArg = IsSatisfiedBy(argument);
return _matcher is IArgumentFormatter matcherFormatter
? matcherFormatter.Format(argument, highlight: !isSatisfiedByArg)
: ArgumentFormatter.Default.Format(argument, highlight: !isSatisfiedByArg);
}
// (...)
} This means that we can just simply have the proxies implement private class GenericToNonGenericMatcherProxy<T> : IArgumentMatcher, IArgumentFormatter
{
string Format(object? arg, bool highlight) =>
_matcher is IArgumentFormatter matcherFormatter
? matcherFormatter.Format(argument, highlight)
: ArgumentFormatter.Default.Format(argument, highlight);
// (...)
} (Since |
One problem is we have two formatting concepts here: formatting matching/non-matching calls, and formatting specifications. The latter is the one causing a problem here. It currently works off Alternatively we could add an |
So if I understand correctly, the 'problematic' output comes from the public abstract class ArgumentMatcher<T> : IArgumentMatcher<T>, IArgumentMatcher
{
/// <inheritdoc cref="IArgumentMatcher.ToString" />
public abstract override string ToString();
public abstract bool IsSatisfiedBy(T? argument);
bool IArgumentMatcher.IsSatisfiedBy(object? argument) =>IsSatisfiedBy((T?)argument);
} By the way, it would be handy for custom argument matchers to have access to .. and maybe also change code to only use proxies if they are needed: /// <summary>
/// Enqueues a matcher for the method argument in current position and returns the value which should be
/// passed back to the method you invoke.
/// </summary>
public static ref T? Enqueue<T>(IArgumentMatcher<T> argumentMatcher)
{
var matcher = switch argumentMatcher
{
null => throw new ArgumentNullException(nameof(argumentMatcher)),
IArgumentMatcher m => m,
IDescribeNonMatches => new ArgumentMatcher.GenericToNonGenericMatcherProxyWithDescribe<T>(argumentMatcher),
_ => new ArgumentMatcher.GenericToNonGenericMatcherProxy<T>(argumentMatcher)
}
return ref ArgumentMatcher.EnqueueArgSpecification<T>(new ArgumentSpecification(typeof (T), matcher));
} |
- Add IDescribeSpecification to allow custom arg matchers to provide custom output for "expected to receive" entries. - Fallback to ToString when IDescribeSpecification not implemented. - Update code comment docs accordingly. Closes nsubstitute#796.
I've created #806 to try to address this. Please take a look at let me know if you feel it is sufficient. 🙇 |
- Add IDescribeSpecification to allow custom arg matchers to provide custom output for "expected to receive" entries. - Fallback to ToString when IDescribeSpecification not implemented. - Update code comment docs accordingly. Closes nsubstitute#796.
Looks good to me on a first glance. Best of both worlds: An interface with a clear purpose for it without breaking code that uses the existing mechanism. |
- Add IDescribeSpecification to allow custom arg matchers to provide custom output for "expected to receive" entries. - Fallback to ToString when IDescribeSpecification not implemented. - Update code comment docs accordingly. Closes nsubstitute#796.
- Add IDescribeSpecification to allow custom arg matchers to provide custom output for "expected to receive" entries. - Fallback to ToString when IDescribeSpecification not implemented. - Update code comment docs accordingly. Relates to nsubstitute#796.
- Add IDescribeSpecification to allow custom arg matchers to provide custom output for "expected to receive" entries. - Fallback to ToString when IDescribeSpecification not implemented. - Update code comment docs accordingly. Relates to nsubstitute#796.
When implementing a custom argument matcher, it would be nice that the output of what is expected would be the same of the default matchers (or is customizable):
Example:
sut.Received().MyMethod(Arg.Is<MyType>(p => p.Property == 42));
output:
With a custom argument matcher: (note the name of an internal proxy is used to describe the expected match)
.. or (of it implements
IDescribeNonMatches
):Even if my custom matcher overrides
.ToString()
(whichArg.Is(Expression<Predicate<T>>)
) seems to use to get the string of what is expected, the output does not change.The text was updated successfully, but these errors were encountered: