Bug 23310

Summary: Noisy Koha::Biblio
Product: Koha Reporter: M. Tompsett <mtompset>
Component: Architecture, internals, and plumbingAssignee: Bugs List <koha-bugs>
Status: Pushed to master --- QA Contact: Testopia <testopia>
Severity: trivial    
Priority: P5 - low CC: david, hannah.co, m.de.rooy, martin.renvoize
Version: master   
Hardware: All   
OS: All   
Change sponsored?: --- Patch complexity: ---
Who signed the patch off: Text to go in the release notes:
Version(s) released in:
19.11.00
Attachments: Bug 23310: Reduce noise in Koha::Biblio
Bug 23310: Reduce noise in Koha::Biblio
Bug 23310: Reduce noise in Koha::Biblio
Bug 23310: Reduce noise in Koha::Biblio
Bug 23310: Reduce noise in Koha::Biblio

Description M. 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 M. Tompsett 2019-07-12 15:32:45 UTC Comment hidden (obsolete)
Comment 2 M. Tompsett 2019-07-12 16:52:32 UTC Comment hidden (obsolete)
Comment 3 M. 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 M. 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