We need more unit tests!
Created attachment 2716 [details] [review] adds some basic unit tests
Created attachment 2726 [details] [review] more basic unit tests to C4 modules
(In reply to comment #2) > Created attachment 2726 [details] [review] > more basic unit tests to C4 modules I think that these tests fails if Perl module "Cache::Memcached::Fast" is not in Perl path, but this dependency is not referenced in file "C4/Installer/PerlDependencies.pm". It will probably be useful to add "Cache::Memcached::Fast" to the list of dependencies in "C4/Installer/PerlDependencies.pm" with a minimal version and probably an optional status.
Actually, I plan to remove the dependency of Cache::Memcached::Fast, it isn't being used by anything in Koha, and there is no immediate plans to make it be. I will create another bug for that and remove it.
This bug is mentioned in: Bug 5327: Add basic unit tests to some C4 modules http://lists.koha-community.org/pipermail/koha-patches/2010-October/012777.html Bug 5327: Add basic unit tests to some C4 modules http://lists.koha-community.org/pipermail/koha-patches/2010-October/012781.html Bug 5327: Add basic unit tests to some C4 modules http://lists.koha-community.org/pipermail/koha-patches/2010-October/012782.html Bug 5327: Add basic unit tests to some C4 modules http://lists.koha-community.org/pipermail/koha-patches/2010-October/012784.html Bug 5327: Add unit test for ItemType.pm http://lists.koha-community.org/pipermail/koha-patches/2010-October/012810.html Bug 5327: More basic unit tests for C4 modules http://lists.koha-community.org/pipermail/koha-patches/2010-October/012813.html Bug 5327: More basic unit tests for C4 modules http://lists.koha-community.org/pipermail/koha-patches/2010-October/012816.html bug 5327: follow-up to fix SIP tests http://lists.koha-community.org/pipermail/koha-patches/2010-October/012815.html
This bug has a patch presently committed to HEAD and 3.2.x. Please take the appropriate action to update the status of this bug. Remember, bugs which have been resolved should be tested and marked closed, preferably by the original reporter.
At the moment all C4 modules have a basic unit test. (use) More tests are still required.
This bug has a fix which was committed to the 3.2.x branch and released in 3.2.1. Please take time to update the status of this bug and mark it RESOLVED FIXED so that it may be closed.
This bug is mentioned in: Bug 5327 - New Unit Test http://lists.koha-community.org/pipermail/koha-patches/2011-January/013606.html patch bug 5327 http://lists.koha-community.org/pipermail/koha-patches/2011-January/013616.html Patch Bug 5327 http://lists.koha-community.org/pipermail/koha-patches/2011-January/013617.html new patche bug 5327 http://lists.koha-community.org/pipermail/koha-patches/2011-January/013618.html Bug 5327 - More Unit Tests http://lists.koha-community.org/pipermail/koha-patches/2011-January/013619.html Bug 5327 - Even More Unit Tests http://lists.koha-community.org/pipermail/koha-patches/2011-January/013621.html Bug 5327 Record.t http://lists.koha-community.org/pipermail/koha-patches/2011-January/013623.html Bug 5327 Record.t http://lists.koha-community.org/pipermail/koha-patches/2011-January/013626.html Bug 5327 - Amazon unit tests http://lists.koha-community.org/pipermail/koha-patches/2011-January/013628.html Bug 5327 Accounts test file made http://lists.koha-community.org/pipermail/koha-patches/2011-January/013674.html Bug 5327 - Output_JSONStream.t http://lists.koha-community.org/pipermail/koha-patches/2011-January/013687.html Bug 5327 - Output_JSONStream.t http://lists.koha-community.org/pipermail/koha-patches/2011-January/013689.html
Created attachment 5485 [details] [review] fix whitespace in the test
(In reply to comment #10) > Created attachment 5485 [details] [review] > fix whitespace in the test I meant in the t/db_dependent/Record.t test
perl t/db_dependent/Record.t 1..10 ok 1 - use C4::Record; ok 2 - error works DBD::mysql::db do failed: Unknown or incorrect time zone: 'Pacific/Auckland' at C4/Context.pm line 700, <DATA> line 496. ok 3 - testing marc2xml ok 4 - testing marc2xml ok 5 - testing xml2marc ok 6 - testing marc2dcxml ok 7 - testing marcQualified not ok 8 - testing marc2mosxml # Failed test 'testing marc2mosxml' # at t/db_dependent/Record.t line 85. # got: '<?xml version="1.0" encoding="UTF-8"?> # <mods xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xmlns="http://www.loc.gov/mods/v3" version="3.1" xsi:schemaLocation="http://www.loc.gov/mods/v3 http://www.loc.gov/standards/mods/v3/mods-3-1.xsd"> # <typeOfResource/> # <originInfo> # <issuance/> # </originInfo> # <recordInfo/> # </mods> # ' # expected: '<?xml version="1.0" encoding="UTF-8"?> # <mods xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xmlns="http://www.loc.gov/mods/v3" version="3.1" xsi:schemaLocation="http://www.loc.gov/mods/v3 http://www.loc.gov/standards/mods/v3/mods-3-1.xsd"> # <typeOfResource/> # <originInfo> # <issuance/> # </originInfo> # <recordInfo/> # </mods> # ' ok 9 - testing bibtex ok 10 - Html umlauts # Looks like you failed 1 test of 10 If i revert the patch, the test passes
Created attachment 6236 [details] [review] Bug 5327 : Adding more unit tests
Created attachment 6237 [details] [review] Bug 5327 : Adding more unit tests
Created attachment 6373 [details] [review] Adds missing case for boolean unit tests, adds in actual unit tests for barkertaylor Hopefully this will bump boolean's conditional coverage up to 100% and should also improve the coverage of BarkerTaylor's significantly.
Created attachment 6560 [details] [review] Bug 5327: Unit tests for C4/Exteral/BakerTaylor.pm Signed-off-by: Chris Cormack <chrisc@catalyst.net.nz>
Created attachment 6561 [details] [review] Bug 5327 : Complete coverage for BackgroundJob.pm
Created attachment 6562 [details] [review] [SIGNED-OFF] Bug 5327 : Adding more unit tests Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de> New tests run without problems.
Created attachment 6563 [details] [review] [SIGNED-OFF] Bug 5327 : Complete coverage for BackgroundJob.pm Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de> Tests run without problems.
Chris-es, I tried to apply those patches on the branch 5327, but one of them does not apply anymore: Bug 5327: Unit tests for C4/Exteral/BakerTaylor.pm Apply? [yn] y Applying: Bug 5327: Unit tests for C4/Exteral/BakerTaylor.pm fatal: sha1 information is lacking or useless (t/Boolean.t). Repository lacks necessary blobs to fall back on 3-way merge. Cannot fall back to three-way merge. Patch failed at 0001 Bug 5327: Unit tests for C4/Exteral/BakerTaylor.pm When you have resolved this problem run "git am --resolved". If you would prefer to skip this patch, instead run "git am --skip". To restore the original branch and stop patching run "git am --abort". Patch left in /tmp/Bug-5327-Unit-tests-for-C4ExteralBakerTaylorpm-gQ0UGT.patch Could you rebase against the 5327 branch and update the patch please ?
Hi Paul, all patches apply for me using 'git bz apply'?
(In reply to comment #21) > Hi Paul, all patches apply for me using 'git bz apply'? Katrin, do you have noticed i'm trying to apply them not from master, but from origin/new/bug_5327 ? (after doing a git checkout --track origin/new/bug_5327)
Paul you will want to merge master into your 5327 branch, then apply them. That branch is old so will be out of date with master. We don't base patches against the branches, but against master. So if a branch is old, it needs to be brought up to date, don't rebase though, a simple merge will bring it up to master level.
Created attachment 6613 [details] [review] Bug 5327 : Fixing unit test for SQLHelper.pm
Created attachment 6614 [details] [review] Bug 5327 : Fixing unit test for SQLHelper.pm
Created attachment 6616 [details] [review] Bug 5327: Fixing the Members.t test
2 more requiring sign off
Created attachment 6620 [details] [review] Bug 5327: Fixing the Members.t test Signed-off-by: Liz Rea <wizzyrea@gmail.com> Confirm all tests complete
That is NOT AT ALL RIGHT. I had to change some of the incidental data in the test to run it - and that's what that was. My dumb. signoff inc.
Created attachment 6621 [details] [review] Bug 5327 : Fixing unit test for SQLHelper.pm Signed-off-by: Liz Rea <wizzyrea@gmail.com> Confirm that all SQLHelper tests pass.
Created attachment 6622 [details] [review] [Signed off] Bug 5327: Fixing the Members.t test Signed-off-by: Liz Rea <wizzyrea@gmail.com> Confirm Members.t tests complete
Comment on attachment 5485 [details] [review] fix whitespace in the test this patch pushed today
Comment on attachment 6560 [details] [review] Bug 5327: Unit tests for C4/Exteral/BakerTaylor.pm This patch pushed today
Comment on attachment 6562 [details] [review] [SIGNED-OFF] Bug 5327 : Adding more unit tests This patch pushed today, with a small follow-up to initial comment
Comment on attachment 6563 [details] [review] [SIGNED-OFF] Bug 5327 : Complete coverage for BackgroundJob.pm This patch pushed today
Comment on attachment 6621 [details] [review] Bug 5327 : Fixing unit test for SQLHelper.pm This patch pushed today, HOWEVER: Chris, could you explain why you removed the SearchInTable tests and replace it by nothing ? I see it fixes the test, but why are those 2 tests irrelevant now ?
Comment on attachment 6622 [details] [review] [Signed off] Bug 5327: Fixing the Members.t test Patch pushed today HOWEVER: Chris, could you explain why BRANCH_IDX now need to be 4 instead of 5 ?
Created attachment 6643 [details] [review] Patch to increase boolean.t coverage I think my previous patch to Boolean.t got lost in the works, I forgot to change the count so ChrisC corrected it for me in his patch, but my patch wasnt applied, so the number of tests was invalid, Paul decreased the counter so the test would pass. Attached is patch to correct Boolean.t (both the missing case and the count).
Created attachment 6650 [details] [review] Bug 5327 - fixing t/db_dependent/Record.t to not fail because of whitespace. To test: run prove -v t/db_dependent/Record.t The test should complete successfully. I should note - if this test was actually testing the whitespace and the problem with the output was valid, please go ahead and fix that problem and disregard this patch. To me, the generated output looked better than the expected result, so I made them match.
Comment on attachment 6650 [details] [review] Bug 5327 - fixing t/db_dependent/Record.t to not fail because of whitespace. keke, ChrisC did this exact same thing *and I even signed off on it.* This attachment is hereby invalid. ;)
Created attachment 6655 [details] [review] Unit test for SIP/Sip/Checksum.pm
Created attachment 6679 [details] [review] Test for AuthoritiesMarc/UNIMARC.pm
(In reply to comment #36) > Comment on attachment 6621 [details] [review] > Bug 5327 : Fixing unit test for SQLHelper.pm > > This patch pushed today, HOWEVER: > > Chris, could you explain why you removed the SearchInTable tests and replace it > by nothing ? > I see it fixes the test, but why are those 2 tests irrelevant now ? Because the SearchInTable function has changed, it does OR, not AND, those tests will never ever pass
(In reply to comment #37) > Comment on attachment 6622 [details] [review] > [Signed off] Bug 5327: Fixing the Members.t test > > Patch pushed today > > HOWEVER: > Chris, could you explain why BRANCH_IDX now need to be 4 instead of 5 ? Because the branchcode is the 5th (not 6th) element in the array (starts at 0 of course) These two fixes were me fixing tests that others had broken, so hopefully in future I won't be doing this :)
Created attachment 6681 [details] [review] Added unit test for Scrubber.pm
Created attachment 6682 [details] [review] Patch removes unneeded self=shift from TTParser::unshift_token While Duncan was creating unit tests for TTParser I found an unneeded 'my $self=shift' within TTParser::unshift_token, however ::peep_token and ::next_token do not take a $self argument. No client code uses this so it shouldn't break anything. ~Chris
Created attachment 6683 [details] [review] Added test coverage for TTParser
(In reply to comment #48) > Created attachment 6683 [details] [review] > Added test coverage for TTParser Meant to add this on the attachment, this test relies on the patch submitted by Chris in comment 47.
Created attachment 6726 [details] [review] Added missing reference case to Boolean.t and corrected count
Created attachment 6727 [details] [review] [SIGNED-OFF] Added unit tests for Sip Checksum.pm http://bugs.koha-community.org/show_bug.cgi?id=5327 Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de> Test passes.
Created attachment 6728 [details] [review] [SIGNED-OFF] Added unit test for AuthoritiesMarc/UNIMARC.pm http://bugs.koha-community.org/show_bug.cgi?id=5327 Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de> Test passes.
Created attachment 6729 [details] [review] [SIGNED-OFF] Added unit tests for Scrubber http://bugs.koha-community.org/show_bug.cgi?id=5327 Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de> Test passes.
Created attachment 6730 [details] [review] [SIGNED-OFF] Bug 5327: Patch removes unneeded self=shift from TTParser::unshift_token Removed unneeded self=shift from unshift_token (as peek and next do not take a self, makes interface more consistent) http://bugs.koha-community.org/show_bug.cgi?id=5327 Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de>
Created attachment 6731 [details] [review] [SIGNED-OFF] Added coverage to unit test for TTParser http://bugs.koha-community.org/show_bug.cgi?id=5327 Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de> Test passes.
Created attachment 6732 [details] [review] [SIGNED-OFF] Added missing reference case to Boolean.t and corrected count http://bugs.koha-community.org/show_bug.cgi?id=5327 Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de> Test passes.
Created attachment 6795 [details] [review] Unit test for SIP/ILS/Transaction/RenewAll.pm Requires the Koha sample data to be present to pass correctly.
Created attachment 6799 [details] [review] Patch to move SIP_ILS.t to db_dependent
Created attachment 6805 [details] [review] [SINGED-OFF] Bug 5327: Added unit test for C4/SIP/ILS/Transaction/RenewAll.pm http://bugs.koha-community.org/show_bug.cgi?id=5327 Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de> Test requires a database installed using the sample data. All tests pass.
Hi Duncan, your first patch works, but after moving the test it fails: perl t/db_dependent/SIP_ILS.t 1..4 not ok 1 - use C4::SIP::ILS; # Failed test 'use C4::SIP::ILS;' # at t/db_dependent/SIP_ILS.t line 14. # Tried to use 'C4::SIP::ILS'. # Error: Can't locate ILS/Item.pm in @INC (@INC contains: /home/katrin/kohaclone/t/db_dependent/../C4/SIP /home/katrin/kohaclone /etc/perl /usr/local/lib/perl/5.12.4 /usr/local/share/perl/5.12.4 /usr/lib/perl5 /usr/share/perl5 /usr/lib/perl/5.12 /usr/share/perl/5.12 /usr/local/lib/site_perl .) at /home/katrin/kohaclone/C4/SIP/ILS.pm line 11. # BEGIN failed--compilation aborted at /home/katrin/kohaclone/C4/SIP/ILS.pm line 11. # Compilation failed in require at (eval 8) line 2. # BEGIN failed--compilation aborted at (eval 8) line 2. Can't locate object method "new" via package "ILS::Transaction::RenewAll" (perhaps you forgot to load "ILS::Transaction::RenewAll"?) at t/db_dependent/SIP_ILS.t line 17. # Looks like you planned 4 tests but ran 1. # Looks like you failed 1 test of 1 run. # Looks like your test exited with 255 just after 1.
Changing to signed-off as there are a lot of patches here that are ready for QA.
Comment on attachment 6727 [details] [review] [SIGNED-OFF] Added unit tests for Sip Checksum.pm patch pushed please test
Comment on attachment 6728 [details] [review] [SIGNED-OFF] Added unit test for AuthoritiesMarc/UNIMARC.pm patch pushed, please test
Comment on attachment 6729 [details] [review] [SIGNED-OFF] Added unit tests for Scrubber patch pushed please test
Comment on attachment 6730 [details] [review] [SIGNED-OFF] Bug 5327: Patch removes unneeded self=shift from TTParser::unshift_token patch pushed, please test
Comment on attachment 6805 [details] [review] [SINGED-OFF] Bug 5327: Added unit test for C4/SIP/ILS/Transaction/RenewAll.pm patch pushed
Comment on attachment 6732 [details] [review] [SIGNED-OFF] Added missing reference case to Boolean.t and corrected count patch pushed
QA comments: 2 tests not merged: ==== TEST 1 ==== [SIGNED-OFF] Added coverage to unit test for TTParser I don't understand what this patch is adding. A little bit more explanatory on the commit notes welcomed... ==== TEST 2 ==== Applying: Moving SIP_ILS.t to db_dependent fatal: sha1 information is lacking or useless (t/SIP_ILS.t). Repository lacks necessary blobs to fall back on 3-way merge. Cannot fall back to three-way merge. Patch failed at 0001 Moving SIP_ILS.t to db_dependent When you have resolved this problem run "git am --resolved". If you would prefer to skip this patch, instead run "git am --skip".
Comment on attachment 6730 [details] [review] [SIGNED-OFF] Bug 5327: Patch removes unneeded self=shift from TTParser::unshift_token unobsoleting this patch because I'll revert it Why did I miss the patch break the tests ? I don't know. But here it is: prove xt/author/translatable-templates.t xt/author/translatable-templates.t .. 1/2 # Failed test 'opac templates are translatable' # at xt/author/translatable-templates.t line 54. # Can't locate object method "type" via package "C4::TTParser" at TmplTokenizer.pm line 324, <INPUT> line 1. # # tmpl_process3.pl: ERROR: Text extraction failed: ./xgettext.pl: Ioctl() inappropré pour un périphérique # # tmpl_process3.pl: ERROR: Will not run msgmerge # xt/author/translatable-templates.t .. 2/2 # Failed test 'intranet templates are translatable' # at xt/author/translatable-templates.t line 54. # Can't locate object method "type" via package "C4::TTParser" at TmplTokenizer.pm line 324, <INPUT> line 1. # # tmpl_process3.pl: ERROR: Text extraction failed: ./xgettext.pl: Ioctl() inappropré pour un périphérique # # tmpl_process3.pl: ERROR: Will not run msgmerge # # Looks like you failed 2 tests of 2. xt/author/translatable-templates.t .. Dubious, test returned 2 (wstat 512, 0x200) Failed 2/2 subtests Test Summary Report ------------------- xt/author/translatable-templates.t (Wstat: 512 Tests: 2 Failed: 2) Failed tests: 1-2 Non-zero exit status: 2 after reverting this patch, prove is OK
Comment on attachment 6805 [details] [review] [SINGED-OFF] Bug 5327: Added unit test for C4/SIP/ILS/Transaction/RenewAll.pm push reverted, make jenkins unhappy, seems it's a database problem thing so the other ILS moved to db_dependant is required to have this one working
Created attachment 6819 [details] [review] Corrected findbin path for SIP_ILS bug 5327
Created attachment 6826 [details] [review] [SIGNED-OFF] Bug 5327: Added unit test for C4/SIP/ILS/Transaction/RenewAll.pm http://bugs.koha-community.org/show_bug.cgi?id=5327 Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de> Test requires a database installed using the sample data. All tests pass.
Created attachment 6827 [details] [review] [SIGNED-OFF] Bug 5327: Moving SIP_ILS.t to db_dependent http://bugs.koha-community.org/show_bug.cgi?id=5327 Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de>
Created attachment 6828 [details] [review] [SIGNED-OFF] Bug 5327: Corrected findbin path for SIP_ILS bug 5327 Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de> Tests run successfully now.
The last 3 patches for the SIP tests go together (attachments 6826-6828)
Created attachment 6834 [details] [review] Bug 5327: Added unit test for C4/SIP/ILS/Transaction/RenewAll.pm http://bugs.koha-community.org/show_bug.cgi?id=5327 Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de> Test requires a database installed using the sample data. All tests pass. Signed-off-by: Paul Poulain <paul.poulain@biblibre.com>
Comment on attachment 6834 [details] [review] Bug 5327: Added unit test for C4/SIP/ILS/Transaction/RenewAll.pm QA comment for SIP_ILS patches: prove t/db_dependent/SIP_ILS.t t/db_dependent/SIP_ILS.t .. Subroutine new redefined at /home/paul/koha.dev/koha-community//C4/SIP/ILS.pm line 44. Subroutine find_patron redefined at /home/paul/koha.dev/koha-community//C4/SIP/ILS.pm line 55. Subroutine find_item redefined at /home/paul/koha.dev/koha-community//C4/SIP/ILS.pm line 61. Subroutine institution redefined at /home/paul/koha.dev/koha-community//C4/SIP/ILS.pm line 67. Subroutine institution_id redefined at /home/paul/koha.dev/koha-community//C4/SIP/ILS.pm line 72. Subroutine supports redefined at /home/paul/koha.dev/koha-community//C4/SIP/ILS.pm line 77. Subroutine check_inst_id redefined at /home/paul/koha.dev/koha-community//C4/SIP/ILS.pm line 82. Subroutine to_bool redefined at /home/paul/koha.dev/koha-community//C4/SIP/ILS.pm line 90. Subroutine checkout_ok redefined at /home/paul/koha.dev/koha-community//C4/SIP/ILS.pm line 99. Subroutine checkin_ok redefined at /home/paul/koha.dev/koha-community//C4/SIP/ILS.pm line 104. Subroutine status_update_ok redefined at /home/paul/koha.dev/koha-community//C4/SIP/ILS.pm line 109. Subroutine offline_ok redefined at /home/paul/koha.dev/koha-community//C4/SIP/ILS.pm line 114. Subroutine checkout redefined at /home/paul/koha.dev/koha-community//C4/SIP/ILS.pm line 128. Subroutine checkin redefined at /home/paul/koha.dev/koha-community//C4/SIP/ILS.pm line 174. Subroutine end_patron_session redefined at /home/paul/koha.dev/koha-community//C4/SIP/ILS.pm line 210. Subroutine pay_fee redefined at /home/paul/koha.dev/koha-community//C4/SIP/ILS.pm line 217. Subroutine add_hold redefined at /home/paul/koha.dev/koha-community//C4/SIP/ILS.pm line 238. Subroutine cancel_hold redefined at /home/paul/koha.dev/koha-community//C4/SIP/ILS.pm line 282. Subroutine alter_hold redefined at /home/paul/koha.dev/koha-community//C4/SIP/ILS.pm line 337. Subroutine renew redefined at /home/paul/koha.dev/koha-community//C4/SIP/ILS.pm line 382. Subroutine renew_all redefined at /home/paul/koha.dev/koha-community//C4/SIP/ILS.pm line 447. t/db_dependent/SIP_ILS.t .. ok All tests successful. The 2 follow-ups are really small, i've squashed them into 1 single commit passed QA patch pushed
About the 2 patches TTParser: They make translatable-template.t fail: prove xt/author/translatable-templates.t xt/author/translatable-templates.t .. 1/2 # Failed test 'opac templates are translatable' # at xt/author/translatable-templates.t line 54. # Can't locate object method "type" via package "C4::TTParser" at TmplTokenizer.pm line 324, <INPUT> line 1. # # tmpl_process3.pl: ERROR: Text extraction failed: ./xgettext.pl: Ioctl() inappropré pour un périphérique # # tmpl_process3.pl: ERROR: Will not run msgmerge # xt/author/translatable-templates.t .. 2/2 # Failed test 'intranet templates are translatable' # at xt/author/translatable-templates.t line 54. # Can't locate object method "type" via package "C4::TTParser" at TmplTokenizer.pm line 324, <INPUT> line 1. # # tmpl_process3.pl: ERROR: Text extraction failed: ./xgettext.pl: Ioctl() inappropré pour un périphérique # # tmpl_process3.pl: ERROR: Will not run msgmerge # # Looks like you failed 2 tests of 2. xt/author/translatable-templates.t .. Dubious, test returned 2 (wstat 512, 0x200) Failed 2/2 subtests Test Summary Report ------------------- xt/author/translatable-templates.t (Wstat: 512 Tests: 2 Failed: 2) Failed tests: 1-2 Non-zero exit status: 2 Files=1, Tests=2, 1 wallclock secs ( 0.04 usr 0.00 sys + 0.42 cusr 0.06 csys = 0.52 CPU) when switching back to master, everything work fine: prove xt/author/translatable-templates.t xt/author/translatable-templates.t .. ok All tests successful. Rejecting those patches for instance
unless i'm mistaking, the 2 attached patches are failed QA. updating bug status.
Created attachment 7179 [details] [review] Added unit tests for C4/Utils
Created attachment 7180 [details] [review] added unit tests for Classsortroutine/Generic
Created attachment 7181 [details] [review] Added unit tests for C4/ClassSortRoutine/LCC.pm
Created attachment 7182 [details] [review] added unit tests for C4/ClassSortRoutine/Dewey.pm
Created attachment 7183 [details] [review] Bug 5327 added unit tests for C4/ClassSortRoutine/Generic Signed-off-by: Chris Cormack <chris@catalyst.net.nz>
Created attachment 7184 [details] [review] Bug 5327 added unit tests for C4/Utils Signed-off-by: Chris Cormack <chrisc@catalyst.net.nz>
Created attachment 7185 [details] [review] Bug 5327 added unit tests for C4/ClassSortRoutine/Generic Signed-off-by: Chris Cormack <chrisc@catalyst.net.nz>
Created attachment 7186 [details] [review] bug 5327 add tests for C4/ClassSortRoutine/LCC.pm Signed-off-by: Chris Cormack <chrisc@catalyst.net.nz>
Created attachment 7187 [details] [review] bug 5327 add tests for C4/ClassSortRoutine/LCC.pm Signed-off-by: Chris Cormack <chrisc@catalyst.net.nz>
Created attachment 7188 [details] [review] bug 5327 add tests for C4/ClassSortRoutine/LCC.pm Signed-off-by: Chris Cormack <chrisc@catalyst.net.nz>
Created attachment 7189 [details] [review] [SIGNED-OFF] bug 5327 add tests for C4/ClassSortRoutine/LCC.pm Signed-off-by: Chris Cormack <chrisc@catalyst.net.nz>
Created attachment 7190 [details] [review] [SIGNED-OFF] Bug 5327 Added unit tests for C4/ClassSortRoutine/Dewey.pm Signed-off-by: Chris Cormack <chrisc@catalyst.net.nz>
Created attachment 7191 [details] [review] tests for C4/BackgroundJob.pm
Created attachment 7192 [details] [review] added unit tests for C4/Reports.pm
Created attachment 7194 [details] [review] [SIGNED-OFF] Bug 5327 Added unit tests for C4/Reports.pm Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de> perl t/Reports.t b/t/Reports.t All tests complete successfully.
Created attachment 7195 [details] [review] [SIGNED-OFF] bug 5327 add tests for C4/BackgroundJob.pm Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de> perl t/db_dependent/BackgroundJob.t All tests complete successfully. Note: Test 11 does not tell what it's about, but it has been there before.
Comment on attachment 7184 [details] [review] Bug 5327 added unit tests for C4/Utils QA comment about C4::Utils unit test: I think this package is never user and could be removed. It's declared only in C4/Auth_with_cas.pm and no use seems to be made of the functions in Utils. Could someone confirm ?
Comment on attachment 7185 [details] [review] Bug 5327 added unit tests for C4/ClassSortRoutine/Generic QA comment: no comment, this one is OK
Comment on attachment 7189 [details] [review] [SIGNED-OFF] bug 5327 add tests for C4/ClassSortRoutine/LCC.pm QA comment: Failed QA => a small typo here: "Arguements" is not english, it should be "Arguments" (confirmed by oleonard on IRC, i'm not english native, I had a doubt...)
Comment on attachment 7190 [details] [review] [SIGNED-OFF] Bug 5327 Added unit tests for C4/ClassSortRoutine/Dewey.pm QA comment = this one is OK
Comment on attachment 7194 [details] [review] [SIGNED-OFF] Bug 5327 Added unit tests for C4/Reports.pm QA comment: this one is OK
Comment on attachment 7195 [details] [review] [SIGNED-OFF] bug 5327 add tests for C4/BackgroundJob.pm QA comment: this one is OK
100 comments, 54 patches attached, this entry becomes hard to deal with... shouldn't we transform it into an omnibus (and each test added in a separate bug entry blocking this one) ?
Comment on attachment 7185 [details] [review] Bug 5327 added unit tests for C4/ClassSortRoutine/Generic patch pushed
Comment on attachment 7190 [details] [review] [SIGNED-OFF] Bug 5327 Added unit tests for C4/ClassSortRoutine/Dewey.pm patch pushed
Comment on attachment 7194 [details] [review] [SIGNED-OFF] Bug 5327 Added unit tests for C4/Reports.pm patch pushed
Comment on attachment 7195 [details] [review] [SIGNED-OFF] bug 5327 add tests for C4/BackgroundJob.pm patch pushed
everything pushable has been pushed. The 4 attachements not pushed are failing QA for instance. Please submit an updated patch for LCC.pm & we will see if it's needed to push something for Utils, if Utils is deprecated, then please open a bug to request his removal !
Created attachment 7203 [details] [review] Added unit tests for C4/ClassSortRoutine/LCC.pm Fixed typos in patch
Created attachment 7206 [details] [review] Added unit tests for C4/Koha.pm
Created attachment 7208 [details] [review] added unit tests for C4/SIP/ILS/Transaction/Renew.pm Complained about lots of lines like this: Subroutine checkout redefined at C4/SIP/ILS.pm before I added anything to the file.
Created attachment 7209 [details] [review] Added unit tests for C4/Ris
Created attachment 7220 [details] [review] Added unit tests for C4/AuthoritiesMarc/MARC21.pm
Created attachment 7221 [details] [review] Added unit tests for C4/Search/PazPar2.pm
Created attachment 7222 [details] [review] added unit tests for C4/TmplToken.pm
Created attachment 7223 [details] [review] added unit test for C4/Barcodes/PrinterConfig.pm
Created attachment 7224 [details] [review] tests for C4/Label.pm
Created attachment 7225 [details] [review] Added unit tests for C4/Languages.pm
Created attachment 7226 [details] [review] Added unit tests for C4/Creators/PDF.pm
Created attachment 7227 [details] [review] Added unit tests for C4/Scheduler.pm
Created attachment 7228 [details] [review] Added unit tests for C4/Csv.pm
Created attachment 7229 [details] [review] Signed off
Created attachment 7230 [details] [review] [SIGNED-OFF] Added unit tests for C4/Csv.pm
Created attachment 7231 [details] [review] [SIGNED-OFF] added unit tests for C4/Labels.pm Forgot to set description properly, oops.
Created attachment 7232 [details] [review] [SIGNED-OFF] Added unit tests for C4/AuthoritiesMarc/MARC21.pm
Created attachment 7233 [details] [review] [SIGNED-OFF] added unit test for C4/Barcodes/PrinterConfig.pm
Created attachment 7234 [details] [review] [SIGNED-OFF] Added unit tests for C4/ClassSortRoutine/LCC.pm
Created attachment 7235 [details] [review] [SIGNED-OFF] Added unit tests for C4/Creators/PDF.pm
Created attachment 7236 [details] [review] [SIGNED-OFF] added unit tests for C4/Ris
Created attachment 7237 [details] [review] [SIGNED-OFF] Added unit tests for C4/Scheduler.pm
Created attachment 7238 [details] [review] Added unit tests for C4/Serials.pm
Created attachment 7239 [details] [review] [SIGNED-OFF] Added unit tests for C4/Search/PazPar2.pm
Created attachment 7240 [details] [review] [SIGNED-OFF] added unit tests for C4/TmplToken.pm
Created attachment 7241 [details] [review] [SIGNED-OFF] added unit tests for C4/SIP/ILS/Transaction/Renew.pm
Created attachment 7242 [details] [review] [SIGNED-OFF] Added unit tests for C4/Languages.pm
Comment on attachment 7184 [details] [review] Bug 5327 added unit tests for C4/Utils Utils is obsoled AFAIK, so marking this patch as obsolete. If i'm wrong, remove the obsolete status & let me know
Comment on attachment 7206 [details] [review] Added unit tests for C4/Koha.pm QA Comment : this patch depends on what is in the database +is(C4::Koha::GetSupportName('BK'),"Books", 'testing support name'); +is(C4::Koha::GetSupportName(''),undef, 'testing support name'); It should be in t/db_dependant. and not in t/ marking obsolete as it fails QA 2 possible fixes: * remove db_dependant from those tests & create another one in t/db_dependant * move everything to t/db_dependant
Comment on attachment 7230 [details] [review] [SIGNED-OFF] Added unit tests for C4/Csv.pm this patch has been pushed
Comment on attachment 7231 [details] [review] [SIGNED-OFF] added unit tests for C4/Labels.pm this patch has been pushed
Comment on attachment 7232 [details] [review] [SIGNED-OFF] Added unit tests for C4/AuthoritiesMarc/MARC21.pm this patch has been pushed
Comment on attachment 7233 [details] [review] [SIGNED-OFF] added unit test for C4/Barcodes/PrinterConfig.pm this patch has been pushed
Comment on attachment 7234 [details] [review] [SIGNED-OFF] Added unit tests for C4/ClassSortRoutine/LCC.pm this patch has been pushed
Comment on attachment 7235 [details] [review] [SIGNED-OFF] Added unit tests for C4/Creators/PDF.pm this patch has been pushed
Comment on attachment 7236 [details] [review] [SIGNED-OFF] added unit tests for C4/Ris this patch has been pushed
Comment on attachment 7237 [details] [review] [SIGNED-OFF] Added unit tests for C4/Scheduler.pm this patch has been pushed
Comment on attachment 7238 [details] [review] Added unit tests for C4/Serials.pm this patch has been pushed
Comment on attachment 7239 [details] [review] [SIGNED-OFF] Added unit tests for C4/Search/PazPar2.pm this patch has been pushed
Comment on attachment 7240 [details] [review] [SIGNED-OFF] added unit tests for C4/TmplToken.pm this patch has been pushed
Comment on attachment 7241 [details] [review] [SIGNED-OFF] added unit tests for C4/SIP/ILS/Transaction/Renew.pm this patch has been pushed
Comment on attachment 7242 [details] [review] [SIGNED-OFF] Added unit tests for C4/Languages.pm this patch has been pushed
Comment on attachment 7206 [details] [review] Added unit tests for C4/Koha.pm changing my mind about "obsolete" = it's much easier to keep all pending patches not obsolete !
Created attachment 7400 [details] [review] Bug 5327: Shifting a db dependent test to the right place
Created attachment 7402 [details] [review] Merge remote-tracking branch 'origin/new/bug_5327' Signed-off-by: Chris Hall <chrish@catalyst.net.nz> Signed-off-by: Chris Hall <chrish@catalyst.net.nz> Signed-off-by: Chris Hall <chrish@catalyst.net.nz> Signed-off-by: Chris Hall <chrish@catalyst.net.nz> http://bugs.koha-community.org/show_bug.cgi?id=5327 Signed-off-by: Chris Hall <chrish@catalyst.net.nz> Signed-off-by: Chris Hall <chrish@catalyst.net.nz> Signed-off-by: Chris Hall <chrish@catalyst.net.nz> Signed-off-by: Chris Hall <chrish@catalyst.net.nz> Signed-off-by: Chris Hall <chrish@catalyst.net.nz> Signed-off-by: Chris Hall <chrish@catalyst.net.nz> Signed-off-by: Chris Hall <chrish@catalyst.net.nz> Signed-off-by: Chris Hall <chrish@catalyst.net.nz>
Comment on attachment 7402 [details] [review] Merge remote-tracking branch 'origin/new/bug_5327' Whoops, wrong patch
Created attachment 7403 [details] [review] Bug 5327: Shifting a db dependent test to the right place Signed-off-by: Robin Sheat <robin@catalyst.net.nz>
Created attachment 7405 [details] [review] Bug 5327 - move another test around
Robin / chris_c = I understood db_dependant directory as where we should put tests that work only if the database need something specific to work. A script that run subs using the database but does not depend on what is should be in t/ that's why I've pushed Csv and Reports, and am not sure the 2 patches moving them submitted yesterday are applicable.
(In reply to comment #156) > Robin / chris_c = I understood db_dependant directory as where we should put > tests that work only if the database need something specific to work. > A script that run subs using the database but does not depend on what is should > be in t/ > that's why I've pushed Csv and Reports, and am not sure the 2 patches moving > them submitted yesterday are applicable. db_dependent is for the tests that require a db connection, the ones in just t/ don't need a database. They are the ones run during a make test, having ones that require a db connection means tests fail during install which is confusing for users. So no db_dependent is for tests that depend on a db connection, not some specific data. (no other tests in t/ depend on a db connection)
Comment on attachment 7403 [details] [review] Bug 5327: Shifting a db dependent test to the right place patch pushed
Comment on attachment 7405 [details] [review] Bug 5327 - move another test around this patch pushed
The 3 attachments not pushed are failing QA,so marking this bug failed QA
Created attachment 7573 [details] [review] Bug 5327 : Removing a broken test
New patch needs signoff and will stop the failing tests on jenkins
Created attachment 7592 [details] [review] Bug 5327: Patch removes unneeded self=shift from TTParser::unshift_token Removed unneeded self=shift from unshift_token (as peek and next do not take a self, makes interface more consistent) http://bugs.koha-community.org/show_bug.cgi?id=5327 Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de> Signed-off-by: Jared Camins-Esakov <jcamins@cpbibliography.com>
Created attachment 7593 [details] [review] Added coverage to unit test for TTParser http://bugs.koha-community.org/show_bug.cgi?id=5327 Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de> Test passes. Signed-off-by: Jared Camins-Esakov <jcamins@cpbibliography.com>
Created attachment 7594 [details] [review] Bug 5327 : Removing a broken test Signed-off-by: Jared Camins-Esakov <jcamins@cpbibliography.com>
The test for C4/Koha did not work for me, but I tested and signed off on the other three.
Comment on attachment 7594 [details] [review] Bug 5327 : Removing a broken test this patch has been pushed
removing signed-off status, the 3 remaining patches should not be applied
Comment on attachment 7593 [details] [review] Added coverage to unit test for TTParser this patch has been pushed
Comment on attachment 7592 [details] [review] Bug 5327: Patch removes unneeded self=shift from TTParser::unshift_token This patch has been pushed, after discussion with Jared: I missed that those 2 patches had been updated and now work, so my comment 168 was wrong for them. C4/Koha.pm still un-needed though
Comment on attachment 7593 [details] [review] Added coverage to unit test for TTParser Un-obsoleting those patches this one and http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=7592 After pushing them jenkins became unstable again. prove xt/author_translatable_templates.t fails: 13:51 ~/koha.dev/koha-community (master $% u=)$ prove xt/author/translatable-templates.t xt/author/translatable-templates.t .. 1/2 # Failed test 'opac templates are translatable' # at xt/author/translatable-templates.t line 54. # Can't locate object method "type" via package "C4::TTParser" at TmplTokenizer.pm line 324, <INPUT> line 1. # # tmpl_process3.pl: ERROR: Text extraction failed: ./xgettext.pl: Ioctl() inappropré pour un périphérique # # tmpl_process3.pl: ERROR: Will not run msgmerge # # Failed test 'intranet templates are translatable' # at xt/author/translatable-templates.t line 54. # Can't locate object method "type" via package "C4::TTParser" at TmplTokenizer.pm line 324, <INPUT> line 1. # # tmpl_process3.pl: ERROR: Text extraction failed: ./xgettext.pl: Ioctl() inappropré pour un périphérique # # tmpl_process3.pl: ERROR: Will not run msgmerge # # Looks like you failed 2 tests of 2. xt/author/translatable-templates.t .. Dubious, test returned 2 (wstat 512, 0x200) Failed 2/2 subtests Test Summary Report ------------------- xt/author/translatable-templates.t (Wstat: 512 Tests: 2 Failed: 2) Failed tests: 1-2 Non-zero exit status: 2 Files=1, Tests=2, 0 wallclock secs ( 0.04 usr 0.00 sys + 0.46 cusr 0.04 csys = 0.54 CPU) Result: FAIL Reverting them make this test pass. I've reverted the 2 patches
Created attachment 9897 [details] [review] Bug 5327 : Fix for XISBN test, thingisbn is now handing back different results
Comment on attachment 9897 [details] [review] Bug 5327 : Fix for XISBN test, thingisbn is now handing back different results This patch has been pushed, should please jenkins (seems librarything changes the returned results, making the test fail)
Created attachment 10234 [details] [review] Added some C4/Items.pm tests
Created attachment 10616 [details] [review] bug 5327: added some tests for C4/Items.pm Signed-off-by: Jared Camins-Esakov <jcamins@cpbibliography.com>
Last patch was signed off.
Created attachment 10656 [details] [review] t/Koha.t http://bugs.koha-community.org/show_bug.cgi?id=5327 Signed-off-by: Mason James <mtj@kohaaloha.com> http://bugs.koha-community.org/show_bug.cgi?id=7592
Created attachment 10659 [details] [review] Bug 5327 - Unit tests required for all C4 modules renamed: Koha.t -> db_dependent/Koha.t Signed-off-by: Mason James <mtj@kohaaloha.com>
(In reply to comment #178) > Created attachment 10659 [details] [review] > Bug 5327 - Unit tests required for all C4 modules > > renamed: Koha.t -> db_dependent/Koha.t > > Signed-off-by: Mason James <mtj@kohaaloha.com> test works well $ prove -v ./t/db_dependent/Koha.t ./t/db_dependent/Koha.t .. 1..8 ok 1 - use C4::Koha; ok 2 - slashifyDate ok 3 - xml_escape() returns empty string on undef input ok 4 - xml_escape() works as expected ok 5 - ... and does not change input in place ok 6 - _isbn_cleanup removes hyphens ok 7 - _isbn_cleanup removes parenthetical ok 8 - _isbn_cleanup converts ISBN-13 to ISBN-10 ok All tests successful. Files=1, Tests=8, 0 wallclock secs ( 0.02 usr 0.01 sys + 0.22 cusr 0.02 csys = 0.27 CPU) Result: PASS
Created attachment 10660 [details] [review] Added coverage to unit test for TTParser http://bugs.koha-community.org/show_bug.cgi?id=5327 Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de> Test passes. Signed-off-by: Jared Camins-Esakov <jcamins@cpbibliography.com>
(In reply to comment #180) > Created attachment 10660 [details] [review] > Added coverage to unit test for TTParser > > http://bugs.koha-community.org/show_bug.cgi?id=5327 > Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de> > Test passes. > > Signed-off-by: Jared Camins-Esakov <jcamins@cpbibliography.com> Passing QA on this, test passed OK $ prove -v xt/tt_valid.t xt/tt_valid.t .. 1..2 ok 1 - TT syntax: not using TT directive within HTML tag ok 2 - Token received same as original put on stack ok All tests successful. Files=1, Tests=2, 3 wallclock secs ( 0.02 usr 0.01 sys + 2.68 cusr 0.03 csys = 2.74 CPU) Result: PASS
(In reply to comment #175) > Created attachment 10616 [details] [review] > bug 5327: added some tests for C4/Items.pm > > Signed-off-by: Jared Camins-Esakov <jcamins@cpbibliography.com> Passing QA on this patch, all test pass OK $ prove -v t/db_dependent/Items.t 1..7 ok 1 - use C4::Items; # Creating biblio instance for testing. ok 2 - New item is linked to correct biblionumber. ok 3 - New item is linked to correct biblioitemnumber. ok 4 - Retrieved item has correct itemnumber. ok 5 - Retrieved item has correct biblioitemnumber. ok 6 - Modified item barcode successfully to: 987654321. ok 7 - Item deleted as expected. # Deleting biblio testing instance. ok All tests successful. Files=1, Tests=7, 1 wallclock secs ( 0.03 usr 0.00 sys + 0.48 cusr 0.05 csys = 0.56 CPU) Result: PASS
Created attachment 10661 [details] [review] added date and ISBN tests to Koha.t modified: Koha.t $ prove -v ./Koha.t 1..5 ok 1 - use C4::Koha; ok 2 - use C4::Members; 1..6 ok 1 - Insert data in database ok 2 - GetAuthorisedValueByCode ok 3 - GetKohaImageurlFromAuthorisedValues ok 4 - lost and paid works ok 5 - Child area works ok 6 - Withdrawn works ok 3 - Authorized Values Tests 1..4 ok 1 - C4::Koha->can('GetDailyQuote') ok 2 - Got a quote based on id. ok 3 - Got a random quote. ok 4 - Got a quote based on today's date. ok 4 - Daily Quotes Test 1..7 ok 1 - slashifyDate ok 2 - xml_escape() returns empty string on undef input ok 3 - xml_escape() works as expected ok 4 - ... and does not change input in place ok 5 - _isbn_cleanup removes hyphens ok 6 - _isbn_cleanup removes parenthetical ok 7 - _isbn_cleanup converts ISBN-13 to ISBN-10 ok 5 - Date and ISBN tests ok All tests successful. Files=1, Tests=5, 1 wallclock secs ( 0.02 usr 0.01 sys + 0.48 cusr 0.06 csys = 0.57 CPU) Result: PASS
Comment on attachment 10616 [details] [review] bug 5327: added some tests for C4/Items.pm This patch pushed
Comment on attachment 10660 [details] [review] Added coverage to unit test for TTParser This patch pushed
Comment on attachment 10661 [details] [review] added date and ISBN tests to Koha.t This patch pushed
May I suggest that we close this bug an open a new one = 186 comments, 96 attachments, it could become hard to follow
Comment on attachment 10660 [details] [review] Added coverage to unit test for TTParser I've reverted this test, it seems it causes problem to Jenkins, so, unobsoleting it
Im not pushing the patch for Koha.t to stable, it should be in db_dependent It will break installing otherwise. Please push a follow up so I can push them both
Created attachment 10680 [details] [review] Bug 5327 : Follow up for TTParser test
Comment on attachment 10680 [details] [review] Bug 5327 : Follow up for TTParser test This patch pushed
Assigning this bug to myself, a lot of ppl in cc: you can remove yourself from this bug if you don't want to be annoyed anymore. Edit cc list, on the top right of the page
Created attachment 11141 [details] [review] Bug 5327 : Unit tests for C4::ItemType
Created attachment 11186 [details] [review] Patch of 'ItemType.t' - added a few tests
Created attachment 11212 [details] [review] Bug 5327: improve t/ItemType.t Added a few more lines of code to the ItemType.t unit test Signed-off-by: Jared Camins-Esakov <jcamins@cpbibliography.com>
Created attachment 11213 [details] [review] Bug 5327 follow-up: add DBD::Mock dependency In order for the awesome new t/ItemType.t test to work, we need DBD::Mock.
I signed off on the test, but the follow-up needs a sign off.
(In reply to comment #195) > Created attachment 11186 [details] [review] > Patch of 'ItemType.t' - added a few tests Hi Bart, just for information = your git config is not correct. Git says that submit patches with a mail address of : From: Bart Jorgensen <bartjorgensen@ubuntu> I suspect it's not your real mail address ;-) Check here to fix it: http://wiki.koha-community.org/wiki/Version_Control_Using_Git#Fill_some_parameters Could you resubmit with a valid mail address please ?
Created attachment 11278 [details] [review] Changed Itemtype.t patch
Created attachment 11279 [details] [review] Bug 5327 : Added a few more lines of code to the ItemType.t unit test Signed-off-by: Jared Camins-Esakov <jcamins@cpbibliography.com> Signed-off-by: Chris Cormack <chrisc@catalyst.net.nz>
Created attachment 11280 [details] [review] Bug 5327 follow-up: add DBD::Mock dependency In order for the awesome new t/ItemType.t test to work, we need DBD::Mock. Signed-off-by: Chris Cormack <chrisc@catalyst.net.nz>
Created attachment 11281 [details] [review] Added tests to C4 Matcher.
Created attachment 11339 [details] [review] Bug 5327 : Added a few more lines of code to the ItemType.t unit test Signed-off-by: Jared Camins-Esakov <jcamins@cpbibliography.com> Signed-off-by: Chris Cormack <chrisc@catalyst.net.nz> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Created attachment 11340 [details] [review] Bug 5327 follow-up: add DBD::Mock dependency In order for the awesome new t/ItemType.t test to work, we need DBD::Mock. Signed-off-by: Chris Cormack <chrisc@catalyst.net.nz> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Created attachment 11341 [details] [review] Bug 5327 Tests for C4::Matcher Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Created attachment 11440 [details] [review] Added unit tests to 'Members_AttributeTypes.t' increases overal coverage of C4/Members/AttributeTypes.pm from 7.7% to 33% :)
Created attachment 11467 [details] [review] Added fake db and a test
Created attachment 11469 [details] [review] [SIGNED-OFF] Bug 5327 : Added a few more lines of code to the ItemType.t unit test Signed-off-by: Jared Camins-Esakov <jcamins@cpbibliography.com> Signed-off-by: Chris Cormack <chrisc@catalyst.net.nz> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de>
Created attachment 11470 [details] [review] [PASSED-QA] Bug 5327 follow-up: add DBD::Mock dependency In order for the awesome new t/ItemType.t test to work, we need DBD::Mock. Signed-off-by: Chris Cormack <chrisc@catalyst.net.nz> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de>
Created attachment 11471 [details] [review] [SIGNED-OFF] Bug 5327 Tests for C4::Matcher Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de>
Created attachment 11472 [details] [review] [SIGNED-OFF] Added and tested a mock-db tested a couple of features http://bugs.koha-community.org/show_bug.cgi?id=5327 Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de> All tests pass!
Created attachment 11473 [details] [review] [SIGNED-OFF] Added a mock-db table and tested a line http://bugs.koha-community.org/show_bug.cgi?id=5327 Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de> All tests pass!
Created attachment 11500 [details] [review] [PASSED-QA]Bug 5327 : Added a few more lines of code to the ItemType.t unit test Signed-off-by: Jared Camins-Esakov <jcamins@cpbibliography.com> Signed-off-by: Chris Cormack <chrisc@catalyst.net.nz> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de>
Created attachment 11503 [details] [review] [PASSED-QA]Bug 5327 Tests for C4::Matcher Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de> Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Created attachment 11505 [details] [review] [PASSED-QA]Added and tested a mock-db tested a couple of features http://bugs.koha-community.org/show_bug.cgi?id=5327 Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de> All tests pass! Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Created attachment 11506 [details] [review] [PASSED-QA]Added a mock-db table and tested a line http://bugs.koha-community.org/show_bug.cgi?id=5327 Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de> All tests pass! Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Last 5 patches resubmited (perltidy) Marked as Passed QA
Hello all, Jonathan tells me to try DBD::Mock for one of my patch (Bug 5339), which I did. But I noticed some odd behaviour and I'm wondering if we are using it like we should. Example: # This assume C4::Context::_new_dbh is mocked. my $rs = [ ['id', 'column1', ...], [1, 'a', ...], [2, 'b', ...] ]; my $dbh = C4::Context->dbh; $dbh->{mock_add_resultset} = $rs; my @a = my_function(); my_function execute a single 'select' query and return data like in $rs, but in fact, even if my query is "SELECT * FROM bar WHERE id=1", it returns all the resultset. And even worse, if my query is something like "SELECT foo FROM bar", the whole resultset is still returned. My question is: how can we use DBD::Mock for testing subroutines while subroutines will always return what we expect, and not what they should? Is there something I'm doing wrong or something I don't understand?
(In reply to comment #219) > Hello all, > > Jonathan tells me to try DBD::Mock for one of my patch (Bug 5339), which I > did. > But I noticed some odd behaviour and I'm wondering if we are using it like > we should. > Example: > > # This assume C4::Context::_new_dbh is mocked. > > my $rs = [ > ['id', 'column1', ...], > [1, 'a', ...], > [2, 'b', ...] > ]; > my $dbh = C4::Context->dbh; > $dbh->{mock_add_resultset} = $rs; > my @a = my_function(); > > > my_function execute a single 'select' query and return data like in $rs, > but in fact, even if my query is "SELECT * FROM bar WHERE id=1", it returns > all the resultset. And even worse, if my query is something like > "SELECT foo FROM bar", the whole resultset is still returned. > > My question is: how can we use DBD::Mock for testing subroutines while > subroutines > will always return what we expect, and not what they should? > > Is there something I'm doing wrong or something I don't understand? Yep You can also associate a resultset with a particular SQL statement instead of adding them in the order they will be fetched: $dbh->{mock_add_resultset} = { sql => 'SELECT foo, bar FROM baz', results => [ [ 'foo', 'bar' ], [ 'this_one', 'that_one' ], [ 'this_two', 'that_two' ], ], }; http://search.cpan.org/~dichi/DBD-Mock-1.43/lib/DBD/Mock.pm
Created attachment 11633 [details] [review] Added unit tests to 'Installer_perlmodules'
Created attachment 11762 [details] [review] Testing c4 Images
Created attachment 11763 [details] [review] [PASSED-QA] Added tests to installer_perlmodules http://bugs.koha-community.org/show_bug.cgi?id=5327 Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de>
Created attachment 11764 [details] [review] [SIGNED-OFF] Bug 5327: Testing c4 Images Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de>
Created attachment 11765 [details] [review] Tests C4 Social Data
Created attachment 11846 [details] [review] [SIGNED OFF]Bug 5327: Unit tests for C4 socialdata Signed-off-by: Jared Camins-Esakov <jcamins@cpbibliography.com>
Created attachment 11985 [details] [review] Bug 5327 : Fixing a typo in the test that was causing a false fail
Created attachment 11993 [details] [review] [PASSED-QA] Bug 5327 : Fixing a typo in the test that was causing a false fail Signed-off-by: Jared Camins-Esakov <jcamins@cpbibliography.com> t/db_dependent/Items.t was failing despite valid data before patch, works fine after
Note that final patch is required in order to fix currently failing tests on Jenkins. Also note that the first 5 patches already passed QA.
Created attachment 12002 [details] [review] [PASSED-QA]Bug 5327: Unit tests for C4 socialdata Signed-off-by: Jared Camins-Esakov <jcamins@cpbibliography.com>
Created attachment 12003 [details] [review] [PASSED-QA]Bug 5327: Testing c4 Images Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de>
(In reply to comment #231) (In reply to comment #230) New perltidied patches
Comment on attachment 11470 [details] [review] [PASSED-QA] Bug 5327 follow-up: add DBD::Mock dependency This patch has been pushed
Comment on attachment 11500 [details] [review] [PASSED-QA]Bug 5327 : Added a few more lines of code to the ItemType.t unit test This patch has been pushed
Comment on attachment 11503 [details] [review] [PASSED-QA]Bug 5327 Tests for C4::Matcher This patch has been pushed
Comment on attachment 11505 [details] [review] [PASSED-QA]Added and tested a mock-db tested a couple of features This patch has been pushed
Comment on attachment 11506 [details] [review] [PASSED-QA]Added a mock-db table and tested a line This patch has been pushed
Comment on attachment 11763 [details] [review] [PASSED-QA] Added tests to installer_perlmodules This patch has been pushed
Comment on attachment 11993 [details] [review] [PASSED-QA] Bug 5327 : Fixing a typo in the test that was causing a false fail This patch has been pushed
Comment on attachment 12002 [details] [review] [PASSED-QA]Bug 5327: Unit tests for C4 socialdata This patch has been pushed
Comment on attachment 12003 [details] [review] [PASSED-QA]Bug 5327: Testing c4 Images This patch has been pushed
I think there is general agreement that future patches for this issue should be filed as separate bugs and set as depending on this bug. The number of comments on this one is becoming unwieldy.
There is no activity on this bug for more than 2 years and a lot of other bug reports have been opened. This one can be closed.