Bug 12427 - C4::Members::Attributes.pm needs unit tests
Summary: C4::Members::Attributes.pm needs unit tests
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Test Suite (show other bugs)
Version: unspecified
Hardware: All All
: P5 - low normal (vote)
Assignee: Yohann Dufour
QA Contact: Galen Charlton
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2014-06-16 12:32 UTC by Yohann Dufour
Modified: 2015-06-04 23:30 UTC (History)
5 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 12427: adding unit tests of module C4::Members::Attributes.pm (1.47 KB, patch)
2014-06-18 07:06 UTC, Yohann Dufour
Details | Diff | Splinter Review
Bug 12427: adding unit tests of module C4::Members::Attributes.pm (13.30 KB, patch)
2014-06-18 07:16 UTC, Yohann Dufour
Details | Diff | Splinter Review
Bug 12427: adding unit tests of module C4::Members::Attributes.pm (1.47 KB, patch)
2014-06-18 07:24 UTC, Yohann Dufour
Details | Diff | Splinter Review
Bug 12427: adding unit tests of module C4::Members::Attributes.pm (13.30 KB, patch)
2014-06-19 11:51 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 12427: Allow execution of UTs on a DB with issues (796 bytes, patch)
2014-06-19 11:51 UTC, Jonathan Druart
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 12427: adding unit tests of module C4::Members::Attributes.pm (13.47 KB, patch)
2014-06-23 00:28 UTC, Bernardo Gonzalez Kriegel
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 12427: Allow execution of UTs on a DB with issues (869 bytes, patch)
2014-06-23 00:29 UTC, Bernardo Gonzalez Kriegel
Details | Diff | Splinter Review
[PASSED QA] Bug 12427: adding unit tests of module C4::Members::Attributes.pm (13.53 KB, patch)
2014-06-24 04:44 UTC, Katrin Fischer
Details | Diff | Splinter Review
[PASSED QA] Bug 12427: Allow execution of UTs on a DB with issues (925 bytes, patch)
2014-06-24 04:44 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-16 12:32:20 UTC
The module C4::Members::Attributes.pm is not tested
Comment 1 Yohann Dufour 2014-06-18 07:06:56 UTC Comment hidden (obsolete)
Comment 2 Yohann Dufour 2014-06-18 07:16:40 UTC Comment hidden (obsolete)
Comment 3 Yohann Dufour 2014-06-18 07:24:21 UTC Comment hidden (obsolete)
Comment 4 Chris Cormack 2014-06-18 20:31:19 UTC
DBD::mysql::db do failed: Cannot delete or update a parent row: a foreign key constraint fails (`koha_test`.`issues`, CONSTRAINT `issues_ibfk_1` FOREIGN KEY (`borrowernumber`) REFERENCES `borrowers` (`borrowernumber`) ON UPDATE CASCADE) at t/db_dependent/Members_Attributes.t line 34.
DBD::mysql::db do failed: Cannot delete or update a parent row: a foreign key constraint fails (`koha_test`.`issues`, CONSTRAINT `issues_ibfk_1` FOREIGN KEY (`borrowernumber`) REFERENCES `borrowers` (`borrowernumber`) ON UPDATE CASCADE) at t/db_dependent/Members_Attributes.t line 34.
# Looks like you planned 60 tests but ran 1.
# Looks like your test exited with 255 just after 1.


Fails on a non empty db, (ie if you have any borrowers who have attributes in the database).

Do we care about that? Or do we only care that it passes on an empty (or near empty) database?
Comment 5 Jonathan Druart 2014-06-19 08:15:56 UTC
(In reply to Chris Cormack from comment #4)
> Do we care about that? Or do we only care that it passes on an empty (or
> near empty) database?

Hi Chris,
I recommend to Yohann to execute the unit tests with the DB generated by the patch on bug 10337.
I have been doing this for 1 year now. All db_dependent tests pass using this DB (except the 3 broken currently).
Comment 6 Katrin Fischer 2014-06-19 11:17:42 UTC
I feel it would be nice if the unit tests could be run on any db - maybe the patch could be improved a little for the case Chris found?
Comment 7 Jonathan Druart 2014-06-19 11:31:34 UTC
Looking quickly at this patch, the only dependencies are 
+    categorycode => 'S',
+    branchcode => 'CPL',

This is the case for almost ALL db dependent unit tests.

Some tests are very painful to write (for instance, try to write a unit tests in the ACQ module...).
It is good not to have to add all data before starting to test something.
Comment 8 Jonathan Druart 2014-06-19 11:35:41 UTC
However, I think this discussion should have done on bug 10337 (in discussion for 1 year ago).
Comment 9 Katrin Fischer 2014-06-19 11:39:57 UTC
You are right, that this is not the right place for a general discussion. I was referring to this comment from Chris:

>Fails on a non empty db, (ie if you have any borrowers who have attributes in >the database

Maybe it would be enough then to add another delete line to make sure there are no other borrowers with patron attributes?
Comment 10 Jonathan Druart 2014-06-19 11:51:53 UTC Comment hidden (obsolete)
Comment 11 Jonathan Druart 2014-06-19 11:51:59 UTC Comment hidden (obsolete)
Comment 12 Jonathan Druart 2014-06-19 11:53:01 UTC
(In reply to Katrin Fischer from comment #9)
> Maybe it would be enough then to add another delete line to make sure there
> are no other borrowers with patron attributes?

Agreed, the deletion failed if the DB contains issues.
I squashed the 2 first patches and added a follow-up.
Comment 13 Bernardo Gonzalez Kriegel 2014-06-23 00:28:58 UTC Comment hidden (obsolete)
Comment 14 Bernardo Gonzalez Kriegel 2014-06-23 00:29:05 UTC Comment hidden (obsolete)
Comment 15 Katrin Fischer 2014-06-24 04:44:04 UTC
Created attachment 29185 [details] [review]
[PASSED QA] Bug 12427: adding unit tests of module C4::Members::Attributes.pm

The subroutines UpdateBorrowerAttribute, SearchIdMatchingAttribute, DeleteBorrowerAttribute, GetBorrowerAttributes, GetAttributes, GetBorrowerAttributeValue, SetBorrowerAttributes and CheckUniqueness of the module C4::Members::Attributes.pm were not tested.

Test plan:
1/ Execute the command: prove t/db_dependent/Members_Attributes.t
2/ The result has to be:
t/db_dependent/Members_Attributes.t .. ok
All tests successful.
Files=1, Tests=60,  2 wallclock secs ( 0.04 usr  0.01 sys +  1.45 cusr  0.08 csys =  1.58 CPU)
Result: PASS

Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com>
Test with patrons on db having issues and attributes
New test file, all test pass
No koha-qa errors

Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de>
Comment 16 Katrin Fischer 2014-06-24 04:44:17 UTC
Created attachment 29186 [details] [review]
[PASSED QA] Bug 12427: Allow execution of UTs on a DB with issues

Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com>
Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de>
Comment 17 Tomás Cohen Arazi 2014-06-24 12:45:37 UTC
Patches pushed to master.

Thanks Yohann
Comment 18 Galen Charlton 2014-06-25 22:36:43 UTC
Pushed to 3.16.x for inclusion in 3.16.1.