Add timestamp.parse_rfc3339 function

[mooreryan]

See #9

Grammar for parsing and for property test generators taken from section 5.6 - Internet Date/Time Format of RFC 3339.

Most of the testing is done by comparing this to test oracles with property tests. I have FFI for Erlang and JS versions to compare to as well as a test file of 1000 timestamps parsed by the OCaml ptime library. That is included because the Erlang and JS versions have slight limitations compared to the the implementation in this module (mainly regarding fractional second precision), but if you don't want it in there I can take it out.

Couple of issues with the tests that will need to be resolved at some point:

• The Erlang oracle used in the property tests will need to be updated once this is fixed upstream. (The Erlang oracle code adjusts the time to correct for that issue.)
• The int_uniform used in timestamp_rfc3339_timestamp_roundtrip_property_test doesn't get to the boundaries of 0000 to 9999 years. It's out of the range of the prng int generator. (I fixed this in a subsequent commit.)
• The round trip test from string -> timestamp -> string will need to be updated once to_rfc3339 handles fractional seconds.

[mooreryan]

The only part of the interface that I needed to change (other than the parse function) was exposing a duration.normalised function so that I could convert a timestamp into a duration.

[mooreryan]

After fixing the range of the timestamp generator, I had to cherry pick the commit from this other PR: #10.

[lpil]

Wow! Super nice work!! I've left a bunch of questions and notes inline.

Thank you so much!!

[lpil]

Delete this function and inline the body into all the places it was used please. Tests should be as simple as possible

[lpil]

What's k?

[mooreryan]

k is just the name of the loop counter. I could change the name to something more descriptive?

[mooreryan]

Thanks for looking at this and the comments!

Most of your comments I was able to address without additional clarification. I left a couple of follow up comments.

Would you like me to squash all the tiny commits I created as I was addressing the issues?

[mooreryan]

Oh two other questions for you:

---

I had thought to make some tests like this:

pub fn parse_rfc3339_returns_error_for_bad_timestamp_0_test() {
  timestamp.parse_rfc3339("12349-01-01T00:00:00Z")
  |> should.equal(Error(Nil))
}

pub fn parse_rfc3339_returns_error_for_bad_timestamp_1_test() {
  timestamp.parse_rfc3339("1234-019-01T00:00:00Z")
  |> should.equal(Error(Nil))
}

with specific examples that break the grammar in some way, but then I thought it might not be worth the effort. What do you think? (Ideally, I could just generate non-conforming datetime strings, but that would be a bit of work.)

---

The other thing, do you want me to make a couple of parsing tests that don't use oracles at all? Some stuff like

pub fn blah_test() {
  let assert Ok(ts) = timestamp.parse_rfc3339(...)
  timestamp.to_unix_seconds_and_nanoseconds(ts)
  |> should.equal(#(..., ...))

There are quite a few specific examples that are tested (in the parse_rfc3339_matches_oracle_example_N_test functions), but they use the oracles, and so you can't glance at the tests to see the expected values. I don't think this is necessarily an issue, what do you think?

[lpil]

I don't mind RE tests that show it not parsing. Your call.

The other thing, do you want me to make a couple of parsing tests that don't use oracles at all

Maybe a couple would be nice! Thank you.

[mooreryan]

I updated the tests, and made a few more tweaks to address the last unresolved comments.

The last unresolved issue is regarding how the doc comment of the function should be--whether it should detail the specifications of the parsing (comment is here: #12 (comment)). I'm not really sure what is best regarding that comment.

[lpil]

Thank you!!! Fantastic work!

Let's leave it as you've got it there 👍