Sometimes 254$a and 254$b are not both defined. This creates noise in Koha/Biblio.pm around line 579ish. This is repeatable by running the gambit of tests and reading the plack-error log file.
Created attachment 91495 [details] [review] Bug 23310: Reduce noise in Koha::Biblio The use of uninitialized value flooding plack-error can get rather distracting and annoying. By using "// q{}" after each of the subfield() calls this eliminates the problem TEST PLAN - run tests before, noise in plack-error log - apply patch - restart_all - run tests again, no Koha/Biblio noise after the restart - run qa test tools
Created attachment 91496 [details] [review] Bug 23310: Reduce noise in Koha::Biblio The use of uninitialized value flooding plack-error can get rather distracting and annoying. By using "// q{}" after each of the subfield() calls this eliminates the problem TEST PLAN --------- vertical break where you stop your cut-and-paste. This test should be repeatable with cut-and-paste easily. echo | sudo tee /var/log/koha/kohadev/plack-error.log restart_all kshell prove t/db_dependent/Koha/Biblio.t cat /var/log/koha/kohadev/plack-error.log --- noise in plack-error log exit git bz apply 23310 echo | sudo tee /var/log/koha/kohadev/plack-error.log restart_all kshell prove t/db_dependent/Koha/Biblio.t cat /var/log/koha/kohadev/plack-error.log --- no noise in plack-error log kshell qa -v 2 -c 1
Created attachment 91497 [details] [review] Bug 23310: Reduce noise in Koha::Biblio The use of uninitialized value flooding plack-error can get rather distracting and annoying. By using "// q{}" after each of the subfield() calls this eliminates the problem TEST PLAN --------- vertical break where you stop your cut-and-paste. This test should be repeatable with cut-and-paste easily. restart_all kshell prove t/db_dependent/Koha/Biblio.t -- may strangely be quiet. prove t/db_dependent/Koha/Biblios.t t/db_dependent/Koha/Biblio.t -- should be noisy. prove t/db_dependent/Koha/Biblio.t -- will always be noisy now. exit git bz apply 23310 restart_all kshell prove t/db_dependent/Koha/Biblio.t -- will be quiet. prove t/db_dependent/Koha/Biblios.t t/db_dependent/Koha/Biblio.t -- will be quiet. prove t/db_dependent/Koha/Biblio.t -- will be quiet. qa -v 2 -c 1
Thanks to kidclamp for a more optimal fix.
Created attachment 91544 [details] [review] Bug 23310: Reduce noise in Koha::Biblio The use of uninitialized value flooding plack-error can get rather distracting and annoying. By using "// q{}" after each of the subfield() calls this eliminates the problem TEST PLAN --------- vertical break where you stop your cut-and-paste. This test should be repeatable with cut-and-paste easily. restart_all kshell prove t/db_dependent/Koha/Biblio.t -- may strangely be quiet. prove t/db_dependent/Koha/Biblios.t t/db_dependent/Koha/Biblio.t -- should be noisy. prove t/db_dependent/Koha/Biblio.t -- will always be noisy now. exit git bz apply 23310 restart_all kshell prove t/db_dependent/Koha/Biblio.t -- will be quiet. prove t/db_dependent/Koha/Biblios.t t/db_dependent/Koha/Biblio.t -- will be quiet. prove t/db_dependent/Koha/Biblio.t -- will be quiet. qa -v 2 -c 1 Signed-off-by: David Nind <david@davidnind.com>
Created attachment 91549 [details] [review] Bug 23310: Reduce noise in Koha::Biblio The use of uninitialized value flooding plack-error can get rather distracting and annoying. By using "// q{}" after each of the subfield() calls this eliminates the problem TEST PLAN --------- vertical break where you stop your cut-and-paste. This test should be repeatable with cut-and-paste easily. restart_all kshell prove t/db_dependent/Koha/Biblio.t -- may strangely be quiet. prove t/db_dependent/Koha/Biblios.t t/db_dependent/Koha/Biblio.t -- should be noisy. prove t/db_dependent/Koha/Biblio.t -- will always be noisy now. exit git bz apply 23310 restart_all kshell prove t/db_dependent/Koha/Biblio.t -- will be quiet. prove t/db_dependent/Koha/Biblios.t t/db_dependent/Koha/Biblio.t -- will be quiet. prove t/db_dependent/Koha/Biblio.t -- will be quiet. qa -v 2 -c 1 Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Nice little fix and I'm adding an SO line to PQA on this bit.. however overall it's a FQA as I'd really like to see a simple regression test added please ;)
Just for the record here are the results of testing. Before applying patch ~~~~~~~~~~~~~~~~~~~~~ root@8ef967b22df7:koha(bz23310)$ prove t/db_dependent/Koha/Biblio.t t/db_dependent/Koha/Biblio.t .. 5/6 Use of uninitialized value in concatenation (.) or string at /kohadevbox/koha/Koha/Biblio.pm line 582. Use of uninitialized value in concatenation (.) or string at /kohadevbox/koha/Koha/Biblio.pm line 582. Use of uninitialized value in concatenation (.) or string at /kohadevbox/koha/Koha/Biblio.pm line 582. t/db_dependent/Koha/Biblio.t .. ok All tests successful. Files=1, Tests=6, 2 wallclock secs ( 0.02 usr 0.01 sys + 1.75 cusr 0.16 csys = 1.94 CPU) Result: PASS root@8ef967b22df7:koha(bz23310)$ prove t/db_dependent/Koha/Biblios.t t/db_dependent/Koha/Biblio.t t/db_dependent/Koha/Biblios.t .. ok t/db_dependent/Koha/Biblio.t ... 4/6 Use of uninitialized value in concatenation (.) or string at /kohadevbox/koha/Koha/Biblio.pm line 582. Use of uninitialized value in concatenation (.) or string at /kohadevbox/koha/Koha/Biblio.pm line 582. Use of uninitialized value in concatenation (.) or string at /kohadevbox/koha/Koha/Biblio.pm line 582. t/db_dependent/Koha/Biblio.t ... ok All tests successful. Files=2, Tests=11, 5 wallclock secs ( 0.03 usr 0.01 sys + 4.43 cusr 0.57 csys = 5.04 CPU) Result: PASS root@8ef967b22df7:koha(bz23310)$ prove t/db_dependent/Koha/Biblio.t t/db_dependent/Koha/Biblio.t .. 4/6 Use of uninitialized value in concatenation (.) or string at /kohadevbox/koha/Koha/Biblio.pm line 582. Use of uninitialized value in concatenation (.) or string at /kohadevbox/koha/Koha/Biblio.pm line 582. Use of uninitialized value in concatenation (.) or string at /kohadevbox/koha/Koha/Biblio.pm line 582. t/db_dependent/Koha/Biblio.t .. ok All tests successful. Files=1, Tests=6, 2 wallclock secs ( 0.02 usr 0.00 sys + 1.71 cusr 0.18 csys = 1.91 CPU) Result: PASS After applying patch ~~~~~~~~~~~~~~~~~~~~ root@8ef967b22df7:koha(bz23310)$ prove t/db_dependent/Koha/Biblio.t t/db_dependent/Koha/Biblio.t .. ok All tests successful. Files=1, Tests=6, 2 wallclock secs ( 0.03 usr 0.00 sys + 1.67 cusr 0.22 csys = 1.92 CPU) Result: PASS root@8ef967b22df7:koha(bz23310)$ prove t/db_dependent/Koha/Biblios.t t/db_dependent/Koha/Biblio.t t/db_dependent/Koha/Biblios.t .. ok t/db_dependent/Koha/Biblio.t ... ok All tests successful. Files=2, Tests=11, 5 wallclock secs ( 0.03 usr 0.01 sys + 4.29 cusr 0.49 csys = 4.82 CPU) Result: PASS root@8ef967b22df7:koha(bz23310)$ prove t/db_dependent/Koha/Biblio.t t/db_dependent/Koha/Biblio.t .. ok All tests successful. Files=1, Tests=6, 2 wallclock secs ( 0.03 usr 0.00 sys + 1.77 cusr 0.16 csys = 1.96 CPU) Result: PASS
- $title = $record->subfield( '245', 'a' ) . $record->subfield( '245', 'b' ); + $title = $record->field('245')->as_string('ab'); Personally, I wouldnt request a test for this change. We are relying here on the module MARC::Field, and we are not testing all its functions?
Happy to be corrected here. Let's set PQA [U+1F600]
Nice work! Pushed to master for 19.11.00