Attachment 89527 Details for Bug 22877 – Bug 22877: (QA follow-up) Clarify intent of tests and remove one.

[patch] Bug 22877: (QA follow-up) Clarify intent of tests and remove one.

Bug-22877-QA-follow-up-Clarify-intent-of-tests-and.patch (text/plain), 6.23 KB, created by Martin Renvoize (ashimema) on 2019-05-10 08:37:45 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Martin Renvoize (ashimema)

Created: 2019-05-10 08:37:45 UTC

Size: 6.23 KB

patch

obsolete

>From 6f296277d9b230c21121a4fc95fdbf18b8d858f8 Mon Sep 17 00:00:00 2001
>From: Martin Renvoize <martin.renvoize@ptfs-europe.com>
>Date: Thu, 9 May 2019 16:34:09 +0100
>Subject: [PATCH] Bug 22877: (QA follow-up) Clarify intent of tests and remove
> one.
>
>This patch embelishes the existing comments slightly and removes a test
>which had a FIXME, but was in fact just a duplicate of the test directly
>above it.  I beleive it became a duplicate after the subroutine signature
>changed at some point recently.  I also move the bug 22877 specific test
>into a subtest.
>
>Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
>---
> t/db_dependent/Circulation.t | 60 +++++++++++++++++++++---------------
> 1 file changed, 35 insertions(+), 25 deletions(-)
>
>diff --git a/t/db_dependent/Circulation.t b/t/db_dependent/Circulation.t
>index b84b2d20f9..4234bb506a 100755
>--- a/t/db_dependent/Circulation.t
>+++ b/t/db_dependent/Circulation.t
>@@ -1980,7 +1980,7 @@ subtest 'CanBookBeIssued + AutoReturnCheckedOutItems' => sub {
> 
> 
> subtest 'AddReturn | is_overdue' => sub {
>-    plan tests => 8;
>+    plan tests => 7;
> 
>     t::lib::Mocks::mock_preference('CalculateFinesOnReturn', 1);
>     t::lib::Mocks::mock_preference('finesMode', 'production');
>@@ -2039,49 +2039,59 @@ subtest 'AddReturn | is_overdue' => sub {
>     my $ten_days_ago  = dt_from_string->subtract( days => 10 );
>     $patron = Koha::Patrons->find( $patron->{borrowernumber} );
> 
>-    # No date specify, today will be used
>+    # No return date specified, today will be used => 10 days overdue charged
>     AddIssue( $patron->unblessed, $item->{barcode}, $ten_days_ago ); # date due was 10d ago
>     AddReturn( $item->{barcode}, $library->{branchcode} );
>     is( int($patron->account->balance()), 10, 'Patron should have a charge of 10 (10 days x 1)' );
>     Koha::Account::Lines->search({ borrowernumber => $patron->borrowernumber })->delete;
> 
>-    # specify return date 5 days before => no overdue
>+    # specify return date 5 days before => no overdue charged
>     AddIssue( $patron->unblessed, $item->{barcode}, $five_days_ago ); # date due was 5d ago
>     AddReturn( $item->{barcode}, $library->{branchcode}, undef, $ten_days_ago );
>     is( int($patron->account->balance()), 0, 'AddReturn: pass return_date => no overdue' );
>     Koha::Account::Lines->search({ borrowernumber => $patron->borrowernumber })->delete;
> 
>-    # specify return date 5 days later => overdue
>+    # specify return date 5 days later => 5 days overdue charged
>     AddIssue( $patron->unblessed, $item->{barcode}, $ten_days_ago ); # date due was 10d ago
>     AddReturn( $item->{barcode}, $library->{branchcode}, undef, $five_days_ago );
>     is( int($patron->account->balance()), 5, 'AddReturn: pass return_date => overdue' );
>     Koha::Account::Lines->search({ borrowernumber => $patron->borrowernumber })->delete;
> 
>-    # specify dropbox date 5 days before => no overdue
>-    AddIssue( $patron->unblessed, $item->{barcode}, $five_days_ago ); # date due was 5d ago
>-    AddReturn( $item->{barcode}, $library->{branchcode}, $ten_days_ago );
>-    is( int($patron->account->balance()), 0, 'AddReturn: pass return_date => no overdue' );
>-    Koha::Account::Lines->search({ borrowernumber => $patron->borrowernumber })->delete;
>-
>-    # specify dropbox date 5 days later => overdue, or... not
>+    # specify return date 5 days later, specify exemptfine => no overdue charge
>     AddIssue( $patron->unblessed, $item->{barcode}, $ten_days_ago ); # date due was 10d ago
>-    AddReturn( $item->{barcode}, $library->{branchcode}, $five_days_ago );
>-    is( int($patron->account->balance()), 0, 'AddReturn: pass return_date => no overdue in dropbox mode' ); # FIXME? This is weird, the OVERDUE fine is created ( _CalculateAndUpdateFine > C4::Overdues::UpdateFine ) then remove later (in _FixOverduesOnReturn). Looks like it is a feature
>+    AddReturn( $item->{barcode}, $library->{branchcode}, 1, $five_days_ago );
>+    is( int($patron->account->balance()), 0, 'AddReturn: pass return_date => no overdue' );
>     Koha::Account::Lines->search({ borrowernumber => $patron->borrowernumber })->delete;
> 
>-    # Checkout an item 10 days ago
>-    my $issue = AddIssue( $patron->unblessed, $item->{barcode}, $ten_days_ago ); # date due was 10d ago
>-    # Fake fines cronjob on this checkout
>-    my ( $fine ) = CalcFine( $item , $patron->categorycode, $library->{branchcode}, $ten_days_ago, $now );
>-    UpdateFine({ issue_id => $issue->issue_id, itemnumber => $item->{itemnumber}, borrowernumber => $patron->borrowernumber, amount => $fine, due => output_pref($ten_days_ago) });
>-    is( int($patron->account->balance()),10, "Overdue fine of 10 days overdue");
>-    # Fake longoverdue with charge and not marking returned
>-    LostItem( $item->{itemnumber}, 'cronjob',0 );
>-    is( int($patron->account->balance()),17, "Lost fine of 7 plus 10 days overdue");
>-    # Now we return it today
>-    AddReturn( $item->{barcode}, $library->{branchcode} );
>-    is( int($patron->account->balance()),17, "Should have a single 10 days overdue fine and lost charge");
>+    subtest 'bug 22877' => sub {
>+        my $issue = AddIssue( $patron->unblessed, $item->{barcode}, $ten_days_ago );    # date due was 10d ago
> 
>+        # Fake fines cronjob on this checkout
>+        my ($fine) =
>+          CalcFine( $item, $patron->categorycode, $library->{branchcode},
>+            $ten_days_ago, $now );
>+        UpdateFine(
>+            {
>+                issue_id       => $issue->issue_id,
>+                itemnumber     => $item->{itemnumber},
>+                borrowernumber => $patron->borrowernumber,
>+                amount         => $fine,
>+                due            => output_pref($ten_days_ago)
>+            }
>+        );
>+        is( int( $patron->account->balance() ),
>+            10, "Overdue fine of 10 days overdue" );
>+
>+        # Fake longoverdue with charge and not marking returned
>+        LostItem( $item->{itemnumber}, 'cronjob', 0 );
>+        is( int( $patron->account->balance() ),
>+            17, "Lost fine of 7 plus 10 days overdue" );
>+
>+        # Now we return it today
>+        AddReturn( $item->{barcode}, $library->{branchcode} );
>+        is( int( $patron->account->balance() ),
>+            17, "Should have a single 10 days overdue fine and lost charge" );
>+      }
> };
> 
> subtest '_FixAccountForLostAndReturned' => sub {
>-- 
>2.20.1

Actions: View | Diff | Splinter Review

Attachments on bug 22877: 89505 | 89507 | 89508 | 89509 | 89510 | 89511 | 89512 | 89513 | 89514 | 89516 | 89527 | 89818 | 89819 | 89820 | 89821 | 89858 | 89859 | 89860 | 89861 | 90210 | 90211 | 90212 | 90213