Bug 12457

Summary: The module C4::Members.pm needs more unit tests
Product: Koha Reporter: Yohann Dufour <yohann.dufour>
Component: Test SuiteAssignee: Yohann Dufour <yohann.dufour>
Status: CLOSED FIXED QA Contact: Galen Charlton <gmcharlt>
Severity: normal    
Priority: P5 - low CC: bgkriegel, jonathan.druart, tomascohen
Version: Main   
Hardware: All   
OS: All   
Change sponsored?: --- Patch complexity: Small patch
Documentation contact: Documentation submission:
Text to go in the release notes:
Version(s) released in:
Circulation function:
Bug Depends on:    
Bug Blocks: 12633    
Attachments: Bug 12457: Removal warning in t/db_dependent/Members.t
Bug 12457: Adding unit tests for the routines AddMessage, GetMessages, GetMessagesCount, DeleteMessage, GetPendingIssues, GetAllIssues of the module C4/Members.pm and modification of the routine GetAllIssues
Bug 12457: Adding unit tests for Members.pm
Bug 12457: Allow UT on a non empty database
Bug 12457: Removal warning in t/db_dependent/Members.t
Bug 12457: Adding unit tests for Members.pm
Bug 12457: Allow UT on a non empty database
Bug 12457: Allow UT on a non empty database
Bug 12457: Allow UT on a non empty database
[PASSED QA] Bug 12457: Removal warning in t/db_dependent/Members.t
[PASSED QA] Bug 12457: Adding unit tests for Members.pm
[PASSED QA] Bug 12457: Allow UT on a non empty database

Description Yohann Dufour 2014-06-20 11:29:06 UTC
The routines GetPendingIssues, GetAllIssues, GetMessagesCount, GetGuarantees, HasOverdues and GetExpiryDate of the module C4::Members.pm are not tested.
Comment 1 Yohann Dufour 2014-06-20 12:39:36 UTC Comment hidden (obsolete)
Comment 2 Yohann Dufour 2014-06-23 14:17:07 UTC Comment hidden (obsolete)
Comment 3 Bernardo Gonzalez Kriegel 2014-07-09 15:59:22 UTC
Second patch does not apply

