From 6621378ae08e2e273fe445ae6c45c9e92b357800 Mon Sep 17 00:00:00 2001 From: Philipp Wolfer Date: Wed, 13 Sep 2023 07:18:52 +0200 Subject: [PATCH] On plugin import error unset sys.modules again --- picard/pluginmanager.py | 6 +++- .../testplugins/importerror/dummyplugin.py | 32 +++++++++++++++++ test/test_plugins.py | 36 +++++++++++++------ 3 files changed, 63 insertions(+), 11 deletions(-) create mode 100644 test/data/testplugins/importerror/dummyplugin.py diff --git a/picard/pluginmanager.py b/picard/pluginmanager.py index f5719023b0..0e2a3bfae8 100644 --- a/picard/pluginmanager.py +++ b/picard/pluginmanager.py @@ -324,7 +324,11 @@ def _load_plugin(self, name): # module twice. This executes the plugins code twice and leads # to potential side effects. sys.modules[full_module_name] = plugin_module - spec.loader.exec_module(plugin_module) + try: + spec.loader.exec_module(plugin_module) + except: # noqa: E722 + del sys.modules[full_module_name] + raise plugin = PluginWrapper(plugin_module, plugin_dir, file=module_pathname, manifest_data=manifest_data) diff --git a/test/data/testplugins/importerror/dummyplugin.py b/test/data/testplugins/importerror/dummyplugin.py new file mode 100644 index 0000000000..bbd70d6940 --- /dev/null +++ b/test/data/testplugins/importerror/dummyplugin.py @@ -0,0 +1,32 @@ +# -*- coding: utf-8 -*- +# +# Picard, the next-generation MusicBrainz tagger +# +# Copyright (C) 2019-2021 Laurent Monin +# +# This program is free software; you can redistribute it and/or +# modify it under the terms of the GNU General Public License +# as published by the Free Software Foundation; either version 2 +# of the License, or (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program; if not, write to the Free Software +# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. + + +"""Dummy plugin for tests""" +PLUGIN_NAME = "Dummy plugin" +PLUGIN_AUTHOR = "Zas" +PLUGIN_DESCRIPTION = "Dummy plugin description" +PLUGIN_VERSION = "1.0" +PLUGIN_API_VERSIONS = ["2.0"] +PLUGIN_LICENSE = 'Dummy plugin license' +PLUGIN_LICENSE_URL = 'dummy.plugin.url' + + +raise ImportError diff --git a/test/test_plugins.py b/test/test_plugins.py index f56bfb7aba..6985d0a118 100644 --- a/test/test_plugins.py +++ b/test/test_plugins.py @@ -229,23 +229,32 @@ def test_plugin_install_no_path_no_plugin_name(self): class TestPicardPluginsLoad(TestPicardPluginsCommonTmpDir): - def _test_plugin_load_from_directory(self, name): - pm = PluginManager(plugins_directory=self.tmp_directory) + def setUp(self): + super().setUp() + self.pm = PluginManager(plugins_directory=self.tmp_directory) + self.src_dir = None + + def tearDown(self): + super().tearDown() + unload_plugin('picard.plugins.dummyplugin') + if self.src_dir: + _plugin_dirs.remove(self.src_dir) - src_dir = os.path.dirname(_testplugins[name]) - register_plugin_dir(src_dir) + def _register_plugin_dir(self, name): + self.src_dir = os.path.dirname(_testplugins[name]) + register_plugin_dir(self.src_dir) - msg = "plugins_load_from_directory: %s %r" % (name, src_dir) - pm.load_plugins_from_directory(src_dir) - self.assertEqual(len(pm.plugins), 1, msg) - self.assertEqual(pm.plugins[0].name, 'Dummy plugin', msg) + def _test_plugin_load_from_directory(self, name): + self._register_plugin_dir(name) + msg = "plugins_load_from_directory: %s %r" % (name, self.src_dir) + self.pm.load_plugins_from_directory(self.src_dir) + self.assertEqual(len(self.pm.plugins), 1, msg) + self.assertEqual(self.pm.plugins[0].name, 'Dummy plugin', msg) # if module is properly loaded, this should work from picard.plugins.dummyplugin import DummyPlugin DummyPlugin() - _plugin_dirs.remove(src_dir) - # singlefile def test_plugin_load_from_directory_singlefile(self): self._test_plugin_load_from_directory('singlefile') @@ -262,6 +271,13 @@ def test_plugin_load_from_directory_zipped_singlefile(self): def test_plugin_load_from_directory_module(self): self._test_plugin_load_from_directory('module') + def test_plugin_import_error(self): + module_name = 'picard.plugins.dummyplugin' + self.assertIsNone(sys.modules.get(module_name)) + self._register_plugin_dir('importerror') + self.pm.load_plugins_from_directory(self.src_dir) + self.assertIsNone(sys.modules.get(module_name)) + class TestPluginWrapper(PicardTestCase):