Improve and extend DateTime/DateTimeOffset generation

[cmeeren]

This PR consists of these changes, each in a separate commit:

• Improve correctness of Gen.dateTime docs (DateTime is decidedly not an instant in time; DateTimeOffset is). This change was also done for consistency with the new functions added in later commits.
• Added Gen.dateTimeOffset. It's a "primitive" that is non-trivial to generate (care must be taken to avoid overflows due to adding offsets around the min/max values) and should be in fsharp-hedgehog
• Added ranged variants of Gen.dateTime and Gen.dateTimeOffset, allowing users to specify their own date ranges (I have myself come across the need to limit the date ranges due to complicated time zone stuff some country did a hundred years ago, which is not interesting to account for in my domain)
• Implemented dateTime and dateTimeOffset in terms of their ranged variants

There is one other change I'd like to make, but for some reason it won't compile: I'd like to change the default range of dateTime and dateTimeOffset from constant to either linear or exponential. I don't see a reason why it should be size-independent. But when I tried to change it, I get compile errors:

[cmeeren]

We should probably add tests, at least for the DateTimeOffset generator(s). I experimented a bit with them now, and there may be a bug. I'll have a look at it tomorrow.

[cmeeren]

The bug is now fixed. I have experimented with both dateTimeOffset and dateTimeOffsetRanged in FSI. They seem to produce sensible values according to the size (apart from the default range being constant, as previously mentioned), and never crash. I have also tested them at their min/max values. I feel fairly confident in these generators now, even without tests.

[cmeeren]

To be clear, the only outstanding thing here is the compilation error I mentioned if we should change the default range. I don't know what to do about that error, but it can also be done later (e.g. in another PR) or not at all (if you don't agree with the change). So feel free to review (and see if there is a simple fix to the compilation error when changing the range).

[moodmosaic]

Interesting. Is that issue happening on master or just in v0.8.3?

[cmeeren]

The compilation error? On master (and 0.8.4). I discovered it while working on this PR, so I don't understand how 0.8.3 is relevant.

[moodmosaic]

There is one other change I'd like to make, but for some reason it won't compile: I'd like to change the default range of dateTime and dateTimeOffset from constant to either linear or exponential. I don't see a reason why it should be size-independent. But when I tried to change it, I get compile errors

Perhaps we're missing something in Numeric.fs(?) 🤔 Reminds me of #156.

[moodmosaic]

👍 💯 I left a comment, but overall this looks great.

[moodmosaic]

👍

[cmeeren]

I have made the change now. Note:

• The test dateTime shrinks to correct mid-value no longer checks that it shrinks to a hardcoded value, but still makes sense if one wants to test that it shrinks correctly based on the range. I left it in.
• When fixing the tests, I had to use Range.constant for the reason described in the OP. Furthermore, it's not possible to use the *Bounded ranges, e.g. Range.constantBounded gives an error because DateTime does not support get_Zero.

[moodmosaic]

[..]I had to use Range.constant for the reason described in the OP. Furthermore, it's not possible to use the *Bounded ranges, e.g. Range.constantBounded gives an error because DateTime does not support get_Zero.

I'd like to look into this before merging, if that's OK.

[cmeeren]

I'd like to look into this before merging, if that's OK.

Absolutely.

[cmeeren]

Any chance to have a look at this so it can be merged? :)

[moodmosaic]

[..]I had to use Range.constant for the reason described in the OP. Furthermore, it's not possible to use the *Bounded ranges, e.g. Range.constantBounded gives an error because DateTime does not support get_Zero.

I'd like to look into this before merging, if that's OK.

That compiler error you were getting is because of the scaleLinear function used internally by linearFrom. If you instead work on Ticks and then map to DateTime, it should compile just fine:

@@ -53,11 +53,18 @@ let ``dateTime randomly generates value between max and min ticks`` () =
 [<Fact>]
 let ``dateTime shrinks to correct mid-value`` () =
     let result =
         property {
             let! actual = 
-              Range.constantFrom (System.DateTime (2000, 1, 1)) System.DateTime.MinValue System.DateTime.MaxValue
+              Range.linearFrom
+                (System.DateTime (2000, 1, 1))
+                     .Ticks
+                 System.DateTime.MinValue
+                     .Ticks
+                 System.DateTime.MaxValue
+                     .Ticks
+              |> Range.map System.DateTime
               |> Gen.dateTime
             System.DateTime.Now =! actual
         }
         |> Property.report
         |> Report.render

[moodmosaic]

Thank you! Looking good 👍 I've suggested a small change in the docs. We can merge after that.

[cmeeren]

If you instead work on Ticks and then map to DateTime, it should compile just fine:

Yes, but then you sidestep the whole generator and have effectively written your own generator. Furthermore, the same applies to DateTimeOffset, and that generator is significantly more complicated than just mapping from ticks to DateTimeOffset. Is there nothing that can be done to make these generators support non-constant ranges?

[moodmosaic]

but then you sidestep the whole generator and have effectively written your own generator

I'm not sure I fully understand. I've added .Ticks to each one of the parameters here (and mapped to DateTime)

Range.constantFrom (System.DateTime (2000, 1, 1)) System.DateTime.MinValue System.DateTime.MaxValue

[cmeeren]

I'm not sure I fully understand. I've added .Ticks to each one of the parameters here (and mapped to DateTime)

My bad, I see you map the Range and not the Gen as I thought.