From 90d93ec57ab56bd78524350bf5f9b38496997924 Mon Sep 17 00:00:00 2001 From: David Tchepak Date: Sun, 5 May 2024 14:17:12 +1000 Subject: [PATCH 1/2] Improve output for expected argument matchers - 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 #796. --- .../Core/Arguments/ArgumentMatcher.cs | 6 ++++ .../Core/Arguments/IArgumentMatcher.cs | 12 ++++--- src/NSubstitute/Core/CallSpecification.cs | 4 ++- src/NSubstitute/Core/IDescribeNonMatches.cs | 7 +++- .../Core/IDescribeSpecification.cs | 16 +++++++++ .../ArgumentMatching.cs | 36 ++++++++++++++++++- 6 files changed, 74 insertions(+), 7 deletions(-) create mode 100644 src/NSubstitute/Core/IDescribeSpecification.cs diff --git a/src/NSubstitute/Core/Arguments/ArgumentMatcher.cs b/src/NSubstitute/Core/Arguments/ArgumentMatcher.cs index 2695a2cf..0fea577d 100644 --- a/src/NSubstitute/Core/Arguments/ArgumentMatcher.cs +++ b/src/NSubstitute/Core/Arguments/ArgumentMatcher.cs @@ -38,6 +38,9 @@ private class GenericToNonGenericMatcherProxy(IArgumentMatcher matcher) : protected readonly IArgumentMatcher _matcher = matcher; public bool IsSatisfiedBy(object? argument) => _matcher.IsSatisfiedBy((T?)argument!); + + public override string ToString() => + (_matcher as IDescribeSpecification)?.DescribeSpecification() ?? _matcher.ToString() ?? ""; } private class GenericToNonGenericMatcherProxyWithDescribe : GenericToNonGenericMatcherProxy, IDescribeNonMatches @@ -48,6 +51,9 @@ public GenericToNonGenericMatcherProxyWithDescribe(IArgumentMatcher matcher) } public string DescribeFor(object? argument) => ((IDescribeNonMatches)_matcher).DescribeFor(argument); + + public override string ToString() => + (_matcher as IDescribeSpecification)?.DescribeSpecification() ?? _matcher.ToString() ?? ""; } private class DefaultValueContainer diff --git a/src/NSubstitute/Core/Arguments/IArgumentMatcher.cs b/src/NSubstitute/Core/Arguments/IArgumentMatcher.cs index b0000d89..e0a0f530 100644 --- a/src/NSubstitute/Core/Arguments/IArgumentMatcher.cs +++ b/src/NSubstitute/Core/Arguments/IArgumentMatcher.cs @@ -1,8 +1,10 @@ namespace NSubstitute.Core.Arguments; /// -/// Provides a specification for arguments for use with . -/// Can additionally implement to give descriptions when arguments do not match. +/// Provides a specification for arguments. +/// Can implement to give descriptions when arguments do not match. +/// Can implement to give descriptions of expected arguments (otherwise +/// `ToString()` will be used for descriptions). /// public interface IArgumentMatcher { @@ -14,8 +16,10 @@ public interface IArgumentMatcher } /// -/// Provides a specification for arguments for use with . -/// Can additionally implement to give descriptions when arguments do not match. +/// Provides a specification for arguments. +/// Can implement to give descriptions when arguments do not match. +/// Can implement to give descriptions of expected arguments (otherwise +/// `ToString()` will be used for descriptions). /// /// Matches arguments of type or compatible type. public interface IArgumentMatcher diff --git a/src/NSubstitute/Core/CallSpecification.cs b/src/NSubstitute/Core/CallSpecification.cs index dce5c7b9..d08b0232 100644 --- a/src/NSubstitute/Core/CallSpecification.cs +++ b/src/NSubstitute/Core/CallSpecification.cs @@ -121,7 +121,9 @@ public IEnumerable NonMatchingArguments(ICall call) public override string ToString() { - var argSpecsAsStrings = _argumentSpecifications.Select(x => x.ToString() ?? string.Empty).ToArray(); + var argSpecsAsStrings = _argumentSpecifications.Select(x => + (x as IDescribeSpecification)?.DescribeSpecification() ?? x.ToString() ?? string.Empty + ).ToArray(); return CallFormatter.Default.Format(GetMethodInfo(), argSpecsAsStrings); } diff --git a/src/NSubstitute/Core/IDescribeNonMatches.cs b/src/NSubstitute/Core/IDescribeNonMatches.cs index d8ba00aa..94814ce6 100644 --- a/src/NSubstitute/Core/IDescribeNonMatches.cs +++ b/src/NSubstitute/Core/IDescribeNonMatches.cs @@ -1,5 +1,10 @@ namespace NSubstitute.Core; +/// +/// A type that can describe how an argument does not match a required condition. +/// Use in conjunction with to provide information about +/// non-matches. +/// public interface IDescribeNonMatches { /// @@ -9,4 +14,4 @@ public interface IDescribeNonMatches /// /// Description of the non-match, or if no description can be provided. string DescribeFor(object? argument); -} \ No newline at end of file +} diff --git a/src/NSubstitute/Core/IDescribeSpecification.cs b/src/NSubstitute/Core/IDescribeSpecification.cs new file mode 100644 index 00000000..dfee9620 --- /dev/null +++ b/src/NSubstitute/Core/IDescribeSpecification.cs @@ -0,0 +1,16 @@ +namespace NSubstitute.Core; + +/// +/// A type that can describe the required conditions to meet a specification. +/// Use in conjunction with to provide information about +/// what it requires to match an argument. +/// +public interface IDescribeSpecification +{ + + /// + /// A concise description of the conditions required to match this specification. + /// + /// + string DescribeSpecification(); +} diff --git a/tests/NSubstitute.Acceptance.Specs/ArgumentMatching.cs b/tests/NSubstitute.Acceptance.Specs/ArgumentMatching.cs index 8b131553..055e8c14 100644 --- a/tests/NSubstitute.Acceptance.Specs/ArgumentMatching.cs +++ b/tests/NSubstitute.Acceptance.Specs/ArgumentMatching.cs @@ -838,6 +838,7 @@ public interface IMyService { void MyMethod(IMyArgument argument); } + public interface IMyArgument { } public class SampleClass { } public class MyStringArgument : IMyArgument { } @@ -845,4 +846,37 @@ public class MyOtherStringArgument : IMyArgument { } public class MySampleClassArgument : IMyArgument { } public class MyOtherSampleClassArgument : IMyArgument { } public class MySampleDerivedClassArgument : MySampleClassArgument { } -} \ No newline at end of file + + [Test] + public void Should_use_ToString_to_describe_custom_arg_matcher_without_DescribesSpec() + { + var ex = Assert.Throws(() => + { + _something.Received().Add(23, ArgumentMatcher.Enqueue(new CustomMatcher())); + }); + Assert.That(ex.Message, Contains.Substring("Add(23, Custom match)")); + } + + [Test] + public void Should_describe_spec_for_custom_arg_matcher_when_implemented() + { + var ex = Assert.Throws(() => + { + _something.Received().Add(23, ArgumentMatcher.Enqueue(new CustomDescribeSpecMatcher())); + }); + Assert.That(ex.Message, Contains.Substring("Add(23, DescribeSpec)")); + } + + class CustomMatcher : IArgumentMatcher, IDescribeNonMatches, IArgumentMatcher + { + public string DescribeFor(object argument) => "failed"; + public bool IsSatisfiedBy(object argument) => false; + public bool IsSatisfiedBy(int argument) => false; + public override string ToString() => "Custom match"; + } + + class CustomDescribeSpecMatcher : CustomMatcher, IDescribeSpecification + { + public string DescribeSpecification() => "DescribeSpec"; + } +} From 72005d02fc9c1440bacfeb4ef961edde0952329f Mon Sep 17 00:00:00 2001 From: David Tchepak Date: Sun, 5 May 2024 22:33:47 +1000 Subject: [PATCH 2/2] Apply review comments - use string.Empty for null value from IDescribeSpecification, rather than falling back to ToString(). This supports the contract that IDescribeSpecification will be used if implemented. Replacing null string.Empty with matches the documented behaviour of IDescribeNonMatches. - updated IDescribeSpecification code docs. - removed GenericToNonGenericMatcherProxyWithDescribe `ToString` as it can use the GenericToNonGenericMatcherProxy superclass implementation. - update ArgumentSpecification to also support IDescribeSpecification for its matcher. - Replace linq with Array.ConvertAll rather than requiring an extra ToArray conversion. --- .../Core/Arguments/ArgumentMatcher.cs | 7 +++---- .../Core/Arguments/ArgumentSpecification.cs | 5 ++++- src/NSubstitute/Core/CallSpecification.cs | 8 +++++--- src/NSubstitute/Core/IDescribeSpecification.cs | 6 +++--- .../ArgumentMatching.cs | 16 +++++++++++++--- 5 files changed, 28 insertions(+), 14 deletions(-) diff --git a/src/NSubstitute/Core/Arguments/ArgumentMatcher.cs b/src/NSubstitute/Core/Arguments/ArgumentMatcher.cs index 0fea577d..2a055fda 100644 --- a/src/NSubstitute/Core/Arguments/ArgumentMatcher.cs +++ b/src/NSubstitute/Core/Arguments/ArgumentMatcher.cs @@ -40,7 +40,9 @@ private class GenericToNonGenericMatcherProxy(IArgumentMatcher matcher) : public bool IsSatisfiedBy(object? argument) => _matcher.IsSatisfiedBy((T?)argument!); public override string ToString() => - (_matcher as IDescribeSpecification)?.DescribeSpecification() ?? _matcher.ToString() ?? ""; + _matcher is IDescribeSpecification describe + ? describe.DescribeSpecification() ?? string.Empty + : _matcher.ToString() ?? string.Empty; } private class GenericToNonGenericMatcherProxyWithDescribe : GenericToNonGenericMatcherProxy, IDescribeNonMatches @@ -51,9 +53,6 @@ public GenericToNonGenericMatcherProxyWithDescribe(IArgumentMatcher matcher) } public string DescribeFor(object? argument) => ((IDescribeNonMatches)_matcher).DescribeFor(argument); - - public override string ToString() => - (_matcher as IDescribeSpecification)?.DescribeSpecification() ?? _matcher.ToString() ?? ""; } private class DefaultValueContainer diff --git a/src/NSubstitute/Core/Arguments/ArgumentSpecification.cs b/src/NSubstitute/Core/Arguments/ArgumentSpecification.cs index 5dca300d..a4fc82ec 100644 --- a/src/NSubstitute/Core/Arguments/ArgumentSpecification.cs +++ b/src/NSubstitute/Core/Arguments/ArgumentSpecification.cs @@ -46,7 +46,10 @@ public string FormatArgument(object? argument) : ArgumentFormatter.Default.Format(argument, highlight: !isSatisfiedByArg); } - public override string ToString() => matcher.ToString() ?? string.Empty; + public override string ToString() => + matcher is IDescribeSpecification describe + ? describe.DescribeSpecification() + : matcher.ToString() ?? string.Empty; public IArgumentSpecification CreateCopyMatchingAnyArgOfType(Type requiredType) { diff --git a/src/NSubstitute/Core/CallSpecification.cs b/src/NSubstitute/Core/CallSpecification.cs index d08b0232..7b099a94 100644 --- a/src/NSubstitute/Core/CallSpecification.cs +++ b/src/NSubstitute/Core/CallSpecification.cs @@ -121,9 +121,11 @@ public IEnumerable NonMatchingArguments(ICall call) public override string ToString() { - var argSpecsAsStrings = _argumentSpecifications.Select(x => - (x as IDescribeSpecification)?.DescribeSpecification() ?? x.ToString() ?? string.Empty - ).ToArray(); + var argSpecsAsStrings = Array.ConvertAll(_argumentSpecifications, x => + x is IDescribeSpecification describe + ? describe.DescribeSpecification() ?? string.Empty + : x.ToString() ?? string.Empty + ); return CallFormatter.Default.Format(GetMethodInfo(), argSpecsAsStrings); } diff --git a/src/NSubstitute/Core/IDescribeSpecification.cs b/src/NSubstitute/Core/IDescribeSpecification.cs index dfee9620..b6d30765 100644 --- a/src/NSubstitute/Core/IDescribeSpecification.cs +++ b/src/NSubstitute/Core/IDescribeSpecification.cs @@ -7,10 +7,10 @@ namespace NSubstitute.Core; /// public interface IDescribeSpecification { - /// - /// A concise description of the conditions required to match this specification. + /// A concise description of the conditions required to match this specification, or + /// if a detailed description can not be provided. /// - /// + /// Description of the specification, or if no description can be provided. string DescribeSpecification(); } diff --git a/tests/NSubstitute.Acceptance.Specs/ArgumentMatching.cs b/tests/NSubstitute.Acceptance.Specs/ArgumentMatching.cs index 055e8c14..19e3c99c 100644 --- a/tests/NSubstitute.Acceptance.Specs/ArgumentMatching.cs +++ b/tests/NSubstitute.Acceptance.Specs/ArgumentMatching.cs @@ -862,11 +862,21 @@ public void Should_describe_spec_for_custom_arg_matcher_when_implemented() { var ex = Assert.Throws(() => { - _something.Received().Add(23, ArgumentMatcher.Enqueue(new CustomDescribeSpecMatcher())); + _something.Received().Add(23, ArgumentMatcher.Enqueue(new CustomDescribeSpecMatcher("DescribeSpec"))); }); Assert.That(ex.Message, Contains.Substring("Add(23, DescribeSpec)")); } + [Test] + public void Should_use_empty_string_for_null_describe_spec_for_custom_arg_matcher_when_implemented() + { + var ex = Assert.Throws(() => + { + _something.Received().Add(23, ArgumentMatcher.Enqueue(new CustomDescribeSpecMatcher(null))); + }); + Assert.That(ex.Message, Contains.Substring("Add(23, )")); + } + class CustomMatcher : IArgumentMatcher, IDescribeNonMatches, IArgumentMatcher { public string DescribeFor(object argument) => "failed"; @@ -875,8 +885,8 @@ class CustomMatcher : IArgumentMatcher, IDescribeNonMatches, IArgumentMatcher "Custom match"; } - class CustomDescribeSpecMatcher : CustomMatcher, IDescribeSpecification + class CustomDescribeSpecMatcher(string description) : CustomMatcher, IDescribeSpecification { - public string DescribeSpecification() => "DescribeSpec"; + public string DescribeSpecification() => description; } }