Summary: | Testing C4::OAI::Sets | ||
---|---|---|---|
Product: | Koha | Reporter: | Julian FIOL <julian.fiol> |
Component: | Test Suite | Assignee: | Julian FIOL <julian.fiol> |
Status: | CLOSED FIXED | QA Contact: | Galen Charlton <gmcharlt> |
Severity: | trivial | ||
Priority: | P5 - low | CC: | julian.maurice, tomascohen |
Version: | Main | ||
Hardware: | All | ||
OS: | All | ||
Change sponsored?: | --- | Patch complexity: | --- |
Documentation contact: | Documentation submission: | ||
Text to go in the release notes: | Version(s) released in: | ||
Circulation function: | |||
Attachments: |
Bug 13940 : Testing C4::OAI::Sets
Bug 13940 : Testing C4::OAI::Sets Bug 13940 : Testing C4::OAI::Sets [PASSED QA] Bug 13940 : Testing C4::OAI::Sets Bug 13940 : Testing C4::OAI::Sets (QA followup) Bug 13940 : (QA followup) Add tests for the 2 warnings which was not tested Bug 13940 : (QA followup) Add test for ModOAISet |
Description
Julian FIOL
2015-04-02 09:12:33 UTC
Created attachment 37429 [details] [review] Bug 13940 : Testing C4::OAI::Sets Testing C4::OAI::Sets in t/db_dependent/OAI/Sets.t Adding &DelOAISetsBiblio in @EXPORT at the beginning of OAI/Sets.pm, it was the only one missing TEST PLAN --------- 1. Apply patch 2. prove -v t/db_dependent/OAI/Sets.t -- All 145 tests should run successfully without any error or warning Created attachment 37433 [details] [review] Bug 13940 : Testing C4::OAI::Sets Testing C4::OAI::Sets in t/db_dependent/OAI/Sets.t Adding &DelOAISetsBiblio in @EXPORT at the beginning of OAI/Sets.pm, it was the only one missing TEST PLAN --------- 1. Apply patch 2. prove -v t/db_dependent/OAI/Sets.t -- All 145 tests should run successfully without any error or warning TEST PLAN OPTIONAL ------------------ Go check bug 13899 and see that coverage is 100% for the statements of this module. Coverage BEFORE this patch : Statement : 6,7% Branch : 0,0% Condition : 0,0% Subroutine : 21,1% Pod : 100,0% Coverage AFTER this patch : Statement : 100,0% Branch : 89,5% Condition : 88,8% Subroutine : 100,0% Pod : 100,0% Created attachment 37438 [details] [review] Bug 13940 : Testing C4::OAI::Sets Testing C4::OAI::Sets in t/db_dependent/OAI/Sets.t Adding &DelOAISetsBiblio in @EXPORT at the beginning of OAI/Sets.pm, it was the only one missing TEST PLAN --------- 1. Apply patch 2. prove -v t/db_dependent/OAI/Sets.t -- All 145 tests should run successfully without any error or warning 3. Run koha qa test tools -- No issues found. Additionally, I comfirmed why DelOAISetsBiblio was added in the list of exported functions. $ grep "^sub " C4/OAI/Sets.pm | sort > check1 $ for i in `grep "\&[A-Z]" C4/OAI/Sets.pm | xargs `; do echo $i; done | sort > check2 Of course the outputs have a little extra around them, so $ vi check1 $ vi check2 $ diff check1 check2 Lo and behold, it was that one function. Necessary for 100% coverage. Signed-off-by: Mark Tompsett <mtompset@hotmail.com> Created attachment 37454 [details] [review] [PASSED QA] Bug 13940 : Testing C4::OAI::Sets Testing C4::OAI::Sets in t/db_dependent/OAI/Sets.t Adding &DelOAISetsBiblio in @EXPORT at the beginning of OAI/Sets.pm, it was the only one missing TEST PLAN --------- 1. Apply patch 2. prove -v t/db_dependent/OAI/Sets.t -- All 145 tests should run successfully without any error or warning 3. Run koha qa test tools -- No issues found. Additionally, I comfirmed why DelOAISetsBiblio was added in the list of exported functions. $ grep "^sub " C4/OAI/Sets.pm | sort > check1 $ for i in `grep "\&[A-Z]" C4/OAI/Sets.pm | xargs `; do echo $i; done | sort > check2 Of course the outputs have a little extra around them, so $ vi check1 $ vi check2 $ diff check1 check2 Lo and behold, it was that one function. Necessary for 100% coverage. Signed-off-by: Mark Tompsett <mtompset@hotmail.com> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Comment on attachment 37454 [details] [review] [PASSED QA] Bug 13940 : Testing C4::OAI::Sets >+#Test to enter in the 'else' case of 'AddOAISet' line 280 >+{ >+ my $dbi_st = Test::MockModule->new('DBI::st', no_auto => 1); # ref($sth) == 'DBI::st' >+ $dbi_st->mock('execute', sub { return 0; }); >+ >+ my $setWrong = { >+ 'spec' => 'specWrong', >+ 'name' => 'nameWrong', >+ }; >+ my $setWrong_id = AddOAISet($setWrong); >+ >+ DelOAISet($setWrong_id); >+} It seems to me that you forgot to actually test behaviour here. You are traversing that branch in the code, but still need to test for: - Side effects (no side effects should be tested if such was the case) - The code raises a warning. It whould be tested with a block like this: my $setWrong_id; warning_is ( $setWrong_id = AddOAISet($setWrong) ) 'AddOAISet failed', 'AddOAISet raises warning in XXX condition'; There's also another warning not being tested. You need to add use Test::Warn; for this purpose. I leave you the task to put the pieces together :-D Created attachment 37602 [details] [review] Bug 13940 : Testing C4::OAI::Sets (QA followup) Testing C4::OAI::Sets in t/db_dependent/OAI/Sets.t Adding &DelOAISetsBiblio in @EXPORT at the beginning of OAI/Sets.pm, it was the only one missing To Tomás Cohen Arazi on comment #6 : Adding tests on the 2 warnings not tested with 'warning_is' like you suggested it. Hope it's good enough :) TEST PLAN --------- 1. Apply patch 2. prove -v t/db_dependent/OAI/Sets.t -- All 145 tests should run successfully without any error or warning Maybe I wasn't clear enough. Sorry: The regular workflow when we ask for QA followups is to leave the currently testted patch as-is (with its signoffs, for instance) and provide a "new" patch to be applied on top of the original one. *That* new patch should have "(QA followup)" on its subject. Take a look at bug 13075 for example. You'll see an original bug, and a followup (this time made by me). So, I suggest you obsolete the new one, de-obsolete de passed-qa one, and provide just a small followup with the last changes. Created attachment 37624 [details] [review] Bug 13940 : (QA followup) Add tests for the 2 warnings which was not tested Created attachment 37625 [details] [review] Bug 13940 : (QA followup) Add test for ModOAISet Patches pushed to master. Good job Julian! |