Summary: | _new_Zconn crashes on a bug in t::lib::Mocks::mock_config | ||
---|---|---|---|
Product: | Koha | Reporter: | Marcel de Rooy <m.de.rooy> |
Component: | Architecture, internals, and plumbing | Assignee: | Marcel de Rooy <m.de.rooy> |
Status: | RESOLVED FIXED | QA Contact: | Katrin Fischer <katrin.fischer> |
Severity: | minor | ||
Priority: | P5 - low | CC: | david, jonathan.druart, lucas, nick, tomascohen |
Version: | Main | ||
Hardware: | All | ||
OS: | All | ||
See Also: | https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33277 | ||
Change sponsored?: | --- | Patch complexity: | Small patch |
Documentation contact: | Documentation submission: | ||
Text to go in the release notes: | Version(s) released in: |
23.05.00,22.11.07
|
|
Attachments: |
Bug 33718: Fix the mock of Koha::Config->get in mock_config
Bug 33718: Fix the mock of Koha::Config->get in mock_config Bug 33718: Fix the mock of Koha::Config->get in mock_config Bug 33718: Fix the mock of Koha::Config->get in mock_config Bug 33718: Fix the mock of Koha::Config->get in mock_config |
Description
Marcel de Rooy
2023-05-11 07:12:32 UTC
If it's from tests only you need to call mock_userenv. (In reply to Jonathan Druart from comment #1) > If it's from tests only you need to call mock_userenv. It is a weird error that cannot be reproduced consistently yet. Context passes a second 'listen' parameter to Koha::Config->get and in that routine I did not see that parameter. Needs a bit more digging still. Got it. Patch coming. It is a more fundamental error in t::lib::Mocks. Created attachment 151074 [details] [review] Bug 33718: Fix the mock of Koha::Config->get in mock_config It should not mock calls for other sections than 'config' in koha-conf.xml. Test plan: Without this patch: [1] Enable CatalogModuleRelink and LinkerRelink. This will trigger a SearchAuthorities call when creating a sample biblio. Note: SearchAuthorities calls Zconn and gets back information from a wrong part of koha-conf.xml. [2] Run t/db_dependent/Koha/Pseudonymization.t You should see something like: {UNKNOWN}: Can't use string ("authorities") as a HASH ref while "strict refs" in use at /usr/share/koha/C4/Context.pm line 587. at /usr/share/koha/C4/Biblio.pm line 302 With this patch: [3] Run t/db_dependent/Koha/Pseudonymization.t. Should pass now. Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Created attachment 151075 [details] [review] Bug 33718: Fix the mock of Koha::Config->get in mock_config It should not mock calls for other sections than 'config' in koha-conf.xml. Test plan: Without this patch: [1] Enable CatalogModuleRelink and LinkerRelink. This will trigger a SearchAuthorities call when creating a sample biblio. Note: SearchAuthorities calls Zconn and gets back information from a wrong part of koha-conf.xml. [2] Run t/db_dependent/Koha/Pseudonymization.t You should see something like: {UNKNOWN}: Can't use string ("authorities") as a HASH ref while "strict refs" in use at /usr/share/koha/C4/Context.pm line 587. at /usr/share/koha/C4/Biblio.pm line 302 With this patch: [3] Run t/db_dependent/Koha/Pseudonymization.t. Should pass now. [4] git grep -l mock_config | xargs -i{} prove {} Exclude Mocks.pm. Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Note for QA: The crash refers to authorities (this comes from config/authorityserver in koha-conf while we expected information from the listen section in this line: my $host = _common_config($server, 'listen')->{content}; Created attachment 151101 [details] [review] Bug 33718: Fix the mock of Koha::Config->get in mock_config It should not mock calls for other sections than 'config' in koha-conf.xml. Test plan: Without this patch: [1] Enable CatalogModuleRelink and LinkerRelink. This will trigger a SearchAuthorities call when creating a sample biblio. Note: SearchAuthorities calls Zconn and gets back information from a wrong part of koha-conf.xml. [2] Run t/db_dependent/Koha/Pseudonymization.t You should see something like: {UNKNOWN}: Can't use string ("authorities") as a HASH ref while "strict refs" in use at /usr/share/koha/C4/Context.pm line 587. at /usr/share/koha/C4/Biblio.pm line 302 With this patch: [3] Run t/db_dependent/Koha/Pseudonymization.t. Should pass now. [4] git grep -l mock_config | xargs -i{} prove {} Exclude Mocks.pm. Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Signed-off-by: David Nind <david@davidnind.com> Testing notes (using KTD): 1. I had to enable AutoLinkBiblios to get the error message in step 2 (see comment 59 in bug 33277). 2. Tests pass with no errors after applying the patch. 3. For step 4, I ran: git grep -l mock_config | xargs -i{} prove {} => lots of tests are run and these should all pass except for t/lib/Mocks.pm t/lib/Mocks.pm .. No subtests run Test Summary Report ------------------- t/lib/Mocks.pm (Wstat: 0 Tests: 0 Failed: 0) Parse errors: No plan found in TAP output Files=1, Tests=0, 0 wallclock secs ( 0.01 usr 0.00 sys + 0.16 cusr 0.03 csys = 0.20 CPU) Result: FAIL (In reply to David Nind from comment #8) Thx David. > 1. I had to enable AutoLinkBiblios to get the error message in step 2 (see > comment 59 in bug 33277). I checked my setup. Yes, I had it enabled. Good catch. > t/lib/Mocks.pm .. No subtests run Yes, that was expected :) Created attachment 151110 [details] [review] Bug 33718: Fix the mock of Koha::Config->get in mock_config It should not mock calls for other sections than 'config' in koha-conf.xml. Test plan: Without this patch: [1] Enable AutoLinkBiblios, CatalogModuleRelink and LinkerRelink. This will trigger a SearchAuthorities call when creating a sample biblio. Note: SearchAuthorities calls Zconn and gets back information from a wrong part of koha-conf.xml. [2] Run t/db_dependent/Koha/Pseudonymization.t You should see something like: {UNKNOWN}: Can't use string ("authorities") as a HASH ref while "strict refs" in use at /usr/share/koha/C4/Context.pm line 587. at /usr/share/koha/C4/Biblio.pm line 302 With this patch: [3] Run t/db_dependent/Koha/Pseudonymization.t. Should pass now. [4] git grep -l mock_config | xargs -i{} prove {} Exclude Mocks.pm. Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Signed-off-by: David Nind <david@davidnind.com> Updated the test plan for pref AutoLinkBiblios. Created attachment 151170 [details] [review] Bug 33718: Fix the mock of Koha::Config->get in mock_config It should not mock calls for other sections than 'config' in koha-conf.xml. Test plan: Without this patch: [1] Enable AutoLinkBiblios, CatalogModuleRelink and LinkerRelink. This will trigger a SearchAuthorities call when creating a sample biblio. Note: SearchAuthorities calls Zconn and gets back information from a wrong part of koha-conf.xml. [2] Run t/db_dependent/Koha/Pseudonymization.t You should see something like: {UNKNOWN}: Can't use string ("authorities") as a HASH ref while "strict refs" in use at /usr/share/koha/C4/Context.pm line 587. at /usr/share/koha/C4/Biblio.pm line 302 With this patch: [3] Run t/db_dependent/Koha/Pseudonymization.t. Should pass now. [4] git grep -l mock_config | xargs -i{} prove {} Exclude Mocks.pm. Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> (In reply to Katrin Fischer from comment #12) > Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Thx Pushed to master for 23.05 Nice work everyone! Pushed to 22.11.x for next release |