Add IsCloseTo check for DateTime and DateTimeOffset

tpierrain / NFluent

Smooth your .NET TDD experience with NFluent! NFluent is an ergonomic assertion library which aims to fluent your .NET TDD experience (based on simple Check.That() assertion statements). NFluent aims your tests to be fluent to write (with a super-duper-happy 'dot' auto-completion experience), fluent to read (i.e. as close as possible to plain English expression), but also fluent to troubleshoot, in a less-error-prone way comparing to the classical .NET test frameworks. NFluent is also directly inspired by the awesome Java FEST Fluent assertion/reflection library (http://fest.easytesting.org/)

Apache License 2.0

310 stars 53 forks source link

Add IsCloseTo check for DateTime and DateTimeOffset #311

Closed ocoanet closed 5 years ago

ocoanet commented 5 years ago

Fixes #312.

dupdob commented 5 years ago

Many thanks for this PR. While I think its purpose is self explanatory, could you please open an issue to describe the (expected) behaviour for your new check?

ocoanet commented 5 years ago

Is that ok with you if I keep the TimeSpan based overloads but then converts the TimeSpan to a Duration?

ocoanet commented 5 years ago

I just updated the PR:

The block types are in now the Helpers namespace.
The block types have a Duration field instead of TimeSpan.
The Duration.ToTimeSpan was no longer necessary and was removed.
I added a new factory method for Duration: Duration.FromTimeSpan. It is not required, I can remove it or rename it if needed.
The code is rebased after your latest changes on master.

I used TimeHelper.DiscoverUnit and I was surprised to discover that the MinimumUnitAmount is 2 instead of 1. In my view "1 Seconds" is easier to read than "1000 Milliseconds". I did not change this behavior though.

Tell me if anything needs to be corrected or if the changes need to be squashed.

dupdob commented 5 years ago

Many thanks for these adjustments!

I used TimeHelper.DiscoverUnit and I was surprised to discover that the MinimumUnitAmount is 2 instead of 1. In my view "1 Seconds" is easier to read than "1000 Milliseconds". I did not change this behavior though.

Thanks, here is the rationale: duration are expressed in integer time units, hence the choice of 2 has a minimal value to avoid rounding 1499 ms to 2. Not a very strong case, so I am open to other views.

Just one last request: could you add a test case for Not, especially for when Not.IsCloseTo fails, to have an assertion on the test message? I am afraid I forgot to ask for it in my initial review.

I will update the contributing.md file to list the requested tests.

ocoanet commented 5 years ago

Just one last request: could you add a test case for Not, especially for when Not.IsCloseTo fails, to have an assertion on the test message?

Done.

dupdob commented 5 years ago

Great additions!

dupdob commented 5 years ago

Thanks for a great contribution.