Repository: packit-service/packit
Pull request #663 (2020-01-13 14:57:50)

Allow list in action commands
- Allow list syntax for commands.
- Currently all these three usecases are possible:

```
        I)
        action_name: "one cmd"

        II)
        action_name:
          - "one cmd""

        III)
        action_name:
          - ["one", "cmd"]
```

packit-as-a-service[bot]: Congratulations! The build [has finished](https://copr.fedorainfracloud.org/coprs/packit/packit-service-packit-663/build/1144013/) successfully. :champagne:

You can install the built RPMs by following these steps:

* `sudo yum install -y dnf-plugins-core` on RHEL 8
* `sudo dnf install -y dnf-plugins-core` on Fedora
* `dnf copr enable packit/packit-service-packit-663`
* And now you can install the packages.

Please note that the RPMs should be used only in a testing environment.

Patch:
diff --git a/packit/base_git.py b/packit/base_git.py
index 9f4a5bbf..ff15c80e 100644
--- a/packit/base_git.py
+++ b/packit/base_git.py
@@ -31,9 +31,9 @@ from packit.actions import ActionName
 from packit.command_handler import RUN_COMMAND_HANDLER_MAPPING, CommandHandler
 from packit.config import Config, PackageConfig, RunCommandType
 from packit.exceptions import PackitException
-from packit.specfile import Specfile
 from packit.local_project import LocalProject
 from packit.security import CommitVerifier
+from packit.specfile import Specfile
 from packit.utils import cwd
 
 logger = logging.getLogger(__name__)
@@ -54,10 +54,16 @@ class PackitRepositoryBase:
         self._specfile: Optional[Specfile] = None
         self.allowed_gpg_keys: Optional[List[str]] = None
 
-        logger.debug("command handler = %s", self.config.command_handler)
-        self.handler_kls = RUN_COMMAND_HANDLER_MAPPING[self.config.command_handler]
+        self._handler_kls = None
         self._command_handler: Optional[CommandHandler] = None
 
+    @property
+    def handler_kls(self):
+        if self._handler_kls is None:
+            logger.debug("command handler = %s", self.config.command_handler)
+            self._handler_kls = RUN_COMMAND_HANDLER_MAPPING[self.config.command_handler]
+        return self._handler_kls
+
     @property
     def command_handler(self) -> CommandHandler:
         if self._command_handler is None:
@@ -210,6 +216,50 @@ class PackitRepositoryBase:
         """
         return action in self.package_config.actions
 
+    def get_commands_for_actions(self, action: ActionName) -> List[List[str]]:
+        """
+        Parse the following types of the structure and return list of commands in the form of list.
+
+        I)
+        action_name: "one cmd"
+
+        II)
+        action_name:
+          - "one cmd""
+
+        III)
+        action_name:
+          - ["one", "cmd"]
+
+
+        Returns [["one", "cmd"]] for all of them.
+
+        :param action: str or list[str] or list[list[str]]
+        :return: list[list[str]]
+        """
+        configured_action = self.package_config.actions[action]
+        if isinstance(configured_action, str):
+            configured_action = [configured_action]
+
+        if not isinstance(configured_action, list):
+            raise ValueError(
+                f"Expecting 'str' or 'list' as a command, got '{type(configured_action)}'. "
+                f"The value: {configured_action}"
+            )
+
+        parsed_commands = []
+        for cmd in configured_action:
+            if isinstance(cmd, str):
+                parsed_commands.append(shlex.split(cmd))
+            elif isinstance(cmd, list):
+                parsed_commands.append(cmd)
+            else:
+                raise ValueError(
+                    f"Expecting 'str' or 'list' as a command, got '{type(cmd)}'. "
+                    f"The value: {cmd}"
+                )
+        return parsed_commands
+
     def with_action(self, action: ActionName, env: Optional[Dict] = None) -> bool:
         """
         If the action is defined in the self.package_config.actions,
@@ -235,15 +285,10 @@ class PackitRepositoryBase:
         """
         logger.debug(f"Running {action}.")
         if action in self.package_config.actions:
-            command = self.package_config.actions[action]
-            if isinstance(command, str):
-                commands = [command]
-            else:
-                commands = command
-            logger.info(f"Using user-defined script for {action}: {commands}")
-            for cmd in commands:
-                command_l = shlex.split(cmd)
-                self.command_handler.run_command(command=command_l, env=env)
+            commands_to_run = self.get_commands_for_actions(action)
+            logger.info(f"Using user-defined script for {action}: {commands_to_run}")
+            for cmd in commands_to_run:
+                self.command_handler.run_command(command=cmd, env=env)
             return False
         logger.debug(f"Running default implementation for {action}.")
         return True
@@ -254,18 +299,16 @@ class PackitRepositoryBase:
         """
         Run self.actions[action] command(s) and return their outputs.
         """
-        commands = self.package_config.actions.get(action)
-        if not commands:
+        if action not in self.package_config.actions:
             return None
-        if isinstance(commands, str):
-            commands = [commands]
+
+        commands_to_run = self.get_commands_for_actions(action)
 
         outputs = []
-        for command in commands:
-            command_l = shlex.split(command)
-            logger.info(f"Using user-defined script for {action}: {command_l}")
+        logger.info(f"Using user-defined script for {action}: {commands_to_run}")
+        for cmd in commands_to_run:
             outputs.append(
-                self.command_handler.run_command(command_l, return_output=True, env=env)
+                self.command_handler.run_command(cmd, return_output=True, env=env)
             )
         logger.debug(f"Action command output: {outputs}")
         return outputs


Test patch:
diff --git a/tests/unit/test_upstream.py b/tests/unit/test_upstream.py
index d8921884..007bdd03 100644
--- a/tests/unit/test_upstream.py
+++ b/tests/unit/test_upstream.py
@@ -1,6 +1,7 @@
 import pytest
-from flexmock import flexmock
 
+from flexmock import flexmock
+from packit.actions import ActionName
 from packit.local_project import LocalProject
 from packit.upstream import Upstream
 from tests.spellbook import get_test_config
@@ -64,3 +65,37 @@ def test_create_pull(upstream_mock, upstream_pr_mock, fork_username):
         target_branch="test_target",
         fork_username=fork_username,
     )
+
+
+@pytest.mark.parametrize(
+    "action_config,result",
+    [
+        pytest.param("one cmd", [["one", "cmd"]], id="str_command"),
+        pytest.param(["one cmd"], [["one", "cmd"]], id="list_command"),
+        pytest.param([["one", "cmd"]], [["one", "cmd"]], id="list_in_list_command"),
+        pytest.param(
+            ["one cmd", "second cmd"],
+            [["one", "cmd"], ["second", "cmd"]],
+            id="two_str_commands_in_list",
+        ),
+        pytest.param(
+            [["one", "cmd"], ["second", "cmd"]],
+            [["one", "cmd"], ["second", "cmd"]],
+            id="two_list_commands_in_list",
+        ),
+        pytest.param(
+            [["one", "cmd"], "second cmd"],
+            [["one", "cmd"], ["second", "cmd"]],
+            id="one_str_and_one_list_command_in_list",
+        ),
+    ],
+)
+def test_get_commands_for_actions(action_config, result):
+    ups = Upstream(
+        package_config=flexmock(
+            actions={ActionName.create_archive: action_config}, synced_files=flexmock()
+        ),
+        config=flexmock(),
+        local_project=flexmock(),
+    )
+    assert ups.get_commands_for_actions(action=ActionName.create_archive) == result