Repository: packit-service/packit
Pull request #119 (2019-03-07 10:06:43)

Add decorator for catching exception in CLI
- Add test for @packit_cover_exception decorator.
- Use @cover_packit_exception for CLI commands.
- Add decorator for CLI to catch packit exception.

TomasTomecek: [test]


Patch:
diff --git a/packit/cli/sourcegit_to_dist_git.py b/packit/cli/sourcegit_to_dist_git.py
index c6f8e5ae..e437ab2d 100644
--- a/packit/cli/sourcegit_to_dist_git.py
+++ b/packit/cli/sourcegit_to_dist_git.py
@@ -3,10 +3,10 @@ import os
 
 import click
 
+from packit.cli.utils import cover_packit_exception
 from packit.config import pass_config, get_context_settings, get_local_package_config
 from packit.local_project import LocalProject
 
-
 logger = logging.getLogger(__file__)
 
 
@@ -17,6 +17,7 @@ logger = logging.getLogger(__file__)
 @click.option("--version")
 @click.argument("repo", default=os.path.abspath(os.path.curdir))
 @pass_config
+@cover_packit_exception
 def sg2dg(config, dest_dir, no_new_sources, upstream_ref, version, repo):
     """
     Convert source-git repo to dist-git repo.
diff --git a/packit/cli/sourcegit_to_srpm.py b/packit/cli/sourcegit_to_srpm.py
index 6a1a8d27..6cafa50b 100644
--- a/packit/cli/sourcegit_to_srpm.py
+++ b/packit/cli/sourcegit_to_srpm.py
@@ -3,10 +3,10 @@ import os
 
 import click
 
+from packit.cli.utils import cover_packit_exception
 from packit.config import pass_config, get_context_settings, get_local_package_config
 from packit.local_project import LocalProject
 
-
 logger = logging.getLogger(__file__)
 
 
@@ -16,6 +16,7 @@ logger = logging.getLogger(__file__)
 @click.option("--version")
 @click.argument("repo", default=os.path.abspath(os.path.curdir))
 @pass_config
+@cover_packit_exception
 def sg2srpm(config, dest_dir, upstream_ref, version, repo):
     """
     Generate a srpm from packit.
diff --git a/packit/cli/update.py b/packit/cli/update.py
index d2a8e209..6bdd4e13 100644
--- a/packit/cli/update.py
+++ b/packit/cli/update.py
@@ -4,14 +4,14 @@ Update selected component from upstream in Fedora
 
 import logging
 import os
-import sys
 
 import click
 
 from packit.api import PackitAPI
 from packit.cli.types import LocalProjectParameter
+from packit.cli.utils import cover_packit_exception
 from packit.config import pass_config, get_context_settings, get_local_package_config
-from packit.exceptions import PackitException
+
 logger = logging.getLogger(__file__)
 
 
@@ -31,6 +31,7 @@ logger = logging.getLogger(__file__)
     default=".")
 @click.argument("repo", type=LocalProjectParameter(), default=os.path.abspath(os.path.curdir))
 @pass_config
+@cover_packit_exception
 def update(config, dist_git_path, upstream_git_path, dist_git_branch, repo):
     """
     Release current upstream release into Fedora
@@ -39,8 +40,4 @@ def update(config, dist_git_path, upstream_git_path, dist_git_branch, repo):
     package_config.downstream_project_url = dist_git_path
     package_config.upstream_project_url = upstream_git_path
     api = PackitAPI(config=config, package_config=package_config)
-    try:
-        api.sync_release(dist_git_branch)
-    except PackitException as exc:
-        logger.exception(exc)
-        sys.exit(1)
+    api.sync_release(dist_git_branch, dist_git_path=dist_git_path)
diff --git a/packit/cli/utils.py b/packit/cli/utils.py
new file mode 100644
index 00000000..4d5b87c2
--- /dev/null
+++ b/packit/cli/utils.py
@@ -0,0 +1,62 @@
+import functools
+import logging
+import sys
+
+import click
+
+from packit.exceptions import PackitException
+
+logger = logging.getLogger(__name__)
+
+
+def cover_packit_exception(_func=None, *, exit_code=None):
+    """
+    Decorator for executing the function in the try-except block.
+
+    The PackitException is caught and
+    - raised in debug
+    - sys.exit(exit_code), otherwise
+
+    On other Exceptions we print the message about creating an issue.
+
+
+    If the function receives config, it recognises the debug mode.
+    => use it after the @pass_config decorator
+    """
+
+    def decorator_cover(func):
+        @functools.wraps(func)
+        def covered_func(config=None, *args, **kwargs):
+            try:
+                if config:
+                    func(config, *args, **kwargs)
+                else:
+                    func(*args, **kwargs)
+            except KeyboardInterrupt:
+                click.echo("Quitting on user request.")
+                sys.exit(1)
+            except PackitException as exc:
+                if config and config.debug:
+                    logger.exception(exc)
+                else:
+                    logger.error(exc)
+                sys.exit(exit_code or 2)
+            except Exception as exc:
+                if config and config.debug:
+                    logger.exception(exc)
+                else:
+                    logger.error(exc)
+                    click.echo(
+                        "Unexpected exception occurred,\n"
+                        "please fill an issue here:\n"
+                        "https://github.com/packit-service/packit/issues",
+                        err=True,
+                    )
+                sys.exit(exit_code or 3)
+
+        return covered_func
+
+    if _func is None:
+        return decorator_cover
+    else:
+        return decorator_cover(_func)
diff --git a/packit/cli/watch_fedora_ci.py b/packit/cli/watch_fedora_ci.py
index 314e5c85..9ef7f64c 100644
--- a/packit/cli/watch_fedora_ci.py
+++ b/packit/cli/watch_fedora_ci.py
@@ -6,6 +6,7 @@ import logging
 import click
 
 from packit.api import PackitAPI
+from packit.cli.utils import cover_packit_exception
 from packit.config import get_context_settings, pass_config
 
 logger = logging.getLogger(__name__)
@@ -14,6 +15,7 @@ logger = logging.getLogger(__name__)
 @click.command("watch-fedora-ci", context_settings=get_context_settings())
 @click.argument("message_id", nargs=-1, required=False)
 @pass_config
+@cover_packit_exception
 def watcher(config, message_id):
     """
     watch for flags on PRs: try to process those which we know mapping for