fatal: sha1 information is lacking or useless (t/db_dependent/Members.t).
Repository lacks necessary blobs to fall back on 3-way merge.
Cannot fall back to three-way merge.
Patch failed at 0001 Bug 12457: Adding unit tests for the routines AddMessage, GetMessages, GetMessagesCount, DeleteMessage, GetPendingIssues, GetAllIssues of the module C4/Members.pm and modification of the routine GetAllIssues
Comment 4 Yohann Dufour 2014-07-16 10:21:46 UTC Comment hidden (obsolete)
Comment 5 Bernardo Gonzalez Kriegel 2014-07-17 13:32:14 UTC
First patch Ok, test pass
On second patch, first test pass, but the other two fail :(

t/db_dependent/Members.t ................... ok     
t/db_dependent/Members/GetAllIssues.t ...... DBD::mysql::db do failed: Cannot delete or update a parent row: a foreign key constraint fails (`kohadev`.`items`, CONSTRAINT `items_ibfk_2` FOREIGN KEY (`homebranch`) REFERENCES `branches` (`branchcode`) ON UPDATE CASCADE) at t/db_dependent/Members/GetAllIssues.t line 22.
DBD::mysql::db do failed: Cannot delete or update a parent row: a foreign key constraint fails (`kohadev`.`items`, CONSTRAINT `items_ibfk_2` FOREIGN KEY (`homebranch`) REFERENCES `branches` (`branchcode`) ON UPDATE CASCADE) at t/db_dependent/Members/GetAllIssues.t line 22.
# Looks like your test exited with 255 before it could output anything.
t/db_dependent/Members/GetAllIssues.t ...... Dubious, test returned 255 (wstat 65280, 0xff00)
Failed 16/16 subtests 
t/db_dependent/Members/GetPendingIssues.t .. DBD::mysql::db do failed: Cannot delete or update a parent row: a foreign key constraint fails (`kohadev`.`items`, CONSTRAINT `items_ibfk_2` FOREIGN KEY (`homebranch`) REFERENCES `branches` (`branchcode`) ON UPDATE CASCADE) at t/db_dependent/Members/GetPendingIssues.t line 22.
DBD::mysql::db do failed: Cannot delete or update a parent row: a foreign key constraint fails (`kohadev`.`items`, CONSTRAINT `items_ibfk_2` FOREIGN KEY (`homebranch`) REFERENCES `branches` (`branchcode`) ON UPDATE CASCADE) at t/db_dependent/Members/GetPendingIssues.t line 22.
# Looks like your test exited with 255 before it could output anything.
t/db_dependent/Members/GetPendingIssues.t .. Dubious, test returned 255 (wstat 65280, 0xff00)
Failed 20/20 subtest
Comment 6 Yohann Dufour 2014-07-21 07:28:47 UTC Comment hidden (obsolete)
Comment 7 Jonathan Druart 2014-07-25 14:24:38 UTC Comment hidden (obsolete)
Comment 8 Jonathan Druart 2014-07-25 14:24:43 UTC Comment hidden (obsolete)
Comment 9 Jonathan Druart 2014-07-25 14:24:47 UTC Comment hidden (obsolete)
Comment 10 Jonathan Druart 2014-07-25 14:25:58 UTC
I SO on it because these patches only add unit tests and should be integrate asap.
Comment 11 Katrin Fischer 2014-07-27 22:48:39 UTC
I am sorry, but GetPendingIssues.t still fails for me:

$ perl t/db_dependent/Members/GetPendingIssues.t 
1..20
DBD::mysql::db do failed: Cannot delete or update a parent row: a foreign key constraint fails (`koha`.`items`, CONSTRAINT `items_ibfk_3` FOREIGN KEY (`holdingbranch`) REFERENCES `branches` (`branchcode`) ON UPDATE CASCADE) at t/db_dependent/Members/GetPendingIssues.t line 21.
DBD::mysql::db do failed: Cannot delete or update a parent row: a foreign key constraint fails (`koha`.`items`, CONSTRAINT `items_ibfk_3` FOREIGN KEY (`holdingbranch`) REFERENCES `branches` (`branchcode`) ON UPDATE CASCADE) at t/db_dependent/Members/GetPendingIssues.t line 21.
# Looks like your test exited with 255 before it could output anything.
Comment 12 Yohann Dufour 2014-07-28 07:34:21 UTC Comment hidden (obsolete)
Comment 13 Katrin Fischer 2014-07-28 20:55:55 UTC
I am sorry Yohann, but this still fails for me:

 $ perl t/db_dependent/Members/GetAllIssues.t 
1..16
DBD::mysql::db do failed: Cannot delete or update a parent row: a foreign key constraint fails (`koha`.`issues`, CONSTRAINT `issues_ibfk_1` FOREIGN KEY (`borrowernumber`) REFERENCES `borrowers` (`borrowernumber`) ON UPDATE CASCADE) at t/db_dependent/Members/GetAllIssues.t line 20.
DBD::mysql::db do failed: Cannot delete or update a parent row: a foreign key constraint fails (`koha`.`issues`, CONSTRAINT `issues_ibfk_1` FOREIGN KEY (`borrowernumber`) REFERENCES `borrowers` (`borrowernumber`) ON UPDATE CASCADE) at t/db_dependent/Members/GetAllIssues.t line 20.
# Looks like your test exited with 255 before it could output anything.
Comment 14 Yohann Dufour 2014-07-29 08:47:41 UTC Comment hidden (obsolete)
Comment 15 Katrin Fischer 2014-07-29 21:09:50 UTC
Created attachment 30309 [details] [review]
[PASSED QA] Bug 12457: Removal warning in t/db_dependent/Members.t

When I execute prove t/db_dependent/Members.t, this warning appears twice :
"Argument "Test" isn't numeric in modulus (%) at /home/yohann/koha/C4/Context.pm line 1256."

The warning is due to a invalid use of the routine set_userenv

Test plan:
1/ Execute : prove t/db_dependent/Members.t
2/ The warning appears twice :
"Argument "Test" isn't numeric in modulus (%) at /home/yohann/koha/C4/Context.pm line 1256."
3/ Apply the patch
4/ Execute : prove t/db_dependent/Members.t
5/ The result has to be a success without error or warning :
t/db_dependent/Members.t .. ok
All tests successful.
Files=1, Tests=26,  2 wallclock secs ( 0.04 usr  0.01 sys +  1.50 cusr  0.08 csys =  1.63 CPU)
Result: PASS

Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de>
All new and old tests pass now.
Comment 16 Katrin Fischer 2014-07-29 21:10:09 UTC
Created attachment 30310 [details] [review]
[PASSED QA] Bug 12457: Adding unit tests for Members.pm

Adding unit tests for the routines AddMessage, GetMessages, GetMessagesCount and DeleteMessage in t/db_dependent/Members.t
Adding unit tests for the routines GetPendingIssues and GetAllIssues in separate files : t/db_dependent/Members/GetPendingIssues.t and t/db_dependent/Members/GetAllIssues.t
The routine GetAllIssues has been modified because it does not test if the arguments was defined :
- the borrowernumber argument is required
- if the order argument is not given, it takes a value by default : 'date_due desc'
- the limit argument is optional

Test plan:
1/ Apply the patch
2/ Execute : prove t/db_dependent/Members.t t/db_dependent/Members/GetAllIssues.t t/db_dependent/Members/GetPendingIssues.t
3/ The result has to be a success without error or warning :
t/db_dependent/Members.t ................... ok
t/db_dependent/Members/GetAllIssues.t ...... ok
t/db_dependent/Members/GetPendingIssues.t .. ok
All tests successful.
Files=3, Tests=83,  5 wallclock secs ( 0.06 usr  0.01 sys +  4.68 cusr  0.26 csys =  5.01 CPU)
Result: PASS

Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Amended patch: perltidy on t/db_dependent/Members/*

Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de>
Comment 17 Katrin Fischer 2014-07-29 21:10:45 UTC
Created attachment 30311 [details] [review]
[PASSED QA] Bug 12457: Allow UT on a non empty database

Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de>
Comment 18 Tomás Cohen Arazi (tcohen) 2014-07-30 14:54:33 UTC
Patches pushed to master.

Thanks Yohann!