Bug 23234

Summary: Circulation.t failing when comparing dates that seem identical
Product: Koha Reporter: Jonathan Druart <jonathan.druart>
Component: Test SuiteAssignee: Jonathan Druart <jonathan.druart>
Status: CLOSED FIXED QA Contact: Martin Renvoize (ashimema) <martin.renvoize>
Severity: major    
Priority: P5 - low CC: fridolin.somers, martin.renvoize
Version: unspecified   
Hardware: All   
OS: All   
Change sponsored?: --- Patch complexity: Trivial patch
Documentation contact: Documentation submission:
Text to go in the release notes:
Version(s) released in:
19.11.00,19.05.05
Circulation function:
Bug Depends on: 22917    
Bug Blocks: 25551    
Attachments: Bug 23234: Mock DateTime->now using the time zone
Bug 23234: Mock DateTime->now using the time zone

Description Jonathan Druart 2019-06-28 18:03:39 UTC
Not sure what is behind this.

kohadev-koha@kohadevbox:/home/vagrant/kohaclone$ date
Fri Jun 28 13:02:16 -05 2019

kohadev-koha@kohadevbox:/home/vagrant/kohaclone$ prove t/db_dependent/Circulation.t

        #   Failed test 'Cannot auto renew, too late - no_auto_renewal_after is inclusive(returned code is auto_too_late)'
        #   at t/db_dependent/Circulation.t line 691.
        #          got: 'auto_too_soon'
        #     expected: 'auto_too_late'
        # Looks like you failed 1 test of 14.

    #   Failed test 'too_late_renewal / no_auto_renewal_after'
    #   at t/db_dependent/Circulation.t line 722.

        #   Failed test 'GetLatestAutoRenewDate should return -5 days if no_auto_renewal_after = 5 and date_due is 10 days before'
        #   at t/db_dependent/Circulation.t line 862.
        #          got: '2019-06-23T18:01:00'
        #     expected: '2019-06-23T18:01:00'

        #   Failed test 'GetLatestAutoRenewDate should return +5 days if no_auto_renewal_after = 15 and date_due is 10 days before'
        #   at t/db_dependent/Circulation.t line 869.
        #          got: '2019-07-03T18:01:00'
        #     expected: '2019-07-03T18:01:00'

        #   Failed test 'GetLatestAutoRenewDate should return +2 days if no_auto_renewal_after_hard_limit is defined and not no_auto_renewal_after'
        #   at t/db_dependent/Circulation.t line 876.
        #          got: '2019-06-30T00:00:00'
        #     expected: '2019-06-30T00:00:00'

        #   Failed test 'GetLatestAutoRenewDate should return +2 days if no_auto_renewal_after_hard_limit is < no_auto_renewal_after'
        #   at t/db_dependent/Circulation.t line 882.
        #          got: '2019-06-30T00:00:00'
        #     expected: '2019-06-30T00:00:00'
        # Looks like you failed 4 tests of 5.

    #   Failed test 'GetLatestAutoRenewDate'
    #   at t/db_dependent/Circulation.t line 887.
    # Looks like you failed 2 tests of 71.

#   Failed test 'CanBookBeRenewed tests'
#   at t/db_dependent/Circulation.t line 998.

    #   Failed test 'AddReturn must have debarred the patron'
    #   at t/db_dependent/Circulation.t line 86.
    #          got: ''
    #     expected: '1'
    # AddReturn returned message $VAR1 = {
    #           'WasReturned' => 1
    #         };

    #   Failed test 'Test at line 1967'
    #   at t/db_dependent/Circulation.t line 90.
    #          got: '0'
    #     expected: '1'

    #   Failed test 'Test at line 1967'
    #   at t/db_dependent/Circulation.t line 93.
    #          got: undef
    #     expected: '2019-07-12'
    # Looks like you failed 3 tests of 21.

#   Failed test 'AddReturn + suspension_chargeperiod'
#   at t/db_dependent/Circulation.t line 1976.
# Looks like you failed 2 tests of 45.
t/db_dependent/Circulation.t .. 
Dubious, test returned 2 (wstat 512, 0x200)
Failed 2/45 subtests 

Test Summary Report
-------------------
t/db_dependent/Circulation.t (Wstat: 512 Tests: 45 Failed: 2)
  Failed tests:  15, 29
  Non-zero exit status: 2
Files=1, Tests=45, 27 wallclock secs ( 0.08 usr  0.01 sys + 19.81 cusr  4.37 csys = 24.27 CPU)
Result: FAIL
Comment 1 Jonathan Druart 2019-10-24 10:31:21 UTC
Created attachment 94652 [details] [review]
Bug 23234: Mock DateTime->now using the time zone

The timezone of the two dt was different, one was UTC, the other one was
my local one.
When we mocked DateTime->now we did not use the time zone, we must use
dt_from_string instead

Test plan:
Make sure your environment has a different time zone than UTC
`prove t/db_dependent/Circulation.t` should fail without this patch
Comment 2 Jonathan Druart 2019-11-04 10:33:45 UTC
Upping severity, it's very annoying for people hitting this one (hard to write or execute those tests).
Comment 3 Martin Renvoize (ashimema) 2019-11-04 17:24:58 UTC
Created attachment 95025 [details] [review]
Bug 23234: Mock DateTime->now using the time zone

The timezone of the two dt was different, one was UTC, the other one was
my local one.
When we mocked DateTime->now we did not use the time zone, we must use
dt_from_string instead

Test plan:
Make sure your environment has a different time zone than UTC
`prove t/db_dependent/Circulation.t` should fail without this patch

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 4 Martin Renvoize (ashimema) 2019-11-04 17:25:28 UTC
An annoying bug that's fixed nicely here..  Going straight for QA
Comment 5 Martin Renvoize (ashimema) 2019-11-04 17:27:17 UTC
Nice work!

Pushed to master for 19.11.00
Comment 6 Fridolin Somers 2019-11-15 09:20:05 UTC
Pushed to 19.05.x for 19.05.05