diff --git a/packit/cli/watch_sg_pr.py b/packit/cli/watch_sg_pr.py
index e1833222..36b11c6b 100644
--- a/packit/cli/watch_sg_pr.py
+++ b/packit/cli/watch_sg_pr.py
@@ -7,6 +7,7 @@ import logging
 import click
 
 from packit.bot_api import PackitBotAPI
+from packit.cli.utils import cover_packit_exception
 from packit.config import pass_config
 
 logger = logging.getLogger(__name__)
@@ -15,6 +16,7 @@ logger = logging.getLogger(__name__)
 @click.command("watch-pr")
 @click.argument("message-id", nargs=-1)
 @pass_config
+@cover_packit_exception
 def watch_pr(config, message_id):
     """
     watch for activity on github and create/update a downstream PR
diff --git a/packit/cli/watch_upstream_release.py b/packit/cli/watch_upstream_release.py
index 25e1dbfb..ca3ad7a8 100644
--- a/packit/cli/watch_upstream_release.py
+++ b/packit/cli/watch_upstream_release.py
@@ -7,6 +7,7 @@ import logging
 import click
 
 from packit.bot_api import PackitBotAPI
+from packit.cli.utils import cover_packit_exception
 from packit.config import pass_config
 
 
@@ -18,6 +19,7 @@ logger = logging.getLogger(__name__)
     "message-id", nargs=-1,
 )
 @pass_config
+@cover_packit_exception
 def watch_releases(config, message_id):
     """
     watch for activity on github and for every new upstream release, create a


Test patch:
diff --git a/tests/unit/test_cli_utils.py b/tests/unit/test_cli_utils.py
new file mode 100644
index 00000000..4226490a
--- /dev/null
+++ b/tests/unit/test_cli_utils.py
@@ -0,0 +1,129 @@
+import sys
+
+from flexmock import flexmock
+
+from packit.cli.utils import cover_packit_exception
+from packit.exceptions import PackitException
+
+
+def test_cover_packit_exception_decorator():
+    flexmock(sys).should_receive("exit")
+
+    @cover_packit_exception
+    def covered_func():
+        raise PackitException("Test exception")
+
+    covered_func()
+
+
+def test_cover_packit_exception_decorator_attribute():
+    flexmock(sys).should_receive("exit")
+
+    @cover_packit_exception
+    def covered_func(attribute):
+        assert attribute
+        raise PackitException("Other test exception")
+
+    covered_func(attribute=flexmock())
+
+
+def test_cover_packit_exception_decorator_exit_code_default():
+    flexmock(sys).should_receive("exit").with_args(2)
+
+    @cover_packit_exception()
+    def covered_func():
+        raise PackitException("Test exception")
+
+    covered_func()
+
+
+def test_cover_packit_exception_decorator_exit_code_keyboard_interrupt():
+    flexmock(sys).should_receive("exit").with_args(1)
+
+    @cover_packit_exception()
+    def covered_func():
+        raise KeyboardInterrupt("Test keyboard interrupt")
+
+    covered_func()
+
+
+def test_cover_packit_exception_decorator_exit_code_other_exception():
+    flexmock(sys).should_receive("exit").with_args(3)
+
+    class CustomException(Exception):
+        pass
+
+    @cover_packit_exception()
+    def covered_func():
+        raise CustomException("Test keyboard interrupt")
+
+    covered_func()
+
+
+def test_cover_packit_exception_decorator_exit_code_other_exception_override():
+    flexmock(sys).should_receive("exit").with_args(5)
+
+    class CustomException(Exception):
+        pass
+
+    @cover_packit_exception(exit_code=5)
+    def covered_func():
+        raise CustomException("Test keyboard interrupt")
+
+    covered_func()
+
+
+def test_cover_packit_exception_decorator_exit_code_override():
+    flexmock(sys).should_receive("exit").with_args(5)
+
+    @cover_packit_exception(exit_code=5)
+    def covered_func():
+        raise PackitException("Test exception")
+
+    covered_func()
+
+
+def test_cover_packit_exception_decorator_config_debug_false():
+    flexmock(sys).should_receive("exit")
+
+    @cover_packit_exception
+    def covered_func(config=None):
+        raise PackitException("Test exception")
+
+    covered_func(config=flexmock(debug=False))
+
+
+def test_cover_packit_exception_decorator_config_debug_true():
+    flexmock(sys).should_receive("exit")
+
+    @cover_packit_exception
+    def covered_func(config=None):
+        raise PackitException("Test exception")
+
+    covered_func(config=flexmock(debug=True))
+
+
+def test_cover_packit_exception_decorator_other_exception():
+    flexmock(sys).should_receive("exit").with_args(3)
+
+    class CustomException(Exception):
+        pass
+
+    @cover_packit_exception
+    def covered_func():
+        raise CustomException("Other test exception")
+
+    covered_func()
+
+
+def test_cover_packit_exception_decorator_other_exception_config_debug():
+    flexmock(sys).should_receive("exit")
+
+    class CustomException(Exception):
+        pass
+
+    @cover_packit_exception
+    def covered_func(config=None):
+        raise CustomException("Other test exception")
+
+    covered_func(config=flexmock(debug=True))