Bug 30030

Summary: Test t/db_dependent/Koha/Patrons.t is_going_to_expire fails on U18
Product: Koha Reporter: Fridolin Somers <fridolin.somers>
Component: Test SuiteAssignee: Mason James <mtj>
Status: Failed QA --- QA Contact: Testopia <testopia>
Severity: normal    
Priority: P5 - low CC: jonathan.druart, joonas.kylmala, mtj, tomascohen, victor
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:
Bug Depends on: 29718    
Bug Blocks: 25551    
Attachments: Bug 30030: add 'invalid expiry date' tests, for every timezone

Description Fridolin Somers 2022-02-04 20:10:01 UTC
See https://jenkins.koha-community.org/view/master/job/Koha_Master_U18/lastCompletedBuild/testReport/

    not ok 9 - On invalid "is going to expire" date, the method should not crash with "Invalid local time for date in time zone"
Comment 1 Fridolin Somers 2022-03-18 00:40:42 UTC
kohadev-koha@kohadevbox:/kohadevbox/koha$ prove t/db_dependent/Koha/Patrons.t 
t/db_dependent/Koha/Patrons.t .. 7/44 
    #   Failed test 'On invalid "is going to expire" date, the method should not crash with "Invalid local time for date in time zone"'
    #   at t/db_dependent/Koha/Patrons.t line 348.
    #          got: 'Invalid local time for date in time zone: America/Sao_Paulo
    # '
    #     expected: ''
    # Looks like you failed 1 test of 9.

#   Failed test 'is_going_to_expire'
#   at t/db_dependent/Koha/Patrons.t line 353.
Comment 2 Fridolin Somers 2022-03-18 00:41:16 UTC
Test added by Bug 23079
Comment 3 Fridolin Somers 2022-03-18 01:00:03 UTC
Looks like it is Bug 29403
Comment 4 Fridolin Somers 2022-03-18 01:36:25 UTC
Looks like this is caused in Koha/DateUtils.pm dt_from_string() by :
https://git.koha-community.org/Koha-community/Koha/src/commit/9c6741cbe3cb00c724a2b2cf4c0efcc723484e87/Koha/DateUtils.pm#L246

my $floating = 0;
...
$dt->set_time_zone($server_tz) unless ( $date_only || $floating );

Call in Koha::Patron->is_going_to_expire is :
dt_from_string( $self->dateexpiry, undef, 'floating' )

Maybe we need to set $floating = 1 in case tz = 'floating' ?
Comment 5 Jonathan Druart 2022-03-18 08:34:42 UTC
Did you git bisect already?

It's weird that this is only happening on U18.
Comment 6 Fridolin Somers 2022-03-18 08:59:41 UTC
(In reply to Jonathan Druart from comment #5)
> Did you git bisect already?
Oh I don't know how to do that.

> 
> It's weird that this is only happening on U18.
Indeed, maybe a behavior in perl lib that changed, I could not find.
DateTime lib is a Titanic lib.
Comment 7 Victor Grousset/tuxayo 2022-07-22 21:00:58 UTC
It's not 29403, reverting doesn't fix the issue.

Bisecting shows that it's commit Bug 29718: Mock the timezone
Comment 8 Victor Grousset/tuxayo 2022-07-22 21:04:59 UTC
To have the CI still be meaningful for Ubuntu 18.04 (until 2023-04-26) for 22.05 and in the future 22.11, should 29718 be reverted until then? (assuming it's possible)
Comment 9 Mason James 2022-08-04 03:08:37 UTC
(In reply to Victor Grousset/tuxayo from comment #8)
> To have the CI still be meaningful for Ubuntu 18.04 (until 2023-04-26) for
> 22.05 and in the future 22.11, should 29718 be reverted until then?
> (assuming it's possible)

i've updated possible fix here...

  https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29718#c44
Comment 10 Mason James 2022-08-05 11:31:49 UTC
Created attachment 138683 [details] [review]
Bug 30030: add 'invalid expiry date' tests, for every timezone

this patch aims to add 'invalid expiry date' tests, for every timezone

to test...

1/ apply patch

2/ run test
root@kohadevbox:/kohadevbox/koha# prove -v t/db_dependent/Koha/Patrons.t
...
    1..368
    ...
    ok 32 - On invalid "is going to expire" date, the method should not crash: America/Argentina/Buenos_Aires
    ok 33 - On invalid "is going to expire" date, the method should not crash: America/Argentina/Catamarca
    ok 34 - On invalid "is going to expire" date, the method should not crash: America/Argentina/Cordoba
    ok 35 - On invalid "is going to expire" date, the method should not crash: America/Argentina/Jujuy
    ok 36 - On invalid "is going to expire" date, the method should not crash: America/Argentina/La_Rioja
    ok 37 - On invalid "is going to expire" date, the method should not crash: America/Argentina/Mendoza
    ok 38 - On invalid "is going to expire" date, the method should not crash: America/Argentina/Rio_Gallegos
    ok 39 - On invalid "is going to expire" date, the method should not crash: America/Argentina/Salta
    ok 40 - On invalid "is going to expire" date, the method should not crash: America/Argentina/San_Juan
    ok 41 - On invalid "is going to expire" date, the method should not crash: America/Argentina/San_Luis
    ok 42 - On invalid "is going to expire" date, the method should not crash: America/Argentina/Tucuman
    ok 43 - On invalid "is going to expire" date, the method should not crash: America/Argentina/Ushuaia
    ...
All tests successful.
Files=1, Tests=44, 84 wallclock secs ( 0.17 usr  0.07 sys + 58.19 cusr 24.35 csys = 82.78 CPU)
Result: PASS

3/ confirm all 'invalid expiry date' tests pass OK
Comment 11 Joonas Kylmälä 2022-09-05 17:29:29 UTC
Mason, I think the test patch attached doesn't make sense for other timezones as only in Brazil in 2019 the day light saving time was cancelled. Was the patch made so that ubuntu 18 test failure would be prevented somehow?