Description
Tomás Cohen Arazi (tcohen)
2021-09-27 11:16:56 UTC
Created attachment 125352 [details] [review] Bug 29121: Add new plugin exceptions This patch introduces two new exceptions: - Koha::Exceptions::Plugin::InstallDied - Koha::Exceptions::Plugin::UpgradeDied Tests are added for their stringification output. To test: 1. Apply this patch 2. Run: $ kshell k$ prove t/Koha/Exceptions.t => SUCCESS: Tests pass! 3. Sign off :-D Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Created attachment 125353 [details] [review] Bug 29121: Regression tests This patch adds regression tests for broken ->install and ->upgrade methods on plugins. It adds two dummie plugins named BrokenInstall and BrokenUpgrade, for convenience. Tests are added to catch the warnings that will be logged when trying to load this plugins on different scenarios. Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Created attachment 125354 [details] [review] Bug 29121: Catch errors in ->install and ->upgrade calls on plugins This patch adds a try/catch block when instantiating plugins. Calling ->new on a plugin eventually triggers a call to ->install (this has always been like this since bug 7804). If the ->install method is somehow borked, then the process dies. We need to prevent that, and report back some error took place. That's what this patch does. The same happens to the ->upgrade. To test: 1. Install any plugin you like 2. Restart plack (just in case) => SUCCESS: All good 3. Manually change its install method to: sub install { die "plugin, die!"; } 4. Run: $ koha-mysql kohadev > DELETE FROM plugin_data; (to make sure there's no __INSTALLED__ entry, do on a safe to delete DB). 5. Point your browser to the plugins-home.pl page => FAIL: Boom 6. Apply up to the regression tests 7. Run: $ kshell k$ prove t/db_dependent/Koha/Plugins/Plugins.t \ t/Koha/Exceptions.t => FAIL: Tests fail! 8. Apply this patch 9. Repeat 2 => SUCCESS: Tests pass! 10. Run: $ restart_all 11. Repeat 5 => SUCCESS: The page is not broken 12. Sign off :-D Note: I used $ kshell k$ perl misc/devel/install_plugins.pl to test as well. Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Created attachment 125357 [details] [review] Bug 29121: Add new plugin exceptions This patch introduces two new exceptions: - Koha::Exceptions::Plugin::InstallDied - Koha::Exceptions::Plugin::UpgradeDied Tests are added for their stringification output. To test: 1. Apply this patch 2. Run: $ kshell k$ prove t/Koha/Exceptions.t => SUCCESS: Tests pass! 3. Sign off :-D Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Created attachment 125358 [details] [review] Bug 29121: Add new plugin exceptions This patch introduces two new exceptions: - Koha::Exceptions::Plugin::InstallDied - Koha::Exceptions::Plugin::UpgradeDied Tests are added for their stringification output. To test: 1. Apply this patch 2. Run: $ kshell k$ prove t/Koha/Exceptions.t => SUCCESS: Tests pass! 3. Sign off :-D Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Created attachment 125359 [details] [review] Bug 29121: Regression tests This patch adds regression tests for broken ->install and ->upgrade methods on plugins. It adds two dummie plugins named BrokenInstall and BrokenUpgrade, for convenience. Tests are added to catch the warnings that will be logged when trying to load this plugins on different scenarios. Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Created attachment 125360 [details] [review] Bug 29121: Catch errors in ->install and ->upgrade calls on plugins This patch adds a try/catch block when instantiating plugins. Calling ->new on a plugin eventually triggers a call to ->install (this has always been like this since bug 7804). If the ->install method is somehow borked, then the process dies. We need to prevent that, and report back some error took place. That's what this patch does. The same happens to the ->upgrade. To test: 1. Install any plugin you like 2. Restart plack (just in case) => SUCCESS: All good 3. Manually change its install method to: sub install { die "plugin, die!"; } 4. Run: $ koha-mysql kohadev > DELETE FROM plugin_data; (to make sure there's no __INSTALLED__ entry, do on a safe to delete DB). 5. Point your browser to the plugins-home.pl page => FAIL: Boom 6. Apply up to the regression tests 7. Run: $ kshell k$ prove t/db_dependent/Koha/Plugins/Plugins.t \ t/Koha/Exceptions.t => FAIL: Tests fail! 8. Apply this patch 9. Repeat 2 => SUCCESS: Tests pass! 10. Run: $ restart_all 11. Repeat 5 => SUCCESS: The page is not broken 12. Sign off :-D Note: I used $ kshell k$ perl misc/devel/install_plugins.pl to test as well. Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Created attachment 125361 [details] [review] Bug 29121: Add POD to plugins Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Created attachment 125477 [details] [review] Bug 29121: Add new plugin exceptions This patch introduces two new exceptions: - Koha::Exceptions::Plugin::InstallDied - Koha::Exceptions::Plugin::UpgradeDied Tests are added for their stringification output. To test: 1. Apply this patch 2. Run: $ kshell k$ prove t/Koha/Exceptions.t => SUCCESS: Tests pass! 3. Sign off :-D Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Created attachment 125478 [details] [review] Bug 29121: Regression tests This patch adds regression tests for broken ->install and ->upgrade methods on plugins. It adds two dummie plugins named BrokenInstall and BrokenUpgrade, for convenience. Tests are added to catch the warnings that will be logged when trying to load this plugins on different scenarios. Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Created attachment 125479 [details] [review] Bug 29121: Catch errors in ->install and ->upgrade calls on plugins This patch adds a try/catch block when instantiating plugins. Calling ->new on a plugin eventually triggers a call to ->install (this has always been like this since bug 7804). If the ->install method is somehow borked, then the process dies. We need to prevent that, and report back some error took place. That's what this patch does. The same happens to the ->upgrade. To test: 1. Install any plugin you like 2. Restart plack (just in case) => SUCCESS: All good 3. Manually change its install method to: sub install { die "plugin, die!"; } 4. Run: $ koha-mysql kohadev > DELETE FROM plugin_data; (to make sure there's no __INSTALLED__ entry, do on a safe to delete DB). 5. Point your browser to the plugins-home.pl page => FAIL: Boom 6. Apply up to the regression tests 7. Run: $ kshell k$ prove t/db_dependent/Koha/Plugins/Plugins.t \ t/Koha/Exceptions.t => FAIL: Tests fail! 8. Apply this patch 9. Repeat 2 => SUCCESS: Tests pass! 10. Run: $ restart_all 11. Repeat 5 => SUCCESS: The page is not broken 12. Sign off :-D Note: I used $ kshell k$ perl misc/devel/install_plugins.pl to test as well. Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Created attachment 125480 [details] [review] Bug 29121: Add POD to plugins Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Works as expected, code is clear and passes qa scripts Passing QA Pushed to master for 21.11, thanks to everybody involved! Can you please provide a rebased patch set for 21.05.x? Thanks! |