Add WithParameterName #1466
Add WithParameterName #1466
Conversation
| /// </param> | ||
| public static ExceptionAssertions<TException> WithParameterName<TException>( | ||
| this ExceptionAssertions<TException> parent, | ||
| string paramName, |
There was a problem hiding this comment.
I know this has already been merged, but I'd suggest adding argument validation (!= null) for paramName here to improve the stacktrace in case people misuse the method.
There was a problem hiding this comment.
I didn't think about that case, but ArgumentException allows a null ParamName.
Maybe I'm misunderstanding you, can you give an example on "misuse"?
| params object[] becauseArgs) | ||
| where TException : ArgumentException | ||
| { | ||
| parent.Which.ParamName.Should().Be(paramName, because, becauseArgs); |
There was a problem hiding this comment.
Question: won't the final error message here be a bit cryptic to an outside consumer? Wouldn't it be better to create a custom validation instead of relying on the standard Should().Be in this case?
There was a problem hiding this comment.
Good point, I'll change that.
| /// <param name="becauseArgs"> | ||
| /// Zero or more objects to format using the placeholders in <paramref name="because"/>. | ||
| /// </param> | ||
| public static async Task<ExceptionAssertions<TException>> WithParameterName<TException>( |
There was a problem hiding this comment.
Shouldn't this one have the Async suffix?
There was a problem hiding this comment.
The current approach in this class is that they don't have Async suffix.
Add
WithParameterNameextension to assert on theParamNameproperty onArgumentException.This fixes #1453