Bug 36567 - Datetime warning in t/db_dependent/Circulation.t and t/db_dependent/Circulation/dateexpiry.t
Summary: Datetime warning in t/db_dependent/Circulation.t and t/db_dependent/Circulati...
Status: Pushed to main
Alias: None
Product: Koha
Classification: Unclassified
Component: Test Suite (show other bugs)
Version: Main
Hardware: All All
: P5 - low trivial (vote)
Assignee: Emmi Takkinen
QA Contact: Testopia
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2024-04-10 11:31 UTC by Emmi Takkinen
Modified: 2024-05-03 14:03 UTC (History)
1 user (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:
24.05.00


Attachments
Bug 36567: Get rid of Datetime warning in Circulation.t and dateexpiry.t (2.63 KB, patch)
2024-04-10 11:56 UTC, Emmi Takkinen
Details | Diff | Splinter Review
Bug 36567: Get rid of Datetime warning in Circulation.t and dateexpiry.t (2.63 KB, patch)
2024-04-26 10:16 UTC, Emmi Takkinen
Details | Diff | Splinter Review
Bug 36567: Get rid of Datetime warning in Circulation.t and dateexpiry.t (2.68 KB, patch)
2024-04-26 10:22 UTC, David Nind
Details | Diff | Splinter Review
Bug 36567: Get rid of Datetime warning in Circulation.t and dateexpiry.t (2.73 KB, patch)
2024-05-03 09:21 UTC, Pedro Amorim
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Emmi Takkinen 2024-04-10 11:31:11 UTC
Running t/db_dependent/Circulation.t and t/db_dependent/Circulation/dateexpiry.t causes following warning:

>You are creating a DateTime object with a far future year (9999) and a time 
>zone (Europe/Helsinki). If the time zone you specified has future DST changes 
>this will be very slow. at ../Koha/C4/Circulation.pm line 1614.

Datetime modules pod states (https://metacpan.org/pod/DateTime):
>Currently, constructors will warn if you try to create a far future DateTime 
>(year >= 5000) with any time zone besides floating or UTC. This can be very 
>slow if the time zone has future DST transitions that need to be calculated. 
>If the date is sufficiently far in the future this can be really slow 
>(minutes).
Comment 1 Emmi Takkinen 2024-04-10 11:56:35 UTC
Created attachment 164609 [details] [review]
Bug 36567: Get rid of Datetime warning in Circulation.t and dateexpiry.t

Running either t/db_dependent/Circulation.t or t/db_dependent/Circulation/dateexpiry.t
cause following error to be dispalyed:

You are creating a DateTime object with a far future year (9999) and a time
zone (Europe/Helsinki). If the time zone you specified has future DST changes
this will be very slow.

Smallest allowed value is 4999, so we need to use that rather than 9999 in tests.

To test prove t/db_dependent/Circulation.t and t/db_dependent/Circulation/dateexpiry.t.

Sponsored-by: Koha-Suomi Oy
Comment 2 David Nind 2024-04-19 00:03:43 UTC
Tests fail for prove t/db_dependent/Circulation.t after applying the patches (both tests pass before, only t/db_dependent/Circulation/dateexpiry.t passes after):

prove t/db_dependent/Circulation.t
t/db_dependent/Circulation.t .. 45/70 
    #   Failed test 'renew log successfully added'
    #   at t/db_dependent/Circulation.t line 4968.
    #          got: '2'
    #     expected: '3'
    # Looks like you failed 1 test of 13.
t/db_dependent/Circulation.t .. 49/70 
#   Failed test 'AddRenewal and AddIssuingCharge tests'
#   at t/db_dependent/Circulation.t line 4987.
t/db_dependent/Circulation.t .. 66/70 # Looks like you failed 1 test of 70.
t/db_dependent/Circulation.t .. Dubious, test returned 1 (wstat 256, 0x100)
Failed 1/70 subtests 

Test Summary Report
-------------------
t/db_dependent/Circulation.t (Wstat: 256 (exited 1) Tests: 70 Failed: 1)
  Failed test:  49
  Non-zero exit status: 1
Files=1, Tests=70, 26 wallclock secs ( 0.08 usr  0.01 sys + 17.25 cusr  5.89 csys = 23.23 CPU)
Result: FAIL
Comment 3 Emmi Takkinen 2024-04-26 10:15:29 UTC
(In reply to David Nind from comment #2)
> Tests fail for prove t/db_dependent/Circulation.t after applying the patches
> (both tests pass before, only t/db_dependent/Circulation/dateexpiry.t passes
> after):
> 
> prove t/db_dependent/Circulation.t
> t/db_dependent/Circulation.t .. 45/70 
>     #   Failed test 'renew log successfully added'
>     #   at t/db_dependent/Circulation.t line 4968.
>     #          got: '2'
>     #     expected: '3'
>     # Looks like you failed 1 test of 13.
> t/db_dependent/Circulation.t .. 49/70 
> #   Failed test 'AddRenewal and AddIssuingCharge tests'
> #   at t/db_dependent/Circulation.t line 4987.
> t/db_dependent/Circulation.t .. 66/70 # Looks like you failed 1 test of 70.
> t/db_dependent/Circulation.t .. Dubious, test returned 1 (wstat 256, 0x100)
> Failed 1/70 subtests 
> 
> Test Summary Report
> -------------------
> t/db_dependent/Circulation.t (Wstat: 256 (exited 1) Tests: 70 Failed: 1)
>   Failed test:  49
>   Non-zero exit status: 1
> Files=1, Tests=70, 26 wallclock secs ( 0.08 usr  0.01 sys + 17.25 cusr  5.89
> csys = 23.23 CPU)
> Result: FAIL

I'm unable to repeat this and I'm not even sure how these changes could affect those tests. Could this be caused by something in your testing environment?
Comment 4 Emmi Takkinen 2024-04-26 10:16:26 UTC
Created attachment 165635 [details] [review]
Bug 36567: Get rid of Datetime warning in Circulation.t and dateexpiry.t

Running either t/db_dependent/Circulation.t or t/db_dependent/Circulation/dateexpiry.t
cause following error to be dispalyed:

You are creating a DateTime object with a far future year (9999) and a time
zone (Europe/Helsinki). If the time zone you specified has future DST changes
this will be very slow.

Smallest allowed value is 4999, so we need to use that rather than 9999 in tests.

To test prove t/db_dependent/Circulation.t and t/db_dependent/Circulation/dateexpiry.t.

Sponsored-by: Koha-Suomi Oy
Comment 5 Emmi Takkinen 2024-04-26 10:17:17 UTC
Rebased patch, keeping this as Failed QA due comment 2.
Comment 6 David Nind 2024-04-26 10:22:49 UTC
Created attachment 165636 [details] [review]
Bug 36567: Get rid of Datetime warning in Circulation.t and dateexpiry.t

Running either t/db_dependent/Circulation.t or t/db_dependent/Circulation/dateexpiry.t
cause following error to be dispalyed:

You are creating a DateTime object with a far future year (9999) and a time
zone (Europe/Helsinki). If the time zone you specified has future DST changes
this will be very slow.

Smallest allowed value is 4999, so we need to use that rather than 9999 in tests.

To test prove t/db_dependent/Circulation.t and t/db_dependent/Circulation/dateexpiry.t.

Sponsored-by: Koha-Suomi Oy
Signed-off-by: David Nind <david@davidnind.com>
Comment 7 David Nind 2024-04-26 10:24:08 UTC
Both tests now pass after applying the patch.

Not sure whether it was something with my KTD or not...
Comment 8 Pedro Amorim 2024-05-03 09:21:21 UTC
Created attachment 166096 [details] [review]
Bug 36567: Get rid of Datetime warning in Circulation.t and dateexpiry.t

Running either t/db_dependent/Circulation.t or t/db_dependent/Circulation/dateexpiry.t
cause following error to be dispalyed:

You are creating a DateTime object with a far future year (9999) and a time
zone (Europe/Helsinki). If the time zone you specified has future DST changes
this will be very slow.

Smallest allowed value is 4999, so we need to use that rather than 9999 in tests.

To test prove t/db_dependent/Circulation.t and t/db_dependent/Circulation/dateexpiry.t.

Sponsored-by: Koha-Suomi Oy
Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com>
Comment 9 Katrin Fischer 2024-05-03 14:03:58 UTC
Pushed for 24.05!

Well done everyone, thank you!