Summary: | Eliminate warning on undefined branchcode | ||
---|---|---|---|
Product: | Koha | Reporter: | Marcel de Rooy <m.de.rooy> |
Component: | Notices | Assignee: | Marcel de Rooy <m.de.rooy> |
Status: | CLOSED FIXED | QA Contact: | Testopia <testopia> |
Severity: | trivial | ||
Priority: | P5 - low | CC: | fridolin.somers, gmcharlt, mtompset |
Version: | Main | ||
Hardware: | All | ||
OS: | All | ||
GIT URL: | Change sponsored?: | --- | |
Patch complexity: | Trivial patch | Documentation contact: | |
Documentation submission: | Text to go in the release notes: | ||
Version(s) released in: | Circulation function: | ||
Attachments: |
Bug 11732: Eliminate warning on undefined branchcode
Bug 11732: Eliminate warning on undefined branchcode [SIGNED OFF] Bug 11732: Eliminate warning on undefined branchcode Bug 11732: Eliminate warning on undefined branchcode Bug 11732: Eliminate warning on undefined branchcode |
Description
Marcel de Rooy
2014-02-10 12:28:11 UTC
Created attachment 25171 [details] [review] Bug 11732: Eliminate warning on undefined branchcode When you run the Reserves test, you have the warnings: Use of uninitialized value $branchcode in hash element at /usr/share/koha/testclone/C4/Letters.pm line 138. Use of uninitialized value $branchcode in hash element at /usr/share/koha/testclone/C4/Letters.pm line 148. This patch removes that warning. Test plan: Run the Reserves.t again. I tried to test this, but test 7 fails when I run: prove -v t/db_dependent/Reserves.t And an error message is not triggered even in master. I suspect I don't have things set to pretend to be logged in. This eyeballs just fine, but I was wondering... why not move line 129 to after the if? And why not // instead of ||? Sorry, I couldn't trigger this. Created attachment 25208 [details] [review] Bug 11732: Eliminate warning on undefined branchcode When you run the Reserves test, you have the warnings: Use of uninitialized value $branchcode in hash element at /usr/share/koha/testclone/C4/Letters.pm line 138. Use of uninitialized value $branchcode in hash element at /usr/share/koha/testclone/C4/Letters.pm line 148. This patch removes that warning. Test plan: Run the Reserves.t again. (In reply to M. Tompsett from comment #2) > I tried to test this, but test 7 fails when I run: > prove -v t/db_dependent/Reserves.t > And an error message is not triggered even in master. I suspect I don't have > things set to pretend to be logged in. The test creates branches and categories, even sets a pref, so this apparently requires more digging into other circulation settings.. It is interesting to know why it fails (with the AddReturn call, I understand), but this report just fixes a trivial uninitialized value. > This eyeballs just fine, but I was wondering... > why not move line 129 to after the if? > And why not // instead of ||? Fine with me. New patch submitted. I still can't trigger it with t/db_dependent/Reserves.t However, I did trigger your patch with: perl -e "use C4::Letters; my \$blah=C4::Letters::getletter('circulation','DUE');" This got me thinking. Shouldn't I be able to trigger this with: prove -v t/Letters.t OR prove -v t/db_dependent/Letters.t ? At which point I discovered some rather ugly failings. Could you be explicit to the point of annoyance to confirm that I am trying to test it the same way, and have the same settings, and have similar data which triggers the error in master? (In reply to M. Tompsett from comment #5) > Could you be explicit to the point of annoyance to confirm that I am trying > to test it the same way, and have the same settings, and have similar data > which triggers the error in master? No annoyance as long as you keep testing :) t/db_dependent/Letters.t does indeed produce some ugly errors with me too. Does not call getletter. t/Letters.t does not call getletter; so will not trigger the patch too. The call ..$var=getletter(..).. will imo not trigger the patch; userenv will return false. Note that Reserves.t also overrides userenv on line 23. We could do the same: perl -e "use C4::Letters; *C4::Context::userenv= sub { return {} }; my $blah=C4::Letters::getletter('circulation','DUE', 'BRA');" Without the patch, you have a warning. With the patch, you will have not. Created attachment 25230 [details] [review] [SIGNED OFF] Bug 11732: Eliminate warning on undefined branchcode When you run the Reserves test, you have the warnings: Use of uninitialized value $branchcode in hash element at /usr/share/koha/testclone/C4/Letters.pm line 138. Use of uninitialized value $branchcode in hash element at /usr/share/koha/testclone/C4/Letters.pm line 148. This patch removes that warning. Test plan: Run the Reserves.t again. Revised Test Plan ----------------- Run the following on the command line prompt before and after applying the patch: perl -e "use C4::Letters; *C4::Context::userenv= sub { return {} }; my \$blah=C4::Letters::getletter('circulation','DUE', 'BRA');" Before the patch there will be errors (as above), after there will not. Signed-off-by: Mark Tompsett <mtompset@hotmail.com> Created attachment 25274 [details] [review] Bug 11732: Eliminate warning on undefined branchcode When you run the Reserves test, you have the warnings: Use of uninitialized value $branchcode in hash element at /usr/share/koha/testclone/C4/Letters.pm line 138. Use of uninitialized value $branchcode in hash element at /usr/share/koha/testclone/C4/Letters.pm line 148. This patch removes that warning. Test plan: Run the Reserves.t again. Revised Test Plan ----------------- Run the following on the command line prompt before and after applying the patch: perl -e "use C4::Letters; *C4::Context::userenv= sub { return {} }; my \$blah=C4::Letters::getletter('circulation','DUE', 'BRA');" Before the patch there will be errors (as above), after there will not. Signed-off-by: Mark Tompsett <mtompset@hotmail.com> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Created attachment 25275 [details] [review] Bug 11732: Eliminate warning on undefined branchcode When you run the Reserves test, you have the warnings: Use of uninitialized value $branchcode in hash element at /usr/share/koha/testclone/C4/Letters.pm line 138. Use of uninitialized value $branchcode in hash element at /usr/share/koha/testclone/C4/Letters.pm line 148. This patch removes that warning. Test plan: Run the Reserves.t again. Revised Test Plan ----------------- Run the following on the command line prompt before and after applying the patch: perl -e "use C4::Letters; *C4::Context::userenv= sub { return {} }; my \$blah=C4::Letters::getletter('circulation','DUE', 'BRA');" Before the patch there will be errors (as above), after there will not. Signed-off-by: Mark Tompsett <mtompset@hotmail.com> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com> IndependentBranches must be on. Pushed to master. Thanks, Marcel! Pushed to 3.14.x, will be in 3.14.07 |