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
Add NotBeApproximately
version accepting nullable subject and expected
#939
Add NotBeApproximately
version accepting nullable subject and expected
#939
Conversation
Hmm, we don't seem to have a |
And what about merged PR #934? :-) |
Sigh. I wasn't paying attention. |
e855ab7
to
4895f8c
Compare
NotBeApproximately
version accepting nullable subject and expectedNotBeApproximately
version accepting nullable subject and expected
I have just implemented |
I think I disagree with the To write up the three cases in prose form.
The proposed implementation has the output:
I'm more inclined to say that the output should be
|
I'm starting to think we should not be fixing this scenario at all. Why would you be able to compare to |
@dennisdoomen By "in general", do you then mean both The use case for e.g. var actual = new { B = (double?)1 };
var expected = new { B = (double?)2 };
actual.Should().BeEquivalentTo(expected,
opt => opt.Using<double?>(ctx => ctx.Subject.Should().BeApproximately(ctx.Expectation, 1))
.WhenTypeIs<double?>()); |
Alright. In that case, I agree with the last proposal @jnyrup did. |
First of all, if that is not too inconvenient, use "throws/does not throw" terminology instead of "true/false". Somehow, every time I read it, I am confused and have to translate between terms ;-). I recognize your point of view. Let's break it up in two sub-topics for even more clarity.
|
Now I see what you refer to by symmetrical, that is symmetrical to the To recap: I'm not sure how we want to handle this.
|
So there are three possible directions to take:
Any opinions @dennisdoomen |
I think that @jnyrup 's original proposal still holds, both for
|
Likewise, I think that each approach has its pros and cons but I like the one you guys described the most.
is a breaking change. |
I accept ;-) |
I tracked down the PR, where the author apparently carelessly copy/pasted |
Or you have been investing enough in this project that you forgot how much ;-) |
@jnyrup I would blame the person accepting and merging the PR. 😉 |
4895f8c
to
a802df3
Compare
Behaviour adjusted, ready to merge. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@krajek sorry for the late review, I must have missed out on a notification on your latest commit.
Followup of #934.
Just as previously, I start the PR with the float version. Upon approval, I will copy the logic for
decimal?
and `double?' overloads.There are two interesting cases:
subject
has value, butunexpectedValue
does not. Because reversed case throws, for symmetry, I suggest throwing and this is how I implemented it.subject
andunexpectedValue
are null. In that case, I decided not to throw, as semantically there is no way to determine the difference between them. However, I am not sure about this one, please review.