From 56d28c0b9a343cd31b5b2203a4ded9fbd29ad495 Mon Sep 17 00:00:00 2001
From: Philipp Wolfer <ph.wolfer@gmail.com>
Date: Wed, 13 Sep 2023 07:18:52 +0200
Subject: [PATCH] On plugin import error unset sys.modules again

---
 picard/pluginmanager.py |  6 +++++-
 test/test_plugins.py    | 33 +++++++++++++++++++++++----------
 2 files changed, 28 insertions(+), 11 deletions(-)

diff --git a/picard/pluginmanager.py b/picard/pluginmanager.py
index f5719023b0f..0e2a3bfae87 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/test_plugins.py b/test/test_plugins.py
index f56bfb7abab..ee187887200 100644
--- a/test/test_plugins.py
+++ b/test/test_plugins.py
@@ -229,23 +229,29 @@ 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)
 
-        src_dir = os.path.dirname(_testplugins[name])
-        register_plugin_dir(src_dir)
+    def tearDown(self):
+        super().tearDown()
+        _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 +268,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):