Bug 14801 - Fix Reserves.t
Summary: Fix Reserves.t
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Hold requests (show other bugs)
Version: master
Hardware: All All
: P5 - low minor (vote)
Assignee: Marcel de Rooy
QA Contact: Testopia
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2015-09-10 14:39 UTC by Marcel de Rooy
Modified: 2016-06-21 21:40 UTC (History)
4 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Trivial patch
Bot Control: ---
When did the bot last check this:
Who signed the patch off:
Text to go in the release notes:


Attachments
Bug 14801: Fix Reserves.t (2.51 KB, patch)
2015-09-10 14:41 UTC, Marcel de Rooy
Details | Diff | Splinter Review
[Signed-off] Bug 14801: Fix Reserves.t (2.61 KB, patch)
2015-09-10 15:53 UTC, Marc Véron
Details | Diff | Splinter Review
[PASSED QA] Bug 14801: Fix Reserves.t (2.66 KB, patch)
2015-09-13 16:30 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 14801: Fix Reserves.t -- Follow-up for ChargeReserveFee (1.45 KB, patch)
2015-09-14 09:22 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 14801: Fix Reserves.t (2.79 KB, patch)
2015-09-14 09:34 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 14801: Fix Reserves.t -- Follow-up for ChargeReserveFee (1.45 KB, patch)
2015-09-14 09:34 UTC, Marcel de Rooy
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Marcel de Rooy 2015-09-10 14:39:38 UTC
Fix this:

ok 58 - OnShelfHoldsAllowed() disallowed
not ok 59 - Bug 14464 - No fines at beginning
ok 60 - Bug 14464 - 1st reserve correctly created
not ok 61 - Bug 14464 - No fines after cancelling reserve with no charge configured
ok 62 - Bug 14464 - 2nd reserve correctly created
not ok 63 - Bug 14464 - No fines after cancelling reserve with no charge desired
Comment 1 Marcel de Rooy 2015-09-10 14:41:16 UTC Comment hidden (obsolete)
Comment 2 Marc Véron 2015-09-10 15:53:44 UTC Comment hidden (obsolete)
Comment 3 Katrin Fischer 2015-09-13 16:30:31 UTC Comment hidden (obsolete)
Comment 4 Marcel de Rooy 2015-09-14 08:49:58 UTC
Still looking a little bit at this one.
Will probably add a tiny follow-up.
Comment 5 Marcel de Rooy 2015-09-14 09:22:21 UTC Comment hidden (obsolete)
Comment 6 Marcel de Rooy 2015-09-14 09:25:07 UTC
Tomas:
I added a followup that resolves the deeper cause of the failing test.
It is fairly trivial :)
But if you want another signoff or so, please change the status accordingly.
Comment 7 Marcel de Rooy 2015-09-14 09:34:32 UTC
Created attachment 42507 [details] [review]
Bug 14801: Fix Reserves.t

Fix the following errors:
not ok 59 - Bug 14464 - No fines at beginning
ok 60 - Bug 14464 - 1st reserve correctly created
not ok 61 - Bug 14464 - No fines after cancelling reserve with no charge configured
ok 62 - Bug 14464 - 2nd reserve correctly created
not ok 63 - Bug 14464 - No fines after cancelling reserve with no charge desired

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Marc Veron <veron@veron.ch>
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Amended the x==0 test with !x || x==0 to include 0.00 and prevent warn.
Comment 8 Marcel de Rooy 2015-09-14 09:34:36 UTC
Created attachment 42508 [details] [review]
Bug 14801: Fix Reserves.t -- Follow-up for ChargeReserveFee

The problem making some tests fail, actually was the unneeded addition
of zero accountline records by ChargeReserveFee, called by AddReserve.
The balance is still zero, but a test like !$var responds differently
when var is 0.00 instead of 0 or undef.

This patch adjusts the test in ChargeReserveFee in order to prevent
adding these records with 0.00.
The first patch that adjusts the tests in Reserves.t is not strictly
needed anymore, but can stay.

Test plan:
[1] Run t/db_dependent/Reserves.t
[2] Run t/db_dependent/Reserves/GetReserveFee.t

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 9 Tomás Cohen Arazi 2015-09-16 13:52:55 UTC
Patches pushed to master.

Thanks Marcel!