Skip Menu |

This queue is for tickets about the DateTimeX-Start CPAN distribution.

Report information
The Basics
Id: 118417
Status: resolved
Priority: 0/
Queue: DateTimeX-Start

People
Owner: IKEGAMI [...] cpan.org
Requestors: DLAMBLEY [...] cpan.org
Cc:
AdminCc:

Bug Information
Severity: Important
Broken in:
  • v1.2.0
  • v1.0.0
Fixed in: v1.4.0



Subject: Incorrect behaviour with DateTime objects
I believe I have found some incorrect behaviour when determining the start of day from a DateTime object. Consider, $ perl -Ilib -e'use DateTimeX::Start qw/ start_of_date /; $dt = start_of_date(DateTime->new( year => 2016, month => 10, day => 30, hour => 23, time_zone => "Europe/London" )); $dt->set_time_zone("Europe/London"); print $dt->iso8601."\n";' 2016-10-30T00:01:00 I believe the correct result here should be "2016-10-30T00:00:00". The attached patch should resolve this problem, as far as I understand it. I have also created a pull request at https://github.com/ikegami/perl-DateTimeX-Start/pull/1 Best regards, Dave Lambley
Subject: DateTime.patch
From 8332cecd56d8dff3671506a583442ba2c779d629 Mon Sep 17 00:00:00 2001 From: Dave Lambley <dave@adzuna.com> Date: Mon, 17 Oct 2016 16:20:45 +0100 Subject: [PATCH 1/3] Add more tests. Check correct behaviour this year and in the UK. --- t/07_basic.t | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/t/07_basic.t b/t/07_basic.t index b8cc713..863dafd 100755 --- a/t/07_basic.t +++ b/t/07_basic.t @@ -26,9 +26,27 @@ sub dt { { my @tests = ( + # Toronto [ 'An ordinary day', '0.18', sub { start_of_date([ 2014, 2, 3 ], 'America/Toronto' ) }, sub { dt(2014, 2, 3, 5, 0, 0)->set_time_zone('America/Toronto' ) } ], + + # Sao Paulo [ 'A day without midnight', '0.26', sub { start_of_date([ 2013, 10, 20 ], 'America/Sao_Paulo') }, sub { dt(2013, 10, 20, 3, 0, 0)->set_time_zone('America/Sao_Paulo') } ], + [ 'An ordinary day 2', '0.26', sub { start_of_date([ 2016, 10, 15 ], 'America/Sao_Paulo') }, sub { dt(2016, 10, 15, 3, 0, 0)->set_time_zone('America/Sao_Paulo') } ], + [ 'An ordinary day 2 local', '0.26', sub { start_of_date([ 2016, 10, 15 ], 'America/Sao_Paulo') }, sub { dt(2016, 10, 15, 0, 0, 0, 'America/Sao_Paulo') } ], + [ 'A day without midnight 2', '0.26', sub { start_of_date([ 2016, 10, 16 ], 'America/Sao_Paulo') }, sub { dt(2016, 10, 16, 3, 0, 0)->set_time_zone('America/Sao_Paulo') } ], + [ 'A day without midnight 2 local', '0.26', sub { start_of_date([ 2016, 10, 16 ], 'America/Sao_Paulo') }, sub { dt(2016, 10, 16, 1, 0, 0, 'America/Sao_Paulo') } ], + [ 'An ordinary day 3', '0.26', sub { start_of_date([ 2016, 10, 17 ], 'America/Sao_Paulo') }, sub { dt(2016, 10, 17, 2, 0, 0)->set_time_zone('America/Sao_Paulo') } ], + [ 'An ordinary day 3 local', '0.26', sub { start_of_date([ 2016, 10, 17 ], 'America/Sao_Paulo') }, sub { dt(2016, 10, 17, 0, 0, 0, 'America/Sao_Paulo') } ], + + # Havana [ 'A day with two midnights', '1.53', sub { start_of_date([ 2013, 11, 3 ], 'America/Havana' ) }, sub { dt(2013, 11, 3, 4, 0, 0)->set_time_zone('America/Havana' ) } ], + + # UK + [ 'UK, BST', '1.53', sub { start_of_date([ 2016, 10, 30 ], 'Europe/London' ) }, sub { dt(2016, 10, 29,23, 0, 0)->set_time_zone('Europe/London') } ], + [ 'UK, BST local', '1.53', sub { start_of_date([ 2016, 10, 30 ], 'Europe/London' ) }, sub { dt(2016, 10, 30, 0, 0, 0, 'Europe/London') } ], + [ 'UK, UTC local', '1.53', sub { start_of_date([ 2016, 10, 31 ], 'Europe/London' ) }, sub { dt(2016, 10, 31, 0, 0, 0, 'Europe/London') } ], + [ 'UK, UTC', '1.53', sub { start_of_date([ 2016, 10, 31 ], 'Europe/London' ) }, sub { dt(2016, 10, 31, 0, 0, 0)->set_time_zone('Europe/London') } ], + [ 'UK, UTC local', '1.53', sub { start_of_date([ 2016, 10, 31 ], 'Europe/London' ) }, sub { dt(2016, 10, 31, 0, 0, 0, 'Europe/London') } ], ); plan tests => 0+@tests; -- 1.9.1 From 0fdc773ec321a14d91830ba0cc9a837bb14e98de Mon Sep 17 00:00:00 2001 From: Dave Lambley <dave@adzuna.com> Date: Mon, 17 Oct 2016 18:34:37 +0100 Subject: [PATCH 2/3] Test behaviour with objects. --- t/07_basic.t | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/t/07_basic.t b/t/07_basic.t index 863dafd..7578362 100755 --- a/t/07_basic.t +++ b/t/07_basic.t @@ -41,12 +41,21 @@ sub dt { # Havana [ 'A day with two midnights', '1.53', sub { start_of_date([ 2013, 11, 3 ], 'America/Havana' ) }, sub { dt(2013, 11, 3, 4, 0, 0)->set_time_zone('America/Havana' ) } ], + [ 'Sao Paulo, A', '1.53', sub { start_of_date(DateTime->new( year => 2016, month => 10, day => 15, hour => 0, time_zone => 'America/Sao_Paulo' )) }, sub { dt(2016, 10, 15, 0, 0, 0, 'America/Sao_Paulo') } ], + [ 'Sao Paulo, B', '1.53', sub { start_of_date(DateTime->new( year => 2016, month => 10, day => 15, hour => 23, minute => 59, second => 59, time_zone => 'America/Sao_Paulo' )) }, sub { dt(2016, 10, 15, 0, 0, 0, 'America/Sao_Paulo') } ], + [ 'Sao Paulo, C', '1.53', sub { start_of_date(DateTime->new( year => 2016, month => 10, day => 15, hour => 23, minute => 59, second => 59, time_zone => 'America/Sao_Paulo' )->add(seconds => 1)) }, sub { dt(2016, 10, 16, 1, 0, 0, 'America/Sao_Paulo') } ], + # UK [ 'UK, BST', '1.53', sub { start_of_date([ 2016, 10, 30 ], 'Europe/London' ) }, sub { dt(2016, 10, 29,23, 0, 0)->set_time_zone('Europe/London') } ], [ 'UK, BST local', '1.53', sub { start_of_date([ 2016, 10, 30 ], 'Europe/London' ) }, sub { dt(2016, 10, 30, 0, 0, 0, 'Europe/London') } ], [ 'UK, UTC local', '1.53', sub { start_of_date([ 2016, 10, 31 ], 'Europe/London' ) }, sub { dt(2016, 10, 31, 0, 0, 0, 'Europe/London') } ], [ 'UK, UTC', '1.53', sub { start_of_date([ 2016, 10, 31 ], 'Europe/London' ) }, sub { dt(2016, 10, 31, 0, 0, 0)->set_time_zone('Europe/London') } ], [ 'UK, UTC local', '1.53', sub { start_of_date([ 2016, 10, 31 ], 'Europe/London' ) }, sub { dt(2016, 10, 31, 0, 0, 0, 'Europe/London') } ], + + [ 'UK, BST floating', '1.53', sub { start_of_date(DateTime->new( year => 2016, month => 10, day => 30, time_zone => 'floating' ), 'Europe/London' ) }, sub { dt(2016, 10, 29,23, 0, 0)->set_time_zone('Europe/London') } ], + [ 'UK, BST floating late', '1.53', sub { start_of_date(DateTime->new( year => 2016, month => 10, day => 30, hour => 23, time_zone => 'floating' ), 'Europe/London' ) }, sub { dt(2016, 10, 29,23, 0, 0)->set_time_zone('Europe/London') } ], + [ 'UK, BST tz', '1.53', sub { start_of_date(DateTime->new( year => 2016, month => 10, day => 30, time_zone => 'Europe/London' ), 'Europe/London' ) }, sub { dt(2016, 10, 29,23, 0, 0)->set_time_zone('Europe/London') } ], + [ 'UK, BST tz late', '1.53', sub { start_of_date(DateTime->new( year => 2016, month => 10, day => 30, hour => 23, time_zone => 'Europe/London' ), 'Europe/London' ) }, sub { dt(2016, 10, 29,23, 0, 0)->set_time_zone('Europe/London') } ], ); plan tests => 0+@tests; @@ -58,6 +67,7 @@ sub dt { skip("DateTime::TimeZone $min_ver required", 1) if $DateTime::TimeZone::VERSION < $min_ver; + local $@; my $got_dt = eval { $test_code->() }; if (!$got_dt) { my $e = $@; @@ -67,6 +77,7 @@ sub dt { next; } + local $@; my $expected_dt = eval { $expected_code->() }; if (!$expected_dt) { my $e = $@; -- 1.9.1 From 89491f00406447c31ee8b3e880b309574db1b4d0 Mon Sep 17 00:00:00 2001 From: Dave Lambley <dave@adzuna.com> Date: Mon, 17 Oct 2016 18:41:30 +0100 Subject: [PATCH 3/3] Correctly truncate date objects. --- lib/DateTimeX/Start.pm | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/DateTimeX/Start.pm b/lib/DateTimeX/Start.pm index 51354eb..a46f0d1 100644 --- a/lib/DateTimeX/Start.pm +++ b/lib/DateTimeX/Start.pm @@ -52,10 +52,10 @@ sub _start_of_date { return DateTime->from_epoch(epoch => $max_epoch*60, time_zone => $tz); } -sub start_of_date { _start_of_date(undef, @_) } +sub start_of_date { _start_of_date('day', @_) } sub start_of_month { _start_of_date('month', @_) } sub start_of_year { _start_of_date('year', @_) } -sub start_of_today { _start_of_date(undef, DateTime->now( time_zone => $_[0] || 'local' )) } +sub start_of_today { _start_of_date('day', DateTime->now( time_zone => $_[0] || 'local' )) } { no warnings qw( once ); -- 1.9.1
Subject: Re: [rt.cpan.org #118417] Incorrect behaviour with DateTime objects
Date: Mon, 17 Oct 2016 14:28:59 -0400
To: bug-DateTimeX-Start [...] rt.cpan.org
From: Eric Brine <ikegami [...] adaelis.com>
Looking into this.
Subject: Re: [rt.cpan.org #118417] Incorrect behaviour with DateTime objects
Date: Mon, 17 Oct 2016 14:35:10 -0400
To: bug-DateTimeX-Start [...] rt.cpan.org
From: Eric Brine <ikegami [...] adaelis.com>
There does indeed appear to be a bug. $ perl -e' use DateTimeX::Start qw/ start_of_date /; my $dt = start_of_date(DateTime->new( year => 2016, month => 10, day => 30, hour => 23, time_zone => "Europe/London" )); CORE::say $dt->epoch, " ", $dt; $dt->subtract( seconds => 1 ); CORE::say $dt->epoch, " ", $dt; ' 1477782060 2016-10-30T00:01:00 1477782059 2016-10-30T00:00:59
Subject: Re: [rt.cpan.org #118417] Incorrect behaviour with DateTime objects
Date: Mon, 17 Oct 2016 14:46:41 -0400
To: bug-DateTimeX-Start [...] rt.cpan.org
From: Eric Brine <ikegami [...] adaelis.com>
I'm not convinced the correct fix was provided. I'll look into this further in 10 hours. On Mon, Oct 17, 2016 at 2:35 PM, ikegami@adaelis.com via RT < bug-DateTimeX-Start@rt.cpan.org> wrote: Show quoted text
> Queue: DateTimeX-Start > Ticket <URL: https://rt.cpan.org/Ticket/Display.html?id=118417 > > > There does indeed appear to be a bug. > > $ perl -e' > use DateTimeX::Start qw/ start_of_date /; > my $dt = start_of_date(DateTime->new( year => 2016, month => 10, day => > 30, hour => 23, time_zone => "Europe/London" )); > CORE::say $dt->epoch, " ", $dt; > $dt->subtract( seconds => 1 ); > CORE::say $dt->epoch, " ", $dt; > ' > 1477782060 2016-10-30T00:01:00 > 1477782059 2016-10-30T00:00:59 > >
Subject: Re: [rt.cpan.org #118417] Incorrect behaviour with DateTime objects
Date: Wed, 19 Oct 2016 01:04:00 -0400
To: bug-DateTimeX-Start [...] rt.cpan.org
From: Eric Brine <ikegami [...] adaelis.com>
I'll fix it this weekend. The module performs a binary search on epoch timestamps to find the right timestamp. As an optimization, the search space is restricted to timestamps near the initial point. Unfortunately, the search space is too restricted. I'll need to figured out what the correct restriction is, and add tests to check this. On Mon, Oct 17, 2016 at 2:46 PM, Eric Brine <ikegami@adaelis.com> wrote: Show quoted text
> I'm not convinced the correct fix was provided. I'll look into this > further in 10 hours. > > On Mon, Oct 17, 2016 at 2:35 PM, ikegami@adaelis.com via RT < > bug-DateTimeX-Start@rt.cpan.org> wrote: >
>> Queue: DateTimeX-Start >> Ticket <URL: https://rt.cpan.org/Ticket/Display.html?id=118417 > >> >> There does indeed appear to be a bug. >> >> $ perl -e' >> use DateTimeX::Start qw/ start_of_date /; >> my $dt = start_of_date(DateTime->new( year => 2016, month => 10, day => >> 30, hour => 23, time_zone => "Europe/London" )); >> CORE::say $dt->epoch, " ", $dt; >> $dt->subtract( seconds => 1 ); >> CORE::say $dt->epoch, " ", $dt; >> ' >> 1477782060 2016-10-30T00:01:00 >> 1477782059 2016-10-30T00:00:59 >> >>
>
Subject: Re: [rt.cpan.org #118417] Incorrect behaviour with DateTime objects
Date: Mon, 24 Oct 2016 14:08:19 -0400
To: bug-DateTimeX-Start [...] rt.cpan.org
From: Eric Brine <ikegami [...] adaelis.com>
Changing undef to 'day' is indeed a sufficient fix because the start of the day in the target time zone will always be within +/-24 hours of the start of the in UTC. In fact, it will be within -12..+14, so searching in -24..+24 gives a buffer. My plan: - Change undef to 'day' as per your patch. - Document "the start of the day in the target time zone will always be within +/-24 hours of the start of the in UTC" to the list of assumption. - Cause the search to fail if no result is found, rather than silently returning the wrong result. - Add tests. I'm dealing with medical issues which prevented me to actually make these changes this weekend, but at least you know your fix is legitimate if you need the module to work right away.
There are actually two bugs: 1) When the result is exactly 24 hours before the input. 2) When the result is more than 24 hours before the input.
Fixed in v1.4.0, on its way to your local CPAN mirror.