Bug 22254 - t/db_dependent/Koha/Patrons.t contains a DateTime math error
Summary: t/db_dependent/Koha/Patrons.t contains a DateTime math error
Status: RESOLVED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Test Suite (show other bugs)
Version: master
Hardware: All All
: P5 - low normal (vote)
Assignee: Martin Renvoize
QA Contact: Testopia
URL:
Keywords:
Depends on: 17699
Blocks:
  Show dependency treegraph
 
Reported: 2019-02-01 09:53 UTC by Martin Renvoize
Modified: 2019-05-27 12:21 UTC (History)
5 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Small patch
Who signed the patch off:
Text to go in the release notes:
Version(s) released in:


Attachments
Bug 22254: Correction to datehandling in test (2.05 KB, patch)
2019-02-01 09:57 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 22254: Correction to datehandling in test (2.10 KB, patch)
2019-02-01 10:43 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 22254: Correction to datehandling in test (2.40 KB, patch)
2019-02-03 09:25 UTC, Katrin Fischer
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Martin Renvoize 2019-02-01 09:53:09 UTC
So, one of the peculiarities of DateTime maths is that 12 plus 1 doesn't always equal 13 ;)

This calls for an example (using 'limit' for end_of_month mode):

2019-01-31 + 1 month = 2019-02-28
2019-01-31 + 13 months = 2020-02-29 (this one maintains 'end of month' over the leap year)
2019-02-28 + 12 months = 2020-02-28

Koha does the third case but the test is currently setup for the second.
Comment 1 Martin Renvoize 2019-02-01 09:57:29 UTC
Created attachment 84599 [details] [review]
Bug 22254: Correction to datehandling in test

Adding 13 months is not always the same as adding 1 month and then
adding 12 months in DateTime maths.  This patch rectified the test
to match the correct behaviour in koha.
Comment 2 Martin Renvoize 2019-02-01 10:01:00 UTC
Test plan.

1) Modify the test (around line 268) to test against the suspect leap date of 2019-01-31 and watch the test fail.
2) Apply the patch (which also adds this modification to test for such a leap date) and watch the test now pass.
Comment 3 Tomás Cohen Arazi 2019-02-01 10:43:58 UTC
Created attachment 84600 [details] [review]
Bug 22254: Correction to datehandling in test

Adding 13 months is not always the same as adding 1 month and then
adding 12 months in DateTime maths.  This patch rectified the test
to match the correct behaviour in koha.

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 4 Katrin Fischer 2019-02-03 09:25:23 UTC
Created attachment 84677 [details] [review]
Bug 22254: Correction to datehandling in test

Adding 13 months is not always the same as adding 1 month and then
adding 12 months in DateTime maths.  This patch rectified the test
to match the correct behaviour in koha.

Test plan.

1) Modify the test (around line 268) to test against the suspect
   leap date of 2019-01-31 and watch the test fail.
2) Apply the patch (which also adds this modification to test
   for such a leap date) and watch the test now pass.

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 5 Nick Clemens 2019-02-04 17:48:07 UTC
Awesome work all!

Pushed to master for 19.05
Comment 6 Martin Renvoize 2019-02-11 13:40:44 UTC
Pushed to 18.11.x for 18.11.03
Comment 7 Lucas Gass 2019-02-12 20:22:33 UTC
backported to 18.05.x for 18.05.09
Comment 8 Jonathan Druart 2019-02-16 19:04:07 UTC
Good catch Martin. I would have added more dates, like:

+    for my $date ( '2016-03-31', '2016-11-30', '2019-01-31', '2020-02-28', '2020-01-31', '2020-02-29', dt_from_string() ) {

Just in case ;)
Comment 9 Fridolin SOMERS 2019-02-25 10:17:58 UTC
Pushed to 17.11.x for 17.11.16