[Bug 35507] New: TestBuilder.t is failing randomly
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35507 Bug ID: 35507 Summary: TestBuilder.t is failing randomly Change sponsored?: --- Product: Koha Version: unspecified Hardware: All OS: All Status: NEW Severity: normal Priority: P5 - low Component: Test Suite Assignee: chris@bigballofwax.co.nz Reporter: jonathan.druart+koha@gmail.com QA Contact: testopia@bugs.koha-community.org koha_1 | # Failed test 'No encoding warnings!' koha_1 | # at t/db_dependent/TestBuilder.t line 519. koha_1 | # found warning: Failed to load Koha::Plugin::Com::ByWaterSolutions::KitchenSink: Could not find or check module 'Koha::Plugin::Com::ByWaterSolutions::KitchenSink' at /kohadevbox/koha/Koha/Plugins.pm line 124. koha_1 | # didn't expect to find a warning koha_1 | # Looks like you failed 1 test of 1. koha_1 | koha_1 | # Failed test 'build_sample_biblio() tests' koha_1 | # at t/db_dependent/TestBuilder.t line 523. Has been failing on Koha_Master/2759 but fixed in 2760. The output is full of Failed to load Koha::Plugin::Com::ByWaterSolutions::KitchenSink: Could not find or check module 'Koha::Plugin::Com::ByWaterSolutions::KitchenSink' at /kohadevbox/koha/Koha/Plugins.pm line 124. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35507 Jonathan Druart <jonathan.druart+koha@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |katrin.fischer@bsz-bw.de, | |tomascohen@gmail.com -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35507 --- Comment #1 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Failing again with today's run: https://jenkins.koha-community.org/job/Koha_Master/2762/ t_db_dependent_TestBuilder_t.build_sample_biblio() tests (2071) -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35507 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Assignee|chris@bigballofwax.co.nz |m.de.rooy@rijksmuseum.nl CC| |m.de.rooy@rijksmuseum.nl Status|NEW |ASSIGNED --- Comment #2 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Looking here now -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35507 --- Comment #3 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Created attachment 159670 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=159670&action=edit Bug 35507: Clear cache key for enabled plugins at deletion KitchenSink is still in cache key and causes other tests to fail. Test plan: Install some after_biblio_action plugin. Delete the plugin without this patch. Run t/db_dependent/TestBuilder.t [FAIL] Delete plugin with this patch Run t/db_dependent/TestBuilder.t [PASS] -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35507 --- Comment #4 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Will still improve the test plan a bit. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35507 --- Comment #5 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- But it should work: not ok 1 - No encoding warnings! # Failed test 'No encoding warnings!' # at t/db_dependent/TestBuilder.t line 519. # found warning: Failed to load Koha::Plugin::TestItemBarcodeTransform: Could not find or check module 'Koha::Plugin::TestItemBarcodeTransform' at /usr/share/koha/Koha/Plugins.pm line 124. # didn't expect to find a warning # Looks like you failed 1 test of 1. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35507 --- Comment #6 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- With this patch, Plugins.t will remove KitchenSink from the enabled cache key too, resolving issues in other tests. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35507 --- Comment #7 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Should we clear on all plugin-related test files? subtests? Should we generalize this cleanup step? I know we rollback, and then call ->delete() in some places, we should just add this cache cleaning too. $ ls t/db_dependent/Koha/Plugins/ Account_hooks.t* Broken.t* Patron.t* BackgroundJob.t* Circulation_hooks.t* Plugins.t* Barcode_transform_hooks.t* Holds_hooks.t* Recall_hooks.t* Biblio_and_Items_plugin_hooks.t* KitchenSinkPlugin.kpz authority_hooks.t* -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35507 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |jonathan.druart+koha@gmail. | |com, | |kyle@bywatersolutions.com, | |martin.renvoize@ptfs-europe | |.com -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35507 --- Comment #8 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- (In reply to Tomás Cohen Arazi from comment #7)
Should we clear on all plugin-related test files? subtests? Should we generalize this cleanup step? I know we rollback, and then call ->delete() in some places, we should just add this cache cleaning too.
$ ls t/db_dependent/Koha/Plugins/ Account_hooks.t* Broken.t* Patron.t* BackgroundJob.t* Circulation_hooks.t* Plugins.t* Barcode_transform_hooks.t* Holds_hooks.t* Recall_hooks.t* Biblio_and_Items_plugin_hooks.t* KitchenSinkPlugin.kpz authority_hooks.t*
BackgroundJob.t does: $schema->storage->txn_rollback; Koha::Plugins::Methods->delete; Thats definitely in the wrong order. Same for Barcode_transform_hooks.t Copy/paste? Recall_hooks.t Biblio_and_Items_plugin_hooks.t Etc Yes, we should do some cleanup here ! A valid question is however why did we not see this failure much more? It surely depends on the excecution order of test scripts. This still needs a bit more digging. Will try now. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35507 --- Comment #9 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Most of the time it is wrong. If you would have plugins on your system, they are removed too. git grep -C1 -P "Plugins::Methods?->delete" t/db_dependent/Koha/Plugins/Account_hooks.t- t/db_dependent/Koha/Plugins/Account_hooks.t: Koha::Plugins::Methods->delete; t/db_dependent/Koha/Plugins/Account_hooks.t- $schema->storage->txn_rollback; -- t/db_dependent/Koha/Plugins/BackgroundJob.t- $schema->storage->txn_rollback; t/db_dependent/Koha/Plugins/BackgroundJob.t: Koha::Plugins::Methods->delete; t/db_dependent/Koha/Plugins/BackgroundJob.t-}; -- t/db_dependent/Koha/Plugins/Barcode_transform_hooks.t- $schema->storage->txn_rollback; t/db_dependent/Koha/Plugins/Barcode_transform_hooks.t: Koha::Plugins::Methods->delete; t/db_dependent/Koha/Plugins/Barcode_transform_hooks.t-}; -- t/db_dependent/Koha/Plugins/Biblio_and_Items_plugin_hooks.t- $schema->storage->txn_rollback; t/db_dependent/Koha/Plugins/Biblio_and_Items_plugin_hooks.t: Koha::Plugins::Methods->delete; t/db_dependent/Koha/Plugins/Biblio_and_Items_plugin_hooks.t-}; -- t/db_dependent/Koha/Plugins/Circulation_hooks.t- $schema->storage->txn_rollback; t/db_dependent/Koha/Plugins/Circulation_hooks.t: Koha::Plugins::Methods->delete; t/db_dependent/Koha/Plugins/Circulation_hooks.t-}; -- t/db_dependent/Koha/Plugins/Holds_hooks.t- t/db_dependent/Koha/Plugins/Holds_hooks.t: Koha::Plugins::Methods->delete; t/db_dependent/Koha/Plugins/Account_hooks.t- t/db_dependent/Koha/Plugins/Account_hooks.t: Koha::Plugins::Methods->delete; t/db_dependent/Koha/Plugins/Account_hooks.t- $schema->storage->txn_rollback; -- t/db_dependent/Koha/Plugins/BackgroundJob.t- $schema->storage->txn_rollback; t/db_dependent/Koha/Plugins/BackgroundJob.t: Koha::Plugins::Methods->delete; t/db_dependent/Koha/Plugins/BackgroundJob.t-}; -- t/db_dependent/Koha/Plugins/Barcode_transform_hooks.t- $schema->storage->txn_rollback; t/db_dependent/Koha/Plugins/Barcode_transform_hooks.t: Koha::Plugins::Methods->delete; t/db_dependent/Koha/Plugins/Barcode_transform_hooks.t-}; -- t/db_dependent/Koha/Plugins/Biblio_and_Items_plugin_hooks.t- $schema->storage->txn_rollback; t/db_dependent/Koha/Plugins/Biblio_and_Items_plugin_hooks.t: Koha::Plugins::Methods->delete; t/db_dependent/Koha/Plugins/Biblio_and_Items_plugin_hooks.t-}; -- t/db_dependent/Koha/Plugins/Circulation_hooks.t- $schema->storage->txn_rollback; t/db_dependent/Koha/Plugins/Circulation_hooks.t: Koha::Plugins::Methods->delete; t/db_dependent/Koha/Plugins/Circulation_hooks.t-}; -- t/db_dependent/Koha/Plugins/Holds_hooks.t- t/db_dependent/Koha/Plugins/Holds_hooks.t: Koha::Plugins::Methods->delete; t/db_dependent/Koha/Plugins/Holds_hooks.t- $schema->storage->txn_rollback; -- t/db_dependent/Koha/Plugins/Holds_hooks.t- t/db_dependent/Koha/Plugins/Holds_hooks.t: Koha::Plugins::Methods->delete; t/db_dependent/Koha/Plugins/Holds_hooks.t- $schema->storage->txn_rollback; -- t/db_dependent/Koha/Plugins/Holds_hooks.t- t/db_dependent/Koha/Plugins/Holds_hooks.t: Koha::Plugins::Methods->delete; t/db_dependent/Koha/Plugins/Holds_hooks.t- $schema->storage->txn_rollback; -- t/db_dependent/Koha/Plugins/Patron.t- $schema->storage->txn_rollback; t/db_dependent/Koha/Plugins/Patron.t: Koha::Plugins::Methods->delete; t/db_dependent/Koha/Plugins/Patron.t-}; -- t/db_dependent/Koha/Plugins/Plugins.t- # Temporarily remove any installed plugins data t/db_dependent/Koha/Plugins/Plugins.t: Koha::Plugins::Methods->delete; t/db_dependent/Koha/Plugins/Plugins.t- $schema->resultset('PluginData')->delete(); -- t/db_dependent/Koha/Plugins/Plugins.t- # Temporarily remove any installed plugins data t/db_dependent/Koha/Plugins/Plugins.t: Koha::Plugins::Methods->delete; t/db_dependent/Koha/Plugins/Plugins.t- $schema->resultset('PluginData')->delete(); -- t/db_dependent/Koha/Plugins/Plugins.t- # Temporarily remove any installed plugins data t/db_dependent/Koha/Plugins/Plugins.t: Koha::Plugins::Methods->delete; t/db_dependent/Koha/Plugins/Plugins.t- $schema->resultset('PluginData')->delete(); -- root@master:/usr/share/koha# git grep -C1 -P "Plugins::Methods?->delete" t/db_dependent/Koha/Plugins/Account_hooks.t- t/db_dependent/Koha/Plugins/Account_hooks.t: Koha::Plugins::Methods->delete; t/db_dependent/Koha/Plugins/Account_hooks.t- $schema->storage->txn_rollback; -- t/db_dependent/Koha/Plugins/BackgroundJob.t- $schema->storage->txn_rollback; t/db_dependent/Koha/Plugins/BackgroundJob.t: Koha::Plugins::Methods->delete; t/db_dependent/Koha/Plugins/BackgroundJob.t-}; -- t/db_dependent/Koha/Plugins/Barcode_transform_hooks.t- $schema->storage->txn_rollback; t/db_dependent/Koha/Plugins/Barcode_transform_hooks.t: Koha::Plugins::Methods->delete; t/db_dependent/Koha/Plugins/Barcode_transform_hooks.t-}; -- t/db_dependent/Koha/Plugins/Biblio_and_Items_plugin_hooks.t- $schema->storage->txn_rollback; t/db_dependent/Koha/Plugins/Biblio_and_Items_plugin_hooks.t: Koha::Plugins::Methods->delete; t/db_dependent/Koha/Plugins/Biblio_and_Items_plugin_hooks.t-}; -- t/db_dependent/Koha/Plugins/Circulation_hooks.t- $schema->storage->txn_rollback; t/db_dependent/Koha/Plugins/Circulation_hooks.t: Koha::Plugins::Methods->delete; t/db_dependent/Koha/Plugins/Circulation_hooks.t-}; -- t/db_dependent/Koha/Plugins/Holds_hooks.t- t/db_dependent/Koha/Plugins/Holds_hooks.t: Koha::Plugins::Methods->delete; t/db_dependent/Koha/Plugins/Holds_hooks.t- $schema->storage->txn_rollback; -- t/db_dependent/Koha/Plugins/Holds_hooks.t- t/db_dependent/Koha/Plugins/Holds_hooks.t: Koha::Plugins::Methods->delete; t/db_dependent/Koha/Plugins/Holds_hooks.t- $schema->storage->txn_rollback; -- t/db_dependent/Koha/Plugins/Holds_hooks.t- t/db_dependent/Koha/Plugins/Holds_hooks.t: Koha::Plugins::Methods->delete; t/db_dependent/Koha/Plugins/Holds_hooks.t- $schema->storage->txn_rollback; -- t/db_dependent/Koha/Plugins/Patron.t- $schema->storage->txn_rollback; t/db_dependent/Koha/Plugins/Patron.t: Koha::Plugins::Methods->delete; t/db_dependent/Koha/Plugins/Patron.t-}; -- t/db_dependent/Koha/Plugins/Plugins.t- # Temporarily remove any installed plugins data t/db_dependent/Koha/Plugins/Plugins.t: Koha::Plugins::Methods->delete; t/db_dependent/Koha/Plugins/Plugins.t- $schema->resultset('PluginData')->delete(); -- t/db_dependent/Koha/Plugins/Plugins.t- # Temporarily remove any installed plugins data t/db_dependent/Koha/Plugins/Plugins.t: Koha::Plugins::Methods->delete; t/db_dependent/Koha/Plugins/Plugins.t- $schema->resultset('PluginData')->delete(); -- t/db_dependent/Koha/Plugins/Plugins.t- # Temporarily remove any installed plugins data t/db_dependent/Koha/Plugins/Plugins.t: Koha::Plugins::Methods->delete; t/db_dependent/Koha/Plugins/Plugins.t- $schema->resultset('PluginData')->delete(); -- t/db_dependent/Koha/Plugins/Plugins.t- # Temporarily remove any installed plugins data t/db_dependent/Koha/Plugins/Plugins.t: Koha::Plugins::Methods->delete; t/db_dependent/Koha/Plugins/Plugins.t- -- t/db_dependent/Koha/Plugins/Plugins.t- # Make sure there's no previous installs or leftovers on DB t/db_dependent/Koha/Plugins/Plugins.t: Koha::Plugins::Methods->delete; t/db_dependent/Koha/Plugins/Plugins.t- $schema->resultset('PluginData')->delete; -- t/db_dependent/Koha/Plugins/Plugins.t-$schema->storage->txn_begin; t/db_dependent/Koha/Plugins/Plugins.t:Koha::Plugins::Methods->delete; t/db_dependent/Koha/Plugins/Plugins.t-$schema->resultset('PluginData')->delete; -- t/db_dependent/Koha/Plugins/Plugins.t- t/db_dependent/Koha/Plugins/Plugins.t:Koha::Plugins::Methods->delete; -- t/db_dependent/Koha/Plugins/Recall_hooks.t- t/db_dependent/Koha/Plugins/Recall_hooks.t: Koha::Plugins::Methods->delete; t/db_dependent/Koha/Plugins/Recall_hooks.t- $schema->storage->txn_rollback; -- t/db_dependent/Koha/Plugins/authority_hooks.t- $schema->storage->txn_rollback; t/db_dependent/Koha/Plugins/authority_hooks.t: Koha::Plugins::Methods->delete; t/db_dependent/Koha/Plugins/authority_hooks.t-}; root@master:/usr/share/koha# git grep -C1 -P "Plugins::Methods?->delete" > zzz root@master:/usr/share/koha# cat zzz t/db_dependent/Koha/Plugins/Account_hooks.t- t/db_dependent/Koha/Plugins/Account_hooks.t: Koha::Plugins::Methods->delete; t/db_dependent/Koha/Plugins/Account_hooks.t- $schema->storage->txn_rollback; -- t/db_dependent/Koha/Plugins/BackgroundJob.t- $schema->storage->txn_rollback; t/db_dependent/Koha/Plugins/BackgroundJob.t: Koha::Plugins::Methods->delete; t/db_dependent/Koha/Plugins/BackgroundJob.t-}; -- t/db_dependent/Koha/Plugins/Barcode_transform_hooks.t- $schema->storage->txn_rollback; t/db_dependent/Koha/Plugins/Barcode_transform_hooks.t: Koha::Plugins::Methods->delete; t/db_dependent/Koha/Plugins/Barcode_transform_hooks.t-}; -- t/db_dependent/Koha/Plugins/Biblio_and_Items_plugin_hooks.t- $schema->storage->txn_rollback; t/db_dependent/Koha/Plugins/Biblio_and_Items_plugin_hooks.t: Koha::Plugins::Methods->delete; t/db_dependent/Koha/Plugins/Biblio_and_Items_plugin_hooks.t-}; -- t/db_dependent/Koha/Plugins/Circulation_hooks.t- $schema->storage->txn_rollback; t/db_dependent/Koha/Plugins/Circulation_hooks.t: Koha::Plugins::Methods->delete; t/db_dependent/Koha/Plugins/Circulation_hooks.t-}; -- t/db_dependent/Koha/Plugins/Holds_hooks.t- t/db_dependent/Koha/Plugins/Holds_hooks.t: Koha::Plugins::Methods->delete; t/db_dependent/Koha/Plugins/Holds_hooks.t- $schema->storage->txn_rollback; -- t/db_dependent/Koha/Plugins/Holds_hooks.t- t/db_dependent/Koha/Plugins/Holds_hooks.t: Koha::Plugins::Methods->delete; t/db_dependent/Koha/Plugins/Holds_hooks.t- $schema->storage->txn_rollback; -- t/db_dependent/Koha/Plugins/Holds_hooks.t- t/db_dependent/Koha/Plugins/Holds_hooks.t: Koha::Plugins::Methods->delete; t/db_dependent/Koha/Plugins/Holds_hooks.t- $schema->storage->txn_rollback; -- t/db_dependent/Koha/Plugins/Patron.t- $schema->storage->txn_rollback; t/db_dependent/Koha/Plugins/Patron.t: Koha::Plugins::Methods->delete; t/db_dependent/Koha/Plugins/Patron.t-}; -- t/db_dependent/Koha/Plugins/Plugins.t- # Temporarily remove any installed plugins data t/db_dependent/Koha/Plugins/Plugins.t: Koha::Plugins::Methods->delete; t/db_dependent/Koha/Plugins/Plugins.t- $schema->resultset('PluginData')->delete(); -- t/db_dependent/Koha/Plugins/Plugins.t- # Temporarily remove any installed plugins data t/db_dependent/Koha/Plugins/Plugins.t: Koha::Plugins::Methods->delete; t/db_dependent/Koha/Plugins/Plugins.t- $schema->resultset('PluginData')->delete(); -- t/db_dependent/Koha/Plugins/Plugins.t- # Temporarily remove any installed plugins data t/db_dependent/Koha/Plugins/Plugins.t: Koha::Plugins::Methods->delete; t/db_dependent/Koha/Plugins/Plugins.t- $schema->resultset('PluginData')->delete(); -- t/db_dependent/Koha/Plugins/Plugins.t- # Temporarily remove any installed plugins data t/db_dependent/Koha/Plugins/Plugins.t: Koha::Plugins::Methods->delete; t/db_dependent/Koha/Plugins/Plugins.t- -- t/db_dependent/Koha/Plugins/Plugins.t- # Make sure there's no previous installs or leftovers on DB t/db_dependent/Koha/Plugins/Plugins.t: Koha::Plugins::Methods->delete; t/db_dependent/Koha/Plugins/Plugins.t- $schema->resultset('PluginData')->delete; -- t/db_dependent/Koha/Plugins/Plugins.t-$schema->storage->txn_begin; t/db_dependent/Koha/Plugins/Plugins.t:Koha::Plugins::Methods->delete; t/db_dependent/Koha/Plugins/Plugins.t-$schema->resultset('PluginData')->delete; -- t/db_dependent/Koha/Plugins/Plugins.t- t/db_dependent/Koha/Plugins/Plugins.t:Koha::Plugins::Methods->delete; -- t/db_dependent/Koha/Plugins/Recall_hooks.t- t/db_dependent/Koha/Plugins/Recall_hooks.t: Koha::Plugins::Methods->delete; t/db_dependent/Koha/Plugins/Recall_hooks.t- $schema->storage->txn_rollback; -- t/db_dependent/Koha/Plugins/authority_hooks.t- $schema->storage->txn_rollback; t/db_dependent/Koha/Plugins/authority_hooks.t: Koha::Plugins::Methods->delete; t/db_dependent/Koha/Plugins/authority_hooks.t-}; -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35507 --- Comment #10 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- === Preliminary conclusion When you call InstallPlugins, or plugin->enable or ->disable, the cache will already be cleared. This covers quite a few cases. But not all, explaining for a bit the reduced number of failures. Things are definitely related to the KitchenSink stuff in Plugins.t and the order of execution. What about temporarily disabling plugins in TestBuilder.t ? Or in build_sample_biblio ? Does not sound very nice. Not the way to go imo. Adding a clear cache in Methods->delete probably works? Yes, we even catch the Handler->delete case too. Since Handler->delete calls uninstall on the plugin. Deletes from plugin_data. And does Methods->search->delete! But it does not feel like the best solution imo. The actions for deleting records in plugin_data and plugin_methods should be done on a general level and not be repeated in all those test scripts etc. (Which requires more code changes definitely. But reordering all deletes after rollback does require a lot of those changes too..) We need a simple fix that can be backported to versions having bug 29672 (i.e. 22.11) And an enhancement taking care of the cleanup and wrong order stuff. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35507 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |35536 --- Comment #11 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Adding 35536 on top of this one. Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35536 [Bug 35536] Improve cleanup/uninstall of Koha plugins -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35507 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #159670|0 |1 is obsolete| | --- Comment #12 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Comment on attachment 159670 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=159670 Bug 35507: Clear cache key for enabled plugins at deletion Obsoleting in favor of another approach. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35507 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Keywords| |RM_priority -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35507 --- Comment #13 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Is this ready for SO? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35507 --- Comment #14 from Tomás Cohen Arazi <tomascohen@gmail.com> --- (In reply to Marcel de Rooy from comment #8)
BackgroundJob.t does: $schema->storage->txn_rollback; Koha::Plugins::Methods->delete; Thats definitely in the wrong order.
I recall we did it that way because InstallPlugins triggered a commit on the transaction. So we decided 'plugin tests will be considered destructive' (in terms of the DB data) and did it this way so there's no test plugin information on the DB when other tests are run. Maybe that's no longer needed. But I don't recall any recent changes in the area. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35507 --- Comment #15 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- (In reply to Katrin Fischer from comment #13)
Is this ready for SO?
No, I removed the patch :) -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35507 --- Comment #16 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- (In reply to Tomás Cohen Arazi from comment #14)
I recall we did it that way because InstallPlugins triggered a commit on the transaction. So we decided 'plugin tests will be considered destructive' (in terms of the DB data) and did it this way so there's no test plugin information on the DB when other tests are run.
Interesting! I am not sure where that implicit commit hides? Do you see it? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35507 --- Comment #17 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Lots of fun here in the meantime. Discovered that we miss a rollback in Plugins.t too. And running prove Plugins.t ends up with additional data in a clean master install: +----------------------------------------+-----------------------+--------------+ | plugin_class | plugin_key | plugin_value | +----------------------------------------+-----------------------+--------------+ | Koha::Plugin::BadAPIRoute | __INSTALLED_VERSION__ | 0.01 | | Koha::Plugin::MarcFieldValues | __INSTALLED_VERSION__ | 1 | | Koha::Plugin::Test | __ENABLED__ | 0 | | Koha::Plugin::Test | __INSTALLED__ | 1 | | Koha::Plugin::Test | __INSTALLED_VERSION__ | v1.01 | | Koha::Plugin::TestItemBarcodeTransform | __INSTALLED_VERSION__ | 1.01 | +----------------------------------------+-----------------------+--------------+ Still WIP -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35507 --- Comment #18 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Weird stuff. Debugging Plugins.t gives me the following: Between those two lines: [1] my $classname = ref($plugin); AND [2] like( $plugin->test_template($fn), qr/^I am $fn/, 'Template works' ); There must be an implicit commit somewhere. If you end on the classname line, you will have no committed data, after the test_template line you have. Note that test_template calls get_template_and_user (and checkauth etc) in C4/Auth. Somewhere we commit the changes in plugin_data and plugin_methods. This is costing me too much time now to continue. Sorry ;) Assistance appreciated.. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35507 --- Comment #19 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Created attachment 159739 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=159739&action=edit Bug 35507: Add Plugins::Method->delete Delete record in plugin_data too and clear cache. Note: Adding the singular delete, makes the plural loop and call the singular in Koha::Objects. Clearing the cache is done in the store_data call of Koha::Plugins. Test plan: Run t/db_dependent/Koha/Plugins/Method.t Run other Koha plugin tests: -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35507 --- Comment #20 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Attached a patch still WIP. Before entering the rabbit hole of Plugins.t -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35507 --- Comment #21 from Tomás Cohen Arazi <tomascohen@gmail.com> --- (In reply to Marcel de Rooy from comment #19)
Created attachment 159739 [details] [review] Bug 35507: Add Plugins::Method->delete
Delete record in plugin_data too and clear cache.
Note: Adding the singular delete, makes the plural loop and call the singular in Koha::Objects. Clearing the cache is done in the store_data call of Koha::Plugins.
Test plan: Run t/db_dependent/Koha/Plugins/Method.t Run other Koha plugin tests:
(In reply to Marcel de Rooy from comment #18)
Weird stuff. Debugging Plugins.t gives me the following:
Between those two lines:
[1] my $classname = ref($plugin);
AND
[2] like( $plugin->test_template($fn), qr/^I am $fn/, 'Template works' );
There must be an implicit commit somewhere. If you end on the classname line, you will have no committed data, after the test_template line you have.
Note that test_template calls get_template_and_user (and checkauth etc) in C4/Auth. Somewhere we commit the changes in plugin_data and plugin_methods.
This is costing me too much time now to continue. Sorry ;) Assistance appreciated..
CGI::Session \o/ -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35507 --- Comment #22 from Kyle M Hall <kyle@bywatersolutions.com> --- (In reply to Marcel de Rooy from comment #19)
Created attachment 159739 [details] [review] Bug 35507: Add Plugins::Method->delete
Delete record in plugin_data too and clear cache.
Note: Adding the singular delete, makes the plural loop and call the singular in Koha::Objects. Clearing the cache is done in the store_data call of Koha::Plugins.
Test plan: Run t/db_dependent/Koha/Plugins/Method.t Run other Koha plugin tests:
Have you tried enabling DBIC_TRACE? I don't if it will show the implicit commit: DBIC_TRACE=1 prove t/db_dependent/TestBuilder.t -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35507 --- Comment #23 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- (In reply to Tomás Cohen Arazi from comment #21)
CGI::Session
\o/
Yes, I ended with some suspicion about the session flushes (incl Search history). -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35507 --- Comment #24 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- (In reply to Kyle M Hall from comment #22)
(In reply to Marcel de Rooy from comment #19)
Created attachment 159739 [details] [review] [review] Bug 35507: Add Plugins::Method->delete
Delete record in plugin_data too and clear cache.
Note: Adding the singular delete, makes the plural loop and call the singular in Koha::Objects. Clearing the cache is done in the store_data call of Koha::Plugins.
Test plan: Run t/db_dependent/Koha/Plugins/Method.t Run other Koha plugin tests:
Have you tried enabling DBIC_TRACE? I don't if it will show the implicit commit: DBIC_TRACE=1 prove t/db_dependent/TestBuilder.t
Will do. Thx -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35507 --- Comment #25 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- (In reply to Marcel de Rooy from comment #24)
DBIC_TRACE=1 prove t/db_dependent/TestBuilder.t
DBIC seems ok. Running with DBI_TRACE might show more? Seeing in that trace rollback ineffective with AutoCommit enabled at /usr/share/perl5/DBIx/Class/Storage/DBI.pm line 1651. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35507 --- Comment #26 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- <- prepare('SELECT flag, user_permissions.code FROM user_permissions JOIN permissions USING (module_bit, code) JOIN userflags ON (module_bit = bit) JOIN borrowers USING (borrowernumber) WHERE userid = ?')= ( DBI::st=HASH(0x55b072677978) ) [1 items] at Auth.pm line 2165 <- execute(undef)= ( '0E0' ) [1 items] at Auth.pm line 2171 !! The ERROR '2000' was CLEARED by call to fetch method <- fetchrow_hashref= ( undef ) [1 items] at Auth.pm line 2174 <- DESTROY(DBI::st=HASH(0x55b072677bb8))= ( undef ) [1 items] at Auth.pm line 2177 <- DESTROY(DBI::st=HASH(0x55b072677558))= ( undef ) [1 items] at Auth.pm line 2133 <- DESTROY(DBI::st=HASH(0x55b0726772b8))= ( undef ) [1 items] at Auth.pm line 2307 <- FETCH('Active')= ( 1 ) [1 items] at DBI.pm line 939 rollback ineffective with AutoCommit enabled at /usr/share/perl5/DBIx/Class/Storage/DBI.pm line 1651. <- rollback= ( '' ) [1 items] at DBI.pm line 1651 <- disconnect_all= ( ) [0 items] (not implemented) at DBI.pm line 758 # Looks like you planned 62 tests but ran 40. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35507 --- Comment #27 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- get_template_and_user => haspermission => .. => get_user_subpermissions -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35507 --- Comment #28 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Tomas, maybe this works from auth_authenticate_api_request.t : # FIXME: CGI::Session::Driver::DBI explicitly sets AutoCommit=1 [1] which breaks the rollback in out tests. # Until we change into some other library, set SessionStorage to 'tmp' # [1] https://metacpan.org/source/CGI::Session::Driver::DBI#L28 t::lib::Mocks::mock_preference( 'SessionStorage', 'tmp' ); -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35507 --- Comment #29 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Not whole story yet, but now seeing this in KitchenSink: return C4::Context->dbh->do( " CREATE TABLE IF NOT EXISTS $table ( `borrowernumber` INT( 11 ) NOT NULL ) ENGINE = INNODB; " ); Thats a commit ! -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35507 --- Comment #30 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- What do you folks think: Shall we remove the KitchenSink stuff from Plugins.t since it implicitly commits cruft in the database ? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35507 --- Comment #31 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- (In reply to Marcel de Rooy from comment #30)
What do you folks think: Shall we remove the KitchenSink stuff from Plugins.t since it implicitly commits cruft in the database ?
I would opt for moving it in a separate test and creating the database before installing the plugin (it includes an exist test). Controlling what we 'commit' when creating. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35507 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #159739|0 |1 is obsolete| | --- Comment #32 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Created attachment 159744 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=159744&action=edit Bug 35507: Add Plugins::Method->delete Delete record in plugin_data too and clear cache. Note: Adding the singular delete, makes the plural loop and call the singular in Koha::Objects. Clearing the cache is done in the store_data call of Koha::Plugins. Test plan: Run prove t/db_dependent/Koha/Plugins Run prove $(git grep -l Koha::Plugin | grep -P "^t\/db" | grep -v "Koha/Plugins") 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=35507 --- Comment #33 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Created attachment 159745 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=159745&action=edit Bug 35507: Plugins.t - Mock SessionStorage, remove KitchenSink Removing the CGI::Session behavior to auto-commit by mocking SessionStorage makes that the $plugin->test_template does no longer commit. Since KitchenSink includes a CREATE TABLE, removing that code removes the second implicit commit. We might move that code to its own script? Adds a missing rollback too at the end. Test plan: Run prove t/db_dependent/Koha/Plugins/Plugins.t 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=35507 --- Comment #34 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Tomas: We get now one failure here: Koha/REST/Plugin/PluginRoutes.t ..... 3/4 # Failed test 'Plugin enabled, route defined as C4::Context->needs_install is false' # at Koha/REST/Plugin/PluginRoutes.t line 247. # Looks like you failed 1 test of 2. Koha/REST/Plugin/PluginRoutes.t ..... 4/4 # Failed test 'needs_install use case tests' # at Koha/REST/Plugin/PluginRoutes.t line 252. # Looks like you failed 1 test of 4. Koha/REST/Plugin/PluginRoutes.t ..... Dubious, test returned 1 (wstat 256, 0x100) Failed 1/4 subtests It seems that this test assumes that the plugin is there or so, and now no longer is? Can you confirm? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35507 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |35548 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35548 [Bug 35548] Move KitchenSink test on its own and control table creation -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35507 --- Comment #35 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Still thinking too about the REPLACE with 0 in Method->delete. We could also remove but then we need to explicitly clear cache. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35507 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|ASSIGNED |Needs Signoff Patch complexity|--- |Small patch -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35507 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #159744|0 |1 is obsolete| | Attachment #159745|0 |1 is obsolete| | --- Comment #36 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Created attachment 159801 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=159801&action=edit Bug 35507: Plugins.t - Mock SessionStorage, remove KitchenSink Removing the CGI::Session behavior to auto-commit by mocking SessionStorage makes that the $plugin->test_template does no longer commit. Since KitchenSink includes a CREATE TABLE, removing that code removes the second implicit commit. We might move that code to its own script? Adds a missing rollback too at the end. Test plan: Run prove t/db_dependent/Koha/Plugins/Plugins.t Verify that no records got added to plugin_data/methods table. 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=35507 --- Comment #37 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Created attachment 159802 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=159802&action=edit Bug 35507: Add Plugins::Method->delete Delete record in plugin_data too and clear cache. Note: Adding the singular delete, makes the plural loop and call the singular in Koha::Objects. Clearing the cache is done in the store_data call of Koha::Plugins. Test plan: Run prove prove $(git grep -l Koha::Plugin | grep -P "^t\/db") 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=35507 --- Comment #38 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- (In reply to Marcel de Rooy from comment #34)
Tomas:
We get now one failure here:
Koha/REST/Plugin/PluginRoutes.t ..... 3/4 # Failed test 'Plugin enabled, route defined as C4::Context->needs_install is false' # at Koha/REST/Plugin/PluginRoutes.t line 247. # Looks like you failed 1 test of 2. Koha/REST/Plugin/PluginRoutes.t ..... 4/4 # Failed test 'needs_install use case tests' # at Koha/REST/Plugin/PluginRoutes.t line 252. # Looks like you failed 1 test of 4. Koha/REST/Plugin/PluginRoutes.t ..... Dubious, test returned 1 (wstat 256, 0x100) Failed 1/4 subtests
It seems that this test assumes that the plugin is there or so, and now no longer is? Can you confirm?
This seems no longer relevant. Will add a tiny fix on the enh report for this unit test btw. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35507 --- Comment #39 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- (In reply to Marcel de Rooy from comment #35)
Still thinking too about the REPLACE with 0 in Method->delete. We could also remove but then we need to explicitly clear cache.
I did now. Remove instead of disable. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35507 --- Comment #40 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- NOTE FOR QA: This is a small fix that could be backported. Enhancing it and preventing data loss on the enh report. And separate report for restoring KitchenSink stuff. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35507 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Summary|TestBuilder.t is failing |Fix handling plugins in |randomly |unit tests causing random | |failures on Jenkins -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35507 --- Comment #41 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- (In reply to Marcel de Rooy from comment #37)
Note: Adding the singular delete, makes the plural loop and call the singular in Koha::Objects. Clearing the cache is done in the store_data call of Koha::Plugins.
Hmm. I updated that. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35507 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #159802|0 |1 is obsolete| | --- Comment #42 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Created attachment 159803 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=159803&action=edit Bug 35507: Add Plugins::Method->delete Delete record in plugin_data too and clear cache. Note: Adding the singular delete, makes the plural loop and call the singular in Koha::Objects. Test plan: Run prove prove $(git grep -l Koha::Plugin | grep -P "^t\/db") 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=35507 Jonathan Druart <jonathan.druart+koha@gmail.com> 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=35507 Jonathan Druart <jonathan.druart+koha@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #159801|0 |1 is obsolete| | Attachment #159803|0 |1 is obsolete| | --- Comment #43 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- Created attachment 159817 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=159817&action=edit Bug 35507: Plugins.t - Mock SessionStorage, remove KitchenSink Removing the CGI::Session behavior to auto-commit by mocking SessionStorage makes that the $plugin->test_template does no longer commit. Since KitchenSink includes a CREATE TABLE, removing that code removes the second implicit commit. We might move that code to its own script? Adds a missing rollback too at the end. Test plan: Run prove t/db_dependent/Koha/Plugins/Plugins.t Verify that no records got added to plugin_data/methods table. Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> 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=35507 --- Comment #44 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- Created attachment 159818 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=159818&action=edit Bug 35507: Add Plugins::Method->delete Delete record in plugin_data too and clear cache. Note: Adding the singular delete, makes the plural loop and call the singular in Koha::Objects. Test plan: Run prove prove $(git grep -l Koha::Plugin | grep -P "^t\/db") Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> 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=35507 --- Comment #45 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- Trusting Marcel to continue the work he has started here. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35507 --- Comment #46 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- (In reply to Jonathan Druart from comment #45)
Trusting Marcel to continue the work he has started here.
Bug 35536 has been submitted now. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35507 --- Comment #47 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Comment on attachment 159818 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=159818 Bug 35507: Add Plugins::Method->delete Review of attachment 159818: --> (https://bugs.koha-community.org/bugzilla3/page.cgi?id=splinter.html&bug=35507&attachment=159818) ----------------------------------------------------------------- ::: Koha/Plugins/Method.pm @@ +43,5 @@
+ my ($self) = @_; + + # Remove corresponding records from plugin_data. Clear cache. + my $cond = { plugin_class => $self->plugin_class }; + Koha::Database->new->schema->resultset('PluginData')->search($cond)->delete;
This scares me.. delete is called from inside InstallPlugins.. InstallPlugins is called from upload-plugin.pl and misc/devel/install_plugins.pl So.. on first look, this appears to me that for any new plugin installed will nuke all data for all other plugins in the system at the time. Plugin upgrades too. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35507 --- Comment #48 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- InstallPlugins contains Koha::Plugins::Methods->search({ plugin_class => $plugin_class })->delete(); So this is restricted to $plugin_class. What is scary exactly? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35507 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |ASSIGNED -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35507 --- Comment #49 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- (In reply to Marcel de Rooy from comment #48)
InstallPlugins contains Koha::Plugins::Methods->search({ plugin_class => $plugin_class })->delete();
So this is restricted to $plugin_class. What is scary exactly?
Scary: removing from plugin_data too. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35507 --- Comment #50 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- (In reply to Marcel de Rooy from comment #29)
Not whole story yet, but now seeing this in KitchenSink:
return C4::Context->dbh->do( " CREATE TABLE IF NOT EXISTS $table ( `borrowernumber` INT( 11 ) NOT NULL ) ENGINE = INNODB; " );
Thats a commit !
And the DROP TABLE as well :) -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35507 --- Comment #51 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- (In reply to Marcel de Rooy from comment #35)
Still thinking too about the REPLACE with 0 in Method->delete. We could also remove but then we need to explicitly clear cache.
Martin: This was a mistake in hindsight. Before this change, I just disabled. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35507 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #159818|0 |1 is obsolete| | --- Comment #52 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Comment on attachment 159818 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=159818 Bug 35507: Add Plugins::Method->delete Going back to older version and extending test a little bit -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35507 --- Comment #53 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Hmm reintroducing this guy now: t/db_dependent/Koha/REST/Plugin/PluginRoutes.t ............... 3/4 # Failed test 'Plugin enabled, route defined as C4::Context->needs_install is false' # at t/db_dependent/Koha/REST/Plugin/PluginRoutes.t line 247. # Looks like you failed 1 test of 2. t/db_dependent/Koha/REST/Plugin/PluginRoutes.t ............... 4/4 # Failed test 'needs_install use case tests' # at t/db_dependent/Koha/REST/Plugin/PluginRoutes.t line 252. # Looks like you failed 1 test of 4. t/db_dependent/Koha/REST/Plugin/PluginRoutes.t ............... Dubious, test returned 1 (wstat 256, 0x100) -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35507 --- Comment #54 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Created attachment 159846 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=159846&action=edit Bug 35507: Add Plugins::Method->delete Delete record in plugin_data too and clear cache. Note: Adding the singular delete, makes the plural loop and call the singular in Koha::Objects. Clearing the cache is done in the store_data call of Koha::Plugins. Test plan: Run prove $(git grep -l Koha::Plugin | grep -P "^t\/db") 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=35507 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|ASSIGNED |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35507 --- Comment #55 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Created attachment 159847 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=159847&action=edit Bug 35507: (follow-up) Fix PluginRoutes.t Why this works, is a mystery to me :) Test plan: Run t/db_dependent/Koha/REST/Plugin/PluginRoutes.t 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=35507 Kyle M Hall <kyle@bywatersolutions.com> 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=35507 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #159817|0 |1 is obsolete| | Attachment #159846|0 |1 is obsolete| | Attachment #159847|0 |1 is obsolete| | --- Comment #56 from Kyle M Hall <kyle@bywatersolutions.com> --- Created attachment 159848 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=159848&action=edit Bug 35507: Plugins.t - Mock SessionStorage, remove KitchenSink Removing the CGI::Session behavior to auto-commit by mocking SessionStorage makes that the $plugin->test_template does no longer commit. Since KitchenSink includes a CREATE TABLE, removing that code removes the second implicit commit. We might move that code to its own script? Adds a missing rollback too at the end. Test plan: Run prove t/db_dependent/Koha/Plugins/Plugins.t Verify that no records got added to plugin_data/methods table. Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35507 --- Comment #57 from Kyle M Hall <kyle@bywatersolutions.com> --- Created attachment 159849 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=159849&action=edit Bug 35507: Add Plugins::Method->delete Delete record in plugin_data too and clear cache. Note: Adding the singular delete, makes the plural loop and call the singular in Koha::Objects. Clearing the cache is done in the store_data call of Koha::Plugins. Test plan: Run prove $(git grep -l Koha::Plugin | grep -P "^t\/db") Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35507 --- Comment #58 from Kyle M Hall <kyle@bywatersolutions.com> --- Created attachment 159850 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=159850&action=edit Bug 35507: (follow-up) Fix PluginRoutes.t Why this works, is a mystery to me :) Test plan: Run t/db_dependent/Koha/REST/Plugin/PluginRoutes.t Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35507 --- Comment #59 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- (In reply to Kyle M Hall from comment #58)
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Thanks! -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35507 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |BLOCKED --- Comment #60 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Still thinking about a potential issue -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35507 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|BLOCKED |Signed Off -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35507 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #159849|0 |1 is obsolete| | Attachment #159850|0 |1 is obsolete| | --- Comment #61 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Created attachment 159860 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=159860&action=edit Bug 35507: Add verbose parameters in Koha::Plugins Yet another approach ;) Driemaal is scheepsrecht / Third time lucky.. Test plan: Run prove $(git grep -l Koha::Plugin | grep -P "^t\/db") 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=35507 --- Comment #62 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- I finally got the least harmful approach now. Removing the Method->delete here and postponing that approach to 35536 and adding a verbose parameter in Koha::Plugins at the can_load calls. Testing the verbose parameter and further refinements, all go to 35536 now. We spent way too much time here already. Moving the mysterious PluginRoutes.t issue to 35536 too. Final round for Martin hopefully now? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35507 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- QA Contact|testopia@bugs.koha-communit |martin.renvoize@ptfs-europe |y.org |.com -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35507 --- Comment #63 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- prove $(git grep -l Koha::Plugin | grep -P "^t\/db") t/db_dependent/ImportBatch.t ................................. ok t/db_dependent/Koha/Patron/Consents.t ........................ ok t/db_dependent/Koha/Patron_generate_userid.t ................. ok t/db_dependent/Koha/Plugins/Account_hooks.t .................. ok t/db_dependent/Koha/Plugins/BackgroundJob.t .................. ok t/db_dependent/Koha/Plugins/Barcode_transform_hooks.t ........ ok t/db_dependent/Koha/Plugins/Biblio_and_Items_plugin_hooks.t .. ok t/db_dependent/Koha/Plugins/Broken.t ......................... ok t/db_dependent/Koha/Plugins/Circulation_hooks.t .............. ok t/db_dependent/Koha/Plugins/Holds_hooks.t .................... ok t/db_dependent/Koha/Plugins/Patron.t ......................... 1/5 Plugin error (Test Plugin): Exception 'Koha::Exception' thrown 'patron_barcode_transform called with parameter: test_cn_1' Plugin error (Test Plugin): Exception 'Koha::Exception' thrown 'patron_barcode_transform called with parameter: test_cn_1' Plugin error (Test Plugin): Exception 'Koha::Exception' thrown 'patron_barcode_transform called with parameter: test_cn_1' t/db_dependent/Koha/Plugins/Patron.t ......................... ok t/db_dependent/Koha/Plugins/Plugins.t ........................ ok t/db_dependent/Koha/Plugins/Recall_hooks.t ................... ok t/db_dependent/Koha/Plugins/authority_hooks.t ................ ok t/db_dependent/Koha/REST/Plugin/PluginRoutes.t ............... ok t/db_dependent/Koha/Template/Plugin/KohaPlugins.t ............ ok All tests successful. Files=16, Tests=135, 131 wallclock secs ( 0.21 usr 0.06 sys + 122.23 cusr 5.97 csys = 128.47 CPU) Result: PASS -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35507 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #159848|0 |1 is obsolete| | --- Comment #64 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 159861 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=159861&action=edit Bug 35507: Plugins.t - Mock SessionStorage, remove KitchenSink Removing the CGI::Session behavior to auto-commit by mocking SessionStorage makes that the $plugin->test_template does no longer commit. Since KitchenSink includes a CREATE TABLE, removing that code removes the second implicit commit. We might move that code to its own script? Adds a missing rollback too at the end. Test plan: Run prove t/db_dependent/Koha/Plugins/Plugins.t Verify that no records got added to plugin_data/methods table. Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35507 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #159860|0 |1 is obsolete| | --- Comment #65 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 159862 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=159862&action=edit Bug 35507: Add verbose parameters in Koha::Plugins Yet another approach ;) Driemaal is scheepsrecht / Third time lucky.. Test plan: Run prove $(git grep -l Koha::Plugin | grep -P "^t\/db") Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35507 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Passed QA --- Comment #66 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Great work, thanks for all the effort. No regressions found now, QA scripts happy and tests are now consistently passing :) Passing QA -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35507 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Version(s)| |24.05.00 released in| | Status|Passed QA |Pushed to master -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35507 --- Comment #67 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Pushed for 24.05! Well done everyone, thank you! -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35507 Fridolin Somers <fridolin.somers@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Pushed to master |Pushed to stable CC| |fridolin.somers@biblibre.co | |m Version(s)|24.05.00 |24.05.00,23.11.02 released in| | --- Comment #68 from Fridolin Somers <fridolin.somers@biblibre.com> --- Pushed to 23.11.x for 23.11.02 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35507 Lucas Gass <lucas@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |lucas@bywatersolutions.com --- Comment #69 from Lucas Gass <lucas@bywatersolutions.com> --- Doesn't apply cleanly to 23.05.x, no backport. If needed please rebase. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35507 Lucas Gass <lucas@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Pushed to stable |Pushed to oldstable Version(s)|24.05.00,23.11.02 |24.05.00,23.11.02,23.05.09 released in| | --- Comment #70 from Lucas Gass <lucas@bywatersolutions.com> --- Backported to 23.05.x for upcoming 23.05.09. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35507 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Keywords|RM_priority | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35507 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC|martin.renvoize@ptfs-europe | |.com | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35507 Wainui Witika-Park <wainuiwitikapark@catalyst.net.nz> changed: What |Removed |Added ---------------------------------------------------------------------------- Resolution|--- |FIXED CC| |wainuiwitikapark@catalyst.n | |et.nz Status|Pushed to oldstable |RESOLVED --- Comment #71 from Wainui Witika-Park <wainuiwitikapark@catalyst.net.nz> --- Not backporting to 22.11 unless requested -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon＠bugs.koha-community.org