Bug 36917 - Many warnings from t/db_dependent/Authority/Merge.t
Summary: Many warnings from t/db_dependent/Authority/Merge.t
Status: RESOLVED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Test Suite (show other bugs)
Version: unspecified
Hardware: All All
: P5 - low minor
Assignee: Jonathan Druart
QA Contact: Marcel de Rooy
URL:
Keywords:
Depends on: 30047
Blocks: 25515
  Show dependency treegraph
 
Reported: 2024-05-22 08:54 UTC by Jonathan Druart
Modified: 2024-07-04 18:36 UTC (History)
4 users (show)

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


Attachments
Bug 36917: Remove some warnings from t/db_dependent/Authority/Merge.t (1.52 KB, patch)
2024-05-22 08:57 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 36917: Remove some warnings from t/db_dependent/Authority/Merge.t (1.61 KB, patch)
2024-05-22 11:37 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 36917: Remove some warnings from t/db_dependent/Authority/Merge.t (2.09 KB, patch)
2024-05-22 12:02 UTC, Nick Clemens (kidclamp)
Details | Diff | Splinter Review
Bug 36917: (follow-up) Remove Elasticsearch warnings and tidy (2.01 KB, patch)
2024-05-22 12:02 UTC, Nick Clemens (kidclamp)
Details | Diff | Splinter Review
Bug 36917: (follow-up) Add FIXME for marcflavour (997 bytes, patch)
2024-05-22 12:02 UTC, Nick Clemens (kidclamp)
Details | Diff | Splinter Review
Bug 36917: (follow-up) Remove Elasticsearch warnings and tidy (2.08 KB, patch)
2024-05-22 13:05 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 36917: (follow-up) Add FIXME for marcflavour (1.05 KB, patch)
2024-05-22 13:05 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 36917: (follow-up) Remove Elasticsearch warnings and tidy (2.17 KB, patch)
2024-05-24 09:27 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 36917: (follow-up) Add FIXME for marcflavour (1.14 KB, patch)
2024-05-24 09:27 UTC, Marcel de Rooy
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Jonathan Druart 2024-05-22 08:54:49 UTC
Cannot determine authority type for record: 1709 at /kohadevbox/koha/Koha/SearchEngine/Elasticsearch.pm line 589.                                                                                                    
Use of uninitialized value $subfields in pattern match (m//) at /kohadevbox/koha/C4/Heading/MARC21.pm line 412.                                                                                                      
Use of uninitialized value $subfields in pattern match (m//) at /kohadevbox/koha/C4/Heading/MARC21.pm line 448.
Comment 1 Jonathan Druart 2024-05-22 08:55:59 UTC
I think this is because tag 109 is not defined in C4::Heading::MARC21::$auth_heading_fields

What is 109 here?
Comment 2 Jonathan Druart 2024-05-22 08:57:37 UTC
Created attachment 167012 [details] [review]
Bug 36917: Remove some warnings from t/db_dependent/Authority/Merge.t

Cannot determine authority type for record: 1709 at /kohadevbox/koha/Koha/SearchEngine/Elasticsearch.pm line 589.
Use of uninitialized value $subfields in pattern match (m//) at /kohadevbox/koha/C4/Heading/MARC21.pm line 412.
Use of uninitialized value $subfields in pattern match (m//) at /kohadevbox/koha/C4/Heading/MARC21.pm line 448.

This change removes the 2 "Use of uninitialized value $subfields in
pattern match (m//)" warnings

To be honest I am not really sure what I am doing here but this looks
safe enough.

Can one of you confirm?
Comment 3 Marcel de Rooy 2024-05-22 11:12:42 UTC
(In reply to Jonathan Druart from comment #2)
> Can one of you confirm?

Will have a look.
Note that this test ran fine for years so there must be some new trigger ?
Comment 4 Nick Clemens (kidclamp) 2024-05-22 11:29:08 UTC
Probably storing the heading from bug 30047
Comment 5 Marcel de Rooy 2024-05-22 11:32:55 UTC
This might be an issue:

return {} if !$authtype;    # very exceptional
Comment 6 Marcel de Rooy 2024-05-22 11:37:54 UTC
Created attachment 167023 [details] [review]
Bug 36917: Remove some warnings from t/db_dependent/Authority/Merge.t

Cannot determine authority type for record: 1709 at /kohadevbox/koha/Koha/SearchEngine/Elasticsearch.pm line 589.
Use of uninitialized value $subfields in pattern match (m//) at /kohadevbox/koha/C4/Heading/MARC21.pm line 412.
Use of uninitialized value $subfields in pattern match (m//) at /kohadevbox/koha/C4/Heading/MARC21.pm line 448.

This change removes the 2 "Use of uninitialized value $subfields in
pattern match (m//)" warnings

To be honest I am not really sure what I am doing here but this looks
safe enough.

Can one of you confirm?

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 7 Nick Clemens (kidclamp) 2024-05-22 12:02:53 UTC
Created attachment 167025 [details] [review]
Bug 36917: Remove some warnings from t/db_dependent/Authority/Merge.t

Cannot determine authority type for record: 1709 at /kohadevbox/koha/Koha/SearchEngine/Elasticsearch.pm line 589.
Use of uninitialized value $subfields in pattern match (m//) at /kohadevbox/koha/C4/Heading/MARC21.pm line 412.
Use of uninitialized value $subfields in pattern match (m//) at /kohadevbox/koha/C4/Heading/MARC21.pm line 448.

This change removes the 2 "Use of uninitialized value $subfields in
pattern match (m//)" warnings

The warnings are gnerated because the subfields for headings are hardcoded variables in C4::Heading::MARC21
for any genuine authority record we should receive the correct list of subfields. The tests in this case are
mocking a new authtype, and so do not find the correct subfields. Skipping the generation here silences the
warnings and will prevent user defined types from throwing errors as well.

NOTE: Koha allows the user to define their own authority types, however, we have various features hardcoded
that will prevent them from working as expected

WNC amended commit message

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 8 Nick Clemens (kidclamp) 2024-05-22 12:02:55 UTC
Created attachment 167026 [details] [review]
Bug 36917: (follow-up) Remove Elasticsearch warnings and tidy

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 9 Nick Clemens (kidclamp) 2024-05-22 12:02:57 UTC
Created attachment 167027 [details] [review]
Bug 36917: (follow-up) Add FIXME for marcflavour

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 10 Nick Clemens (kidclamp) 2024-05-22 12:03:20 UTC
Back to you to review my follow-ups Joubu
Comment 11 Jonathan Druart 2024-05-22 13:05:27 UTC
Created attachment 167033 [details] [review]
Bug 36917: (follow-up) Remove Elasticsearch warnings and tidy

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 12 Jonathan Druart 2024-05-22 13:05:30 UTC
Created attachment 167034 [details] [review]
Bug 36917: (follow-up) Add FIXME for marcflavour

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 13 Jonathan Druart 2024-05-22 13:05:45 UTC
(In reply to Nick Clemens (kidclamp) from comment #10)
> Back to you to review my follow-ups Joubu

Done, thanks!
Comment 14 Marcel de Rooy 2024-05-24 09:27:32 UTC
Created attachment 167154 [details] [review]
Bug 36917: (follow-up) Remove Elasticsearch warnings and tidy

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 15 Marcel de Rooy 2024-05-24 09:27:34 UTC
Created attachment 167155 [details] [review]
Bug 36917: (follow-up) Add FIXME for marcflavour

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 16 Katrin Fischer 2024-05-24 13:40:23 UTC
Pushed for 24.05!

Well done everyone, thank you!
Comment 17 Fridolin Somers 2024-05-30 08:10:55 UTC
Depends on Bug 30047 not in 23.11.x
Comment 18 Caroline Cyr La Rose 2024-07-04 18:36:02 UTC
Test suite changes don't induce changes to the manual afaik. Closing, but feel free to reopen if the status was put there to document something in the coding guidelines