Repository: qiskit-community/ecosystem
Pull request #32 (2021-09-23 15:31:40)

Commands: refactor commands in classes
<!--
⚠️ If you do not respect this template, your pull request will be closed.
⚠️ Your pull request title should be short detailed and understandable for all.
⚠️ Also, please add a release note file using reno if the change needs to be
  documented in the release notes.
⚠️ If your pull request fixes an open issue, please link to the issue.

- [ ] I have added the tests to cover my changes.
- [ ] I have updated the documentation accordingly.
- [ ] I have read the CONTRIBUTING document.
-->

### Summary
Refactoring of commands into appropriate classes.


IceKhan13: @mickahell @bala-ceg for your visibility 


Patch:
diff --git a/ecosystem/commands/__init__.py b/ecosystem/commands/__init__.py
index 78e1ff2..b47596e 100644
--- a/ecosystem/commands/__init__.py
+++ b/ecosystem/commands/__init__.py
@@ -1,5 +1,4 @@
 """Commands."""
-
-from .command import (_execute_command, _clone_repo,
-                      _cleanup, _run_tox,
-                      _run_lint, _check_tox_ini)
+from .command import (Command, CloneRepoCommand, ShellCommand,
+                      CommandExecutionSummary, FileExistenceCheckCommand,
+                      RunToxCommand)
diff --git a/ecosystem/commands/command.py b/ecosystem/commands/command.py
index 3ef40c1..176f52b 100644
--- a/ecosystem/commands/command.py
+++ b/ecosystem/commands/command.py
@@ -1,69 +1,97 @@
 """Command execution."""
 import os
-import shutil
 import subprocess
+from abc import abstractmethod
 from typing import Optional, List
 
 from ecosystem.models import CommandExecutionSummary
 from ecosystem.utils import logger
 
 
-def _execute_command(command: List[str],
-                     cwd: Optional[str] = None,
-                     name: Optional[str] = None) -> CommandExecutionSummary:
-    """Executes specified command as subprocess in a directory."""
-    with subprocess.Popen(command,
-                          stdout=subprocess.PIPE,
-                          stderr=subprocess.PIPE,
-                          cwd=cwd) as process:
-        logs = []
-        while True:
-            output = str(process.stdout.readline())
-            logger.info(output.strip())
-            logs.append(output.strip())
-            return_code = process.poll()
-            if return_code is not None:
-                logger.info('RETURN CODE: %s}', return_code)
-                for output in process.stdout.readlines():
-                    logger.info(str(output).strip())
-                    logs.append(str(output).strip())
-
-                return CommandExecutionSummary(code=return_code,
-                                               logs=logs,
-                                               name=name)
-
-
-def _clone_repo(repo: str, directory: str) -> CommandExecutionSummary:
-    """Clones repo in directory."""
-    return _execute_command(["git", "-C", directory, "clone", repo])
-
-
-def _check_tox_ini(directory: str) -> CommandExecutionSummary:
-    """Checks for tox file existance."""
-    if not os.path.isfile("{}/tox.ini".format(directory)):
-        return CommandExecutionSummary(code=1,
-                                       logs=[],
-                                       summary="No tox.ini file in project.")
-    return CommandExecutionSummary.empty()
-
-
-def _run_tox(directory: str, env: str) -> CommandExecutionSummary:
-    """Run tox test."""
-    return _execute_command(["tox", "-e{}".format(env),
-                             "--workdir", directory],
-                            cwd=directory,
-                            name="tests")
-
-
-def _run_lint(directory: str) -> CommandExecutionSummary:
-    """Run lint test."""
-    return _execute_command(["tox", "-elint",
-                             "--workdir", directory],
-                            cwd=directory,
-                            name="styles_check")
-
-
-def _cleanup(directory: Optional[str] = None):
-    """Removes temp directory."""
-    if directory and os.path.exists(directory):
-        shutil.rmtree(directory)
+# pylint: disable=arguments-differ
+class Command:
+    """General for executable shell commands."""
+
+    def __init__(self,
+                 name: Optional[str] = None,
+                 cwd: Optional[str] = None):
+        """General class for executable shell commands.
+
+        Args:
+            name: name for command
+            cwd: directory where to execute command
+        """
+        self.name = name or "unnamed_command"
+        self.cwd = cwd
+
+    @classmethod
+    def subprocess_execute(cls,
+                           command: List[str],
+                           name: Optional[str] = None,
+                           cwd: Optional[str] = None):
+        """Executes specified command as subprocess in a directory."""
+        with subprocess.Popen(command,
+                              stdout=subprocess.PIPE,
+                              stderr=subprocess.PIPE,
+                              cwd=cwd) as process:
+            logs = []
+            while True:
+                output = str(process.stdout.readline())
+                logger.info(output.strip())
+                logs.append(output.strip())
+                return_code = process.poll()
+                if return_code is not None:
+                    logger.info('RETURN CODE: %s}', return_code)
+                    for output in process.stdout.readlines():
+                        logger.info(str(output).strip())
+                        logs.append(str(output).strip())
+
+                    return CommandExecutionSummary(code=return_code,
+                                                   logs=logs,
+                                                   name=name)
+
+    @classmethod
+    @abstractmethod
+    def execute(cls, **kwargs) -> CommandExecutionSummary:
+        """Executes command."""
+
+
+class ShellCommand(Command):
+    """Arbitrary shell command."""
+
+    @classmethod
+    def execute(cls, command: List[str], directory: str, **kwargs) -> CommandExecutionSummary:
+        return cls.subprocess_execute(command=command,
+                                      cwd=directory,
+                                      name=" ".join(command))
+
+
+class CloneRepoCommand(Command):
+    """Clone repo command."""
+
+    @classmethod
+    def execute(cls, repo: str, directory: str, **kwargs) -> CommandExecutionSummary:
+        return cls.subprocess_execute(["git", "-C", directory, "clone", repo],
+                                      name="Clone repo")
+
+
+class RunToxCommand(Command):
+    """Running tox."""
+
+    @classmethod
+    def execute(cls, directory: str, env: str, **kwargs) -> CommandExecutionSummary:
+        return cls.subprocess_execute(["tox", "-e{}".format(env), "--workdir", directory],
+                                      cwd=directory,
+                                      name="Tests")
+
+
+class FileExistenceCheckCommand(Command):
+    """Check for file existence."""
+
+    @classmethod
+    def execute(cls, file: str, directory: str, **kwargs) -> CommandExecutionSummary:
+        if not os.path.isfile("{}/{}".format(directory, file)):
+            return CommandExecutionSummary(code=1,
+                                           logs=[],
+                                           summary="No {} file in project.".format(file))
+        return CommandExecutionSummary.empty()
diff --git a/ecosystem/runners/python_styles_runner.py b/ecosystem/runners/python_styles_runner.py
index 9d21a87..53b0762 100644
--- a/ecosystem/runners/python_styles_runner.py
+++ b/ecosystem/runners/python_styles_runner.py
@@ -2,7 +2,7 @@
 import os
 from typing import Optional, Union, cast, List, Tuple
 
