Bug 12457 - The module C4::Members.pm needs more unit tests
Summary: The module C4::Members.pm needs more unit tests
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Test Suite (show other bugs)
Version: master
Hardware: All All
: P5 - low normal (vote)
Assignee: Yohann Dufour
QA Contact: Galen Charlton
URL:
Keywords:
Depends on:
Blocks: 12633
  Show dependency treegraph
 
Reported: 2014-06-20 11:29 UTC by Yohann Dufour
Modified: 2015-06-04 23:33 UTC (History)
4 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Small patch
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:


Attachments
Bug 12457: Removal warning in t/db_dependent/Members.t (1.97 KB, patch)
2014-06-20 12:39 UTC, Yohann Dufour
Details | Diff | Splinter Review
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 (13.92 KB, patch)
2014-06-23 14:17 UTC, Yohann Dufour
Details | Diff | Splinter Review
Bug 12457: Adding unit tests for Members.pm (13.85 KB, patch)
2014-07-16 10:21 UTC, Yohann Dufour
Details | Diff | Splinter Review
Bug 12457: Allow UT on a non empty database (817 bytes, patch)
2014-07-21 07:28 UTC, Yohann Dufour
Details | Diff | Splinter Review
Bug 12457: Removal warning in t/db_dependent/Members.t (2.04 KB, patch)
2014-07-25 14:24 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 12457: Adding unit tests for Members.pm (14.13 KB, patch)
2014-07-25 14:24 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 12457: Allow UT on a non empty database (904 bytes, patch)
2014-07-25 14:24 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 12457: Allow UT on a non empty database (1.45 KB, patch)
2014-07-28 07:34 UTC, Yohann Dufour
Details | Diff | Splinter Review
Bug 12457: Allow UT on a non empty database (1.58 KB, patch)
2014-07-29 08:47 UTC, Yohann Dufour
Details | Diff | Splinter Review
[PASSED QA] Bug 12457: Removal warning in t/db_dependent/Members.t (2.13 KB, patch)
2014-07-29 21:09 UTC, Katrin Fischer
Details | Diff | Splinter Review
[PASSED QA] Bug 12457: Adding unit tests for Members.pm (14.19 KB, patch)
2014-07-29 21:10 UTC, Katrin Fischer
Details | Diff | Splinter Review
[PASSED QA] Bug 12457: Allow UT on a non empty database (1.65 KB, patch)
2014-07-29 21:10 UTC, Katrin Fischer
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
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 2014-07-30 14:54:33 UTC
Patches pushed to master.

Thanks Yohann!