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.
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