Skip to content
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

Parse error, ZonedDateTime #981

Closed
haf opened this issue Oct 4, 2017 · 3 comments
Closed

Parse error, ZonedDateTime #981

haf opened this issue Oct 4, 2017 · 3 comments
Assignees
Labels
Milestone

Comments

@haf
Copy link

haf commented Oct 4, 2017

This test fails:

let tzdb: IDateTimeZoneProvider =
  NodaTime.DateTimeZoneProviders.Tzdb

// ...
      testCase "parse failing zoned date time" <| fun _ ->
        let p = NodaTime.Text.ZonedDateTimePattern.ExtendedFormatOnlyIso.WithZoneProvider tzdb
        let res = p.Parse "1906-08-29T20:58:32 Etc/GMT-12 (+12)"

        if not res.Success then
          Tests.failtestf
            "Should successfully parse generated zoned date time, but got %s"
            res.Exception.Message

With this error message:

[20:43:33 ERR] serialisation/base types/parse failing zoned date time failed in 00:00:00.0350000.
Should successfully parse generated zoned date time, but got The value string does not match a simple character in the format string " ". Value being parsed: '1906-08-29T20:58:32 Etc/GMT-1^2 (+12)'. (^ indicates error position.)

The value was generated thus:

type Arbs =
  static member Instant () =
    let genInst = gen {
      let! y = Gen.choose(1900,2100)
      let! m = Gen.choose(1, 12)
      let! d = Gen.choose(1, System.DateTime.DaysInMonth(y, m))
      let! h = Gen.choose(0,23)
      let! min = Gen.choose(0,59)
      let! sec = Gen.choose(0,59)
      return Instant.FromUtc(y, m, d, h, min, sec)
    }
    Arb.fromGen genInst

  static member Timezone() =
    gen {
      let! tz = Gen.choose (0, tzdb.Ids.Count - 1)
      return tzdb.Item tzdb.Ids.[tz]
    }
    |> Arb.fromGen

  static member ZonedDateTime() =
    gen {
      let! tz = Arb.generate<_>
      let! inst = Arb.generate<_>
      return ZonedDateTime(inst, tz)
    }
    |> Arb.fromGen

Version of things:

macOS 10.12.6, mono 5.2.0.215, NodaTime 1.4

More failing test strings:

  • The value string does not match a simple character in the format string " ". Value being parsed: '1994-02-09T09:02:33 Etc/GMT-1^1 (+11)'. (^ indicates error position.)
  • ...
@jskeet
Copy link
Member

jskeet commented Oct 4, 2017

Thanks for reporting this. I'll look into it tomorrow.
Here's a shorter way of reproducing the problem in C#:

using System;
using NodaTime;
using NodaTime.Text;

class Test
{
    static void Main()
    {
        var pattern = ZonedDateTimePattern.ExtendedFormatOnlyIso
            .WithZoneProvider(DateTimeZoneProviders.Tzdb);
        
        var value = pattern.Parse("1906-08-29T20:58:32 Etc/GMT-12 (+12)").Value;
    }
}

This fails in NodaTime 2.2.0 with regular .NET on Windows.

(Once I've found the bug, I'll need to work out whether or not to backport the fix to the 1.4 series.)

@jskeet jskeet self-assigned this Oct 4, 2017
@jskeet jskeet added the bug label Oct 4, 2017
@jskeet
Copy link
Member

jskeet commented Oct 4, 2017

(I suspect the problem is that GMT-1 is a valid time zone ID, so it thinks that's the time zone. This may be somewhat painful to fix... we'll see.)

jskeet added a commit to jskeet/nodatime that referenced this issue Oct 5, 2017
We were previously stopping too early in the search.

Fixes nodatime#981. Should probably be backported to 1.4.x and 2.2.x.
@jskeet
Copy link
Member

jskeet commented Oct 9, 2017

Closing for the development branch, but will probably backport to 1.4.x and 2.2.x

@jskeet jskeet closed this as completed Oct 9, 2017
This was referenced Oct 9, 2017
@malcolmr malcolmr added this to the 2.2.1 milestone Oct 9, 2017
jskeet added a commit to jskeet/nodatime that referenced this issue Oct 9, 2017
We were previously stopping too early in the search.

Fixes nodatime#981. Should probably be backported to 1.4.x and 2.2.x.
jskeet added a commit that referenced this issue Oct 9, 2017
We were previously stopping too early in the search.

Fixes #981. Should probably be backported to 1.4.x and 2.2.x.
jskeet added a commit to jskeet/nodatime that referenced this issue Oct 12, 2017
This is a cherry-pick of f78a32a for the 1.4 branch.

Fixes nodatime#981.
jskeet added a commit to jskeet/nodatime that referenced this issue Oct 12, 2017
We were previously stopping too early in the search.

Fixes nodatime#981.

(Cherry-picked from f78a32a)
jskeet added a commit to jskeet/nodatime that referenced this issue Oct 12, 2017
We were previously stopping too early in the search.

Fixes nodatime#981.

(Cherry-picked from f78a32a)
jskeet added a commit to jskeet/nodatime that referenced this issue Oct 14, 2017
This is a cherry-pick of f78a32a for the 1.4 branch.

Fixes nodatime#981.
jskeet added a commit to jskeet/nodatime that referenced this issue Oct 14, 2017
This is a cherry-pick of f78a32a for the 1.4 branch.

Fixes nodatime#981.
jskeet added a commit that referenced this issue Oct 14, 2017
We were previously stopping too early in the search.

Fixes #981.

(Cherry-picked from f78a32a)
jskeet added a commit that referenced this issue Oct 14, 2017
This is a cherry-pick of f78a32a for the 1.4 branch.

Fixes #981.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants