Make Plugin Importer more flexible regarding actions

Don't make assumptions about the contents of action files — just import.

BUG:429262

dev-tools/python/dev-requirements.txt

-flake8==3.7.4
-pytest==4.0.2
+flake8==3.8.4
+pytest==6.1.2
 PyQt5==5.11.3
 PyQt5-sip==4.19.13
 

plugins/python/plugin_importer/plugin_importer.py

@@ -28,7 +28,6 @@ import shutil
 import sys
 from tempfile import TemporaryDirectory
 import zipfile
-from xml.etree import ElementTree
 
 
 class PluginImportError(Exception):
@@ -140,16 +139,9 @@ class PluginImporter:
 
     def get_source_actionfile(self, name):
         for filename in self.action_filenames:
-            try:
-                root = ElementTree.fromstring(
-                    self.archive.read(filename).decode('utf-8'))
-            except ElementTree.ParseError as e:
-                raise PluginReadError(
-                    '%s: %s' % (i18n('Action file'), str(e)))
-
-            for action in root.findall('./Actions/Action'):
-                if action.get('name') == name:
-                    return filename
+            _, actionfilename = os.path.split(filename)
+            if actionfilename == '%s.action' % name:
+                return filename
 
     def read_desktop_config(self, desktop_filename):
         config = ConfigParser()

plugins/python/plugin_importer/tests/fixtures/fail_unparsable_action_file/plugin/foo.action

-this is wrong

plugins/python/plugin_importer/tests/fixtures/success_different_action_name/plugin/foo.action

+<?xml version="1.0" encoding="UTF-8"?>
+<ActionCollection version="2" name="Scripts">
+  <Actions category="Scripts">
+    <text>Bar</text>
+
+    <Action name="bar">
+      <icon></icon>
+      <text>Bar...</text>
+      <whatsThis></whatsThis>
+      <toolTip></toolTip>
+      <iconText></iconText>
+      <activationFlags>0</activationFlags>
+      <activationConditions>0</activationConditions>
+      <shortcut></shortcut>
+      <isCheckable>false</isCheckable>
+      <statusTip></statusTip>
+    </Action>
+  </Actions>
+</ActionCollection>

plugins/python/plugin_importer/tests/test_plugin_importer.py

@@ -83,14 +83,16 @@ class PluginImporterTestCase(TestCase):
         `self.resources_dir`.
         """
 
-        assert os.path.exists(os.path.join(self.resources_dir.name, *path))
+        exists = os.path.exists(os.path.join(self.resources_dir.name, *path))
+        assert exists, 'File missing: %s' % os.path.join(*path)
 
     def assert_not_in_resources_dir(self, *path):
         """Helper method to check whether a directory or file doesn't exist
         inside `self.resources_dir`.
         """
 
-        assert not os.path.exists(os.path.join(self.resources_dir.name, *path))
+        exists = os.path.exists(os.path.join(self.resources_dir.name, *path))
+        assert not exists, 'Unexpected file: %s' % os.path.join(*path)
 
     ############################################################
     # The actual tests start below
@@ -134,6 +136,21 @@ class PluginImporterTestCase(TestCase):
         self.assert_in_resources_dir('pykrita', 'foo.desktop')
         self.assert_in_resources_dir('actions', 'foo.action')
 
+    def test_plugin_different_action_name_success(self):
+        """Test: Import a basic plugin."""
+
+        self.zip_plugin('success_different_action_name')
+        importer = PluginImporter(self.zip_filename,
+                                  self.resources_dir.name,
+                                  lambda x: True)
+        imported = importer.import_all()
+        assert len(imported) == 1
+        self.assert_in_resources_dir('pykrita', 'foo')
+        self.assert_in_resources_dir('pykrita', 'foo', '__init__.py')
+        self.assert_in_resources_dir('pykrita', 'foo', 'foo.py')
+        self.assert_in_resources_dir('pykrita', 'foo.desktop')
+        self.assert_in_resources_dir('actions', 'foo.action')
+
     def test_toplevel_plugin_success(self):
         """Test: Import a plugin with everything at toplevel."""
 
@@ -287,14 +304,3 @@ class PluginImporterTestCase(TestCase):
         with pytest.raises(NoPluginsFoundException):
             # We expect an exception
             importer.import_all()
-
-    def test_unparsable_action_file(self):
-        """Test: Import plugin whose action file isn't parsable."""
-
-        self.zip_plugin('fail_unparsable_action_file')
-        importer = PluginImporter(self.zip_filename,
-                                  self.resources_dir.name,
-                                  lambda x: True)
-        with pytest.raises(PluginReadError):
-            # We expect an exception
-            importer.import_all()