On plugin import error unset sys.modules again

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)

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

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):