Bug 23310 - Noisy Koha::Biblio
Summary: Noisy Koha::Biblio
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Architecture, internals, and plumbing (show other bugs)
Version: master
Hardware: All All
: P5 - low trivial (vote)
Assignee: Bugs List
QA Contact: Testopia
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2019-07-12 15:28 UTC by Mark Tompsett
Modified: 2021-06-14 21:28 UTC (History)
4 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:
19.11.00


Attachments
Bug 23310: Reduce noise in Koha::Biblio (1.20 KB, patch)
2019-07-12 15:32 UTC, Mark Tompsett
Details | Diff | Splinter Review
Bug 23310: Reduce noise in Koha::Biblio (1.50 KB, patch)
2019-07-12 16:52 UTC, Mark Tompsett
Details | Diff | Splinter Review
Bug 23310: Reduce noise in Koha::Biblio (1.57 KB, patch)
2019-07-12 17:01 UTC, Mark Tompsett
Details | Diff | Splinter Review
Bug 23310: Reduce noise in Koha::Biblio (1.62 KB, patch)
2019-07-16 10:23 UTC, David Nind
Details | Diff | Splinter Review
Bug 23310: Reduce noise in Koha::Biblio (1.68 KB, patch)
2019-07-16 13:10 UTC, Martin Renvoize
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Mark Tompsett 2019-07-12 15:28:57 UTC
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.
Comment 1 Mark Tompsett 2019-07-12 15:32:45 UTC Comment hidden (obsolete)
Comment 2 Mark Tompsett 2019-07-12 16:52:32 UTC Comment hidden (obsolete)
Comment 3 Mark Tompsett 2019-07-12 17:01:52 UTC
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
Comment 4 Mark Tompsett 2019-07-12 17:05:14 UTC
Thanks to kidclamp for a more optimal fix.
Comment 5 David Nind 2019-07-16 10:23:20 UTC
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>
Comment 6 Martin Renvoize 2019-07-16 13:10:36 UTC
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>
Comment 7 Martin Renvoize 2019-07-16 13:11:39 UTC
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 ;)
Comment 8 David Nind 2019-07-17 08:32:49 UTC
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
Comment 9 Marcel de Rooy 2019-10-26 07:20:48 UTC
-        $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?
Comment 10 Martin Renvoize 2019-10-26 07:26:32 UTC
Happy to be corrected here. Let's set PQA 😀
Comment 11 Martin Renvoize 2019-10-28 12:39:06 UTC
Nice work!

Pushed to master for 19.11.00