On plugin import error unset sys.modules again

metabrainz · Sep 13, 2023 · 6d57c8f · 6d57c8f
1 parent d89934e
commit 6d57c8f
Show file tree

Hide file tree

Showing 2 changed files with 30 additions and 11 deletions.
diff --git 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/test_plugins.py b/test/test_plugins.py
@@ -229,23 +229,31 @@ 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
 
-        src_dir = os.path.dirname(_testplugins[name])
-        register_plugin_dir(src_dir)
+    def tearDown(self):
+        super().tearDown()
+        if self.src_dir:
+            _plugin_dirs.remove(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 _register_plugin_dir(self, name):
+        self.src_dir = os.path.dirname(_testplugins[name])
+        register_plugin_dir(self.src_dir)
+
+    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 +270,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):