-from ecosystem.commands import _run_lint
+from ecosystem.commands import RunToxCommand
 from ecosystem.models import (RepositoryConfiguration, PythonRepositoryConfiguration,
                               CommandExecutionSummary)
 from ecosystem.models.repository import Repository
@@ -64,6 +64,7 @@ class PythonStyleRunner(Runner):
             tox_file.write(repo_config.render_tox_file(ecosystem_deps=[]))
 
         # run lint
-        tox_lint_res = _run_lint(directory=self.cloned_repo_directory)
+        tox_lint_res = RunToxCommand.execute(directory=self.cloned_repo_directory,
+                                             env="lint")
 
         return "-", [tox_lint_res]
diff --git a/ecosystem/runners/runner.py b/ecosystem/runners/runner.py
index 63c465f..8050860 100644
--- a/ecosystem/runners/runner.py
+++ b/ecosystem/runners/runner.py
@@ -5,7 +5,7 @@ from abc import abstractmethod
 from logging import Logger
 from typing import Optional, Union, List, Tuple
 
-from ecosystem.commands import _clone_repo
+from ecosystem.commands import CloneRepoCommand
 from ecosystem.models import CommandExecutionSummary, RepositoryConfiguration
 from ecosystem.models.repository import Repository
 from ecosystem.utils import QiskitEcosystemException
@@ -55,7 +55,8 @@ class Runner:
         self.set_up()
         # clone repository
         self.logger.info("Cloning repository: %s", self.repo)
-        clone_res = _clone_repo(self.repo, directory=self.working_directory)
+        clone_res = CloneRepoCommand.execute(repo=self.repo,
+                                             directory=self.working_directory)
 
         if not clone_res.ok:
             raise QiskitEcosystemException(


Test patch:
diff --git a/ecosystem/runners/python_tests_runner.py b/ecosystem/runners/python_tests_runner.py
index 87686a6..8b18f41 100644
--- a/ecosystem/runners/python_tests_runner.py
+++ b/ecosystem/runners/python_tests_runner.py
@@ -2,7 +2,7 @@
 import os
 from typing import Optional, Union, cast, List, Tuple
 
-from ecosystem.commands import _run_tox
+from ecosystem.commands import RunToxCommand
 from ecosystem.models import (RepositoryConfiguration, PythonRepositoryConfiguration,
                               CommandExecutionSummary)
 from ecosystem.models.repository import Repository
@@ -64,8 +64,8 @@ class PythonTestsRunner(Runner):
             return terra_version, []
 
         # run tox
-        tox_tests_res = _run_tox(directory=self.cloned_repo_directory,
-                                 env=self.python_version)
+        tox_tests_res = RunToxCommand.execute(directory=self.cloned_repo_directory,
+                                              env=self.python_version)
 
         # get terra version from file
         if os.path.exists(f"{self.cloned_repo_directory}/terra_version.txt"):
diff --git a/tests/commands/test_commands.py b/tests/commands/test_commands.py
index ccf3310..59edf05 100644
--- a/tests/commands/test_commands.py
+++ b/tests/commands/test_commands.py
@@ -1,7 +1,7 @@
 """Tests for shell commands."""
 import os
 
-from ecosystem.commands import _execute_command, _clone_repo
+from ecosystem.commands import CloneRepoCommand, ShellCommand
 from ecosystem.models import CommandExecutionSummary
 from tests.common import TestCaseWithResources
 
@@ -11,14 +11,15 @@ class TestCommands(TestCaseWithResources):
 
     def test_execute_command(self):
         """Tests command execution."""
-        execution_result = _execute_command(["ls", "-al"],
-                                            cwd=os.path.dirname(os.path.abspath(__file__)))
+        execution_result = ShellCommand.execute(
+            ["ls", "-al"],
+            directory=os.path.dirname(os.path.abspath(__file__)))
         self.assertTrue(isinstance(execution_result, CommandExecutionSummary))
         self.assertTrue(len(execution_result.logs) > 0)
 
     def test_clone_repo(self):
         """Tests repo cloning."""
-        clone_res = _clone_repo("https://github.com/octocat/Hello-World",
-                                directory=self.path)
+        clone_res = CloneRepoCommand.execute(repo="https://github.com/octocat/Hello-World",
+                                             directory=self.path)
         self.assertTrue(os.path.exists(f"{self.path}/Hello-World"))
         self.assertEqual(clone_res.code, 0)