Bug 36567

Summary: Datetime warning in t/db_dependent/Circulation.t and t/db_dependent/Circulation/dateexpiry.t
Product: Koha Reporter: Emmi Takkinen <emmi.takkinen>
Component: Test SuiteAssignee: Emmi Takkinen <emmi.takkinen>
Status: Pushed to oldstable --- QA Contact: Testopia <testopia>
Severity: trivial    
Priority: P5 - low CC: david, fridolin.somers, lucas
Version: Main   
Hardware: All   
OS: All   
Change sponsored?: --- Patch complexity: ---
Documentation contact: Documentation submission:
Text to go in the release notes:
Version(s) released in:
24.05.00,23.11.06,23.05.12
Attachments: Bug 36567: Get rid of Datetime warning in Circulation.t and dateexpiry.t
Bug 36567: Get rid of Datetime warning in Circulation.t and dateexpiry.t
Bug 36567: Get rid of Datetime warning in Circulation.t and dateexpiry.t
Bug 36567: Get rid of Datetime warning in Circulation.t and dateexpiry.t

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!
Comment 10 Fridolin Somers 2024-05-24 13:05:46 UTC
Pushed to 23.11.x for 23.11.06
Comment 11 Lucas Gass 2024-05-28 21:30:24 UTC
Backported to 23.05.x for upcoming 23.05.12