[Bug 16502] New: Table koha_plugin_com_bywatersolutions_kitchensink_mytable not always dropped after running Plugin.t
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16502 Bug ID: 16502 Summary: Table koha_plugin_com_bywatersolutions_kitchensink_mytable not always dropped after running Plugin.t Change sponsored?: --- Product: Koha Version: master Hardware: All OS: All Status: NEW Severity: normal Priority: P5 - low Component: Architecture, internals, and plumbing Assignee: gmcharlt@gmail.com Reporter: m.de.rooy@rijksmuseum.nl QA Contact: testopia@bugs.koha-community.org Happened to notice that running this test (t/db_dependent/Plugin.t) twice may not have the expected result. Sometimes(?) I need to manually drop the table. Note that quickly adding a transaction does not resolve it. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16502 --- Comment #1 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Created attachment 51544 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=51544&action=edit Bug 16502: Table koha_plugin_com_bywatersolutions_kitchensink_mytable not always dropped after running Plugin.t If you run Plugin.t, the above table will still be present (when you did not enable UseKohaPlugins). This would trigger a warning when running the test a second time. Why? The uninstall call does its work not completely due to a small inconsistency in Koha::Plugins::Handler::delete when calling run without the enable_plugins parameter. This patch resolves that inconsistency and also removes an unneeded skip in Plugin.t in case the KitchenSink module already exists. Note: This is a small fix. But I wonder if the Handler routines run and delete should not have been implemented in Koha::Plugins::Base. Also note that plugins/plugins-uninstall.pl will not be affacted by this change, since it checks whether the pref is enabled before calling the delete method. Test plan: [1] Do not yet install this patch. [2] Verify that plugins are enabled in koha-conf.xml. [3] Disable UseKohaPlugins in System Preferences! [4] Run t/db_dependent/Plugins.t. [5] Verify that table koha_plugin_com_bywatersolutions_kitchensink_mytable still exists. (It should have been deleted.) Remove it manually. [6] Apply this patch. [7] Run the test again. [8] Verify that the table does not exist. [9] Run the test again (without warnings). -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16502 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Patch complexity|--- |Small patch Status|NEW |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16502 --- Comment #2 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Created attachment 51545 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=51545&action=edit Bug 16502: Add additional test to Plugins.t In order to verify if the delete now really works, we add one test in Plugins.t. Test plan: [1] Run the test. [2] Bonus: Comment line 63 in Plugins.t where delete is called. Run the test again. It should fail now. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16502 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Assignee|gmcharlt@gmail.com |m.de.rooy@rijksmuseum.nl -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16502 Srdjan Jankovic <srdjan@catalyst.net.nz> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |srdjan@catalyst.net.nz --- Comment #3 from Srdjan Jankovic <srdjan@catalyst.net.nz> --- There's something odd with that t/db_dependent/Plugins.t. Without the patch, I've changed it: diff --git a/t/db_dependent/Plugins.t b/t/db_dependent/Plugins.t index 3271b1a..c5faf87 100755 --- a/t/db_dependent/Plugins.t +++ b/t/db_dependent/Plugins.t @@ -43,7 +43,7 @@ ok( $plugin->get_qualified_table_name('mytable') eq 'koha_plugin_test_mytable', ok( $plugin->get_plugin_http_path() eq '/plugin/Koha/Plugin/Test', 'Test $plugin->get_plugin_http_path()' ); my @plugins = Koha::Plugins->new({ enable_plugins => 1 })->GetPlugins( 'report' ); -ok( $plugins[0]->get_metadata()->{'name'} eq 'Test Plugin', "Koha::Plugins::GetPlugins functions correctly" ); +is( $plugins[0]->get_metadata()->{'name'}, 'Test Plugin', "Koha::Plugins::GetPlugins functions correctly" ); SKIP: { my $plugins_dir = C4::Context->config("pluginsdir"); and I'm getting not ok 18 - Koha::Plugins::GetPlugins functions correctly # Failed test 'Koha::Plugins::GetPlugins functions correctly' # at t/db_dependent/Plugins.t line 46. # got: 'Example Kitchen-Sink Plugin' # expected: 'Test Plugin' -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16502 --- Comment #4 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- (In reply to Srdjan Jankovic from comment #3)
There's something odd with that t/db_dependent/Plugins.t. ... and I'm getting not ok 18 - Koha::Plugins::GetPlugins functions correctly # Failed test 'Koha::Plugins::GetPlugins functions correctly' # at t/db_dependent/Plugins.t line 46. # got: 'Example Kitchen-Sink Plugin' # expected: 'Test Plugin'
Srdjan, I made the same change and it seems to work with me. But actually this test is based on the assumption that GetPlugins always finds the Test Plugin first. Which could very well not be the case. Since there can be a lot of plugins present, the test should only verify if the Test Plugin is in the array. Also note that the array is not sorted by the routine, so this should have been taken in account too. Adding a follow-up. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16502 --- Comment #5 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Created attachment 51713 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=51713&action=edit Bug 16502: Adjust test for GetPlugins The current test assumes that GetPlugins will return the test plugin as the first one in the array. This is not correct. This patch adjusts the test to a grep. Test plan: Run the test. Bonus: Add additional plugins. Run the test again. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16502 --- Comment #6 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Created attachment 51714 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=51714&action=edit Bug 16502: Replace a few other ok-calls by is-calls Trivial changes that speak for themselves.. Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16502 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |16586 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16586 [Bug 16586] Koha Plugins: Limit results of GetPlugins by metadata -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16502 Srdjan Jankovic <srdjan@catalyst.net.nz> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #51545|0 |1 is obsolete| | --- Comment #7 from Srdjan Jankovic <srdjan@catalyst.net.nz> --- Created attachment 51804 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=51804&action=edit [SIGNED-OFF] Bug 16502: Add additional test to Plugins.t In order to verify if the delete now really works, we add one test in Plugins.t. Test plan: [1] Run the test. [2] Bonus: Comment line 63 in Plugins.t where delete is called. Run the test again. It should fail now. Signed-off-by: Srdjan <srdjan@catalyst.net.nz> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16502 Srdjan Jankovic <srdjan@catalyst.net.nz> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #51713|0 |1 is obsolete| | --- Comment #8 from Srdjan Jankovic <srdjan@catalyst.net.nz> --- Created attachment 51805 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=51805&action=edit [SIGNED-OFF] Bug 16502: Adjust test for GetPlugins The current test assumes that GetPlugins will return the test plugin as the first one in the array. This is not correct. This patch adjusts the test to a grep. Test plan: Run the test. Bonus: Add additional plugins. Run the test again. Signed-off-by: Srdjan <srdjan@catalyst.net.nz> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16502 Srdjan Jankovic <srdjan@catalyst.net.nz> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Signed Off -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16502 Srdjan Jankovic <srdjan@catalyst.net.nz> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #51714|0 |1 is obsolete| | --- Comment #9 from Srdjan Jankovic <srdjan@catalyst.net.nz> --- Created attachment 51806 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=51806&action=edit [SIGNED-OFF] Bug 16502: Replace a few other ok-calls by is-calls Trivial changes that speak for themselves.. Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Signed-off-by: Srdjan <srdjan@catalyst.net.nz> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16502 --- Comment #10 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Thanks Srdjan for signing off. I notice that the first patch did not get your signoff line. Was that intentional or just forgotten? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16502 --- Comment #11 from Srdjan Jankovic <srdjan@catalyst.net.nz> --- Apologies, my mistake, did not adjust for the additional patch. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16502 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #51544|0 |1 is obsolete| | --- Comment #12 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Created attachment 51853 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=51853&action=edit Bug 16502: Table koha_plugin_com_bywatersolutions_kitchensink_mytable not always dropped after running Plugin.t If you run Plugin.t, the above table will still be present (when you did not enable UseKohaPlugins). This would trigger a warning when running the test a second time. Why? The uninstall call does its work not completely due to a small inconsistency in Koha::Plugins::Handler::delete when calling run without the enable_plugins parameter. This patch resolves that inconsistency and also removes an unneeded skip in Plugin.t in case the KitchenSink module already exists. Note: This is a small fix. But I wonder if the Handler routines run and delete should not have been implemented in Koha::Plugins::Base. Also note that plugins/plugins-uninstall.pl will not be affacted by this change, since it checks whether the pref is enabled before calling the delete method. Test plan: [1] Do not yet install this patch. [2] Verify that plugins are enabled in koha-conf.xml. [3] Disable UseKohaPlugins in System Preferences! [4] Run t/db_dependent/Plugins.t. [5] Verify that table koha_plugin_com_bywatersolutions_kitchensink_mytable still exists. (It should have been deleted.) Remove it manually. [6] Apply this patch. [7] Run the test again. [8] Verify that the table does not exist. [9] Run the test again (without warnings). Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Signed-off-by: Srdjan <srdjan@catalyst.net.nz> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16502 --- Comment #13 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Created attachment 51854 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=51854&action=edit Bug 16502: Add additional test to Plugins.t In order to verify if the delete now really works, we add one test in Plugins.t. Test plan: [1] Run the test. [2] Bonus: Comment line 63 in Plugins.t where delete is called. Run the test again. It should fail now. Signed-off-by: Srdjan <srdjan@catalyst.net.nz> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16502 --- Comment #14 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Created attachment 51855 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=51855&action=edit Bug 16502: Adjust test for GetPlugins The current test assumes that GetPlugins will return the test plugin as the first one in the array. This is not correct. This patch adjusts the test to a grep. Test plan: Run the test. Bonus: Add additional plugins. Run the test again. Signed-off-by: Srdjan <srdjan@catalyst.net.nz> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16502 --- Comment #15 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Created attachment 51856 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=51856&action=edit Bug 16502: Replace a few other ok-calls by is-calls Trivial changes that speak for themselves.. Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Signed-off-by: Srdjan <srdjan@catalyst.net.nz> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16502 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |m.de.rooy@rijksmuseum.nl -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16502 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #51804|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16502 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #51805|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16502 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #51806|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16502 --- Comment #16 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- (In reply to Srdjan Jankovic from comment #11)
Apologies, my mistake, did not adjust for the additional patch.
OK I added your signoff line. Thx -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16502 --- Comment #17 from Srdjan Jankovic <srdjan@catalyst.net.nz> --- I meant to do it sometimes today, after settling current affairs. But this is ok too. Thanks. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16502 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Passed QA -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16502 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #51853|0 |1 is obsolete| | Attachment #51854|0 |1 is obsolete| | Attachment #51855|0 |1 is obsolete| | Attachment #51856|0 |1 is obsolete| | --- Comment #18 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 52260 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=52260&action=edit Bug 16502: Table koha_plugin_com_bywatersolutions_kitchensink_mytable not always dropped after running Plugin.t If you run Plugin.t, the above table will still be present (when you did not enable UseKohaPlugins). This would trigger a warning when running the test a second time. Why? The uninstall call does its work not completely due to a small inconsistency in Koha::Plugins::Handler::delete when calling run without the enable_plugins parameter. This patch resolves that inconsistency and also removes an unneeded skip in Plugin.t in case the KitchenSink module already exists. Note: This is a small fix. But I wonder if the Handler routines run and delete should not have been implemented in Koha::Plugins::Base. Also note that plugins/plugins-uninstall.pl will not be affacted by this change, since it checks whether the pref is enabled before calling the delete method. Test plan: [1] Do not yet install this patch. [2] Verify that plugins are enabled in koha-conf.xml. [3] Disable UseKohaPlugins in System Preferences! [4] Run t/db_dependent/Plugins.t. [5] Verify that table koha_plugin_com_bywatersolutions_kitchensink_mytable still exists. (It should have been deleted.) Remove it manually. [6] Apply this patch. [7] Run the test again. [8] Verify that the table does not exist. [9] Run the test again (without warnings). Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Signed-off-by: Srdjan <srdjan@catalyst.net.nz> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16502 --- Comment #19 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 52261 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=52261&action=edit Bug 16502: Add additional test to Plugins.t In order to verify if the delete now really works, we add one test in Plugins.t. Test plan: [1] Run the test. [2] Bonus: Comment line 63 in Plugins.t where delete is called. Run the test again. It should fail now. Signed-off-by: Srdjan <srdjan@catalyst.net.nz> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16502 --- Comment #20 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 52262 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=52262&action=edit Bug 16502: Adjust test for GetPlugins The current test assumes that GetPlugins will return the test plugin as the first one in the array. This is not correct. This patch adjusts the test to a grep. Test plan: Run the test. Bonus: Add additional plugins. Run the test again. Signed-off-by: Srdjan <srdjan@catalyst.net.nz> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16502 --- Comment #21 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 52263 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=52263&action=edit Bug 16502: Replace a few other ok-calls by is-calls Trivial changes that speak for themselves.. Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Signed-off-by: Srdjan <srdjan@catalyst.net.nz> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16502 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Passed QA |Pushed to Master CC| |kyle@bywatersolutions.com --- Comment #22 from Kyle M Hall <kyle@bywatersolutions.com> --- Pushed to master for 16.11, thanks Marcel! -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16502 Frédéric Demians <frederic@tamil.fr> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |frederic@tamil.fr Status|Pushed to Master |Pushed to Stable --- Comment #23 from Frédéric Demians <frederic@tamil.fr> --- Pushed in 16.05. Will be in 16.05.01. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16502 Julian Maurice <julian.maurice@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |julian.maurice@biblibre.com --- Comment #24 from Julian Maurice <julian.maurice@biblibre.com> --- Patches pushed to 3.22.x, will be in 3.22.9 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16502 Chris Cormack <chris@bigballofwax.co.nz> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |chris@bigballofwax.co.nz --- Comment #25 from Chris Cormack <chris@bigballofwax.co.nz> --- Pushed to 3.20.x will be in 3.20.13 -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon＠bugs.koha-community.org