From 335dfdb7a95a92d773749e6f2b5ebf951a792608 Mon Sep 17 00:00:00 2001 From: Guido Günther Date: Thu, 26 Mar 2015 13:06:45 +0100 Subject: command_wrapper: Make error reporting more flexible We allow to substitute stderr, stdout and error_reason in run_error now. These changes the API for derived classses slightly so fix them up as well. --- gbp/command_wrappers.py | 35 ++++++++++++-------- gbp/pkg/pristinetar.py | 4 +-- tests/21_test_command_wrappers.py | 70 +++++++++++++++++++++++++++++++++++++++ 3 files changed, 93 insertions(+), 16 deletions(-) create mode 100644 tests/21_test_command_wrappers.py diff --git a/gbp/command_wrappers.py b/gbp/command_wrappers.py index 0880dc1..72dfee5 100644 --- a/gbp/command_wrappers.py +++ b/gbp/command_wrappers.py @@ -40,7 +40,7 @@ class Command(object): capture_stdout=False): self.cmd = cmd self.args = args - self.run_error = "'%s' failed" % (" ".join([self.cmd] + self.args)) + self.run_error = "'%s' failed: {err_reason}" % (" ".join([self.cmd] + self.args)) self.shell = shell self.capture_stdout = capture_stdout self.capture_stderr = capture_stderr @@ -96,11 +96,18 @@ class Command(object): return self.retcode def _log_err(self): + "Log an error message" + log.err(self._format_err()) + + def _format_err(self): + """Log an error message + + This allows to replace stdout, stderr and err_reason in + the self.run_error. """ - Log an error message allowing to use the captured stout/stderr - """ - log.err("%s: %s" % (self.run_error, - self.err_reason) + return self.run_error.format(stdout=self.stdout, + stderr=self.stderr, + err_reason=self.err_reason) def __call__(self, args=[], quiet=False): """Run the command and raise exception on errors @@ -125,7 +132,7 @@ class Command(object): >>> Command("/foo/bar")(quiet=True) Traceback (most recent call last): ... - CommandExecFailed: execution failed: [Errno 2] No such file or directory + CommandExecFailed: '/foo/bar' failed: execution failed: [Errno 2] No such file or directory """ try: ret = self.__call(args) @@ -134,7 +141,7 @@ class Command(object): if ret: if not quiet: self._log_err() - raise CommandExecFailed(self.err_reason) + raise CommandExecFailed(self._format_err()) def call(self, args, quiet=True): @@ -207,7 +214,7 @@ class UnpackTarArchive(Command): Command.__init__(self, 'tar', exclude + ['-C', dir, compression, '-xf', archive ]) - self.run_error = 'Couldn\'t unpack "%s"' % self.archive + self.run_error = 'Couldn\'t unpack "%s": {err_reason}' % self.archive class PackTarArchive(Command): @@ -222,7 +229,7 @@ class PackTarArchive(Command): Command.__init__(self, 'tar', exclude + ['-C', dir, compression, '-cf', archive, dest]) - self.run_error = 'Couldn\'t repack "%s"' % self.archive + self.run_error = 'Couldn\'t repack "%s": {err_reason}' % self.archive class CatenateTarArchive(Command): @@ -240,7 +247,7 @@ class RemoveTree(Command): def __init__(self, tree): self.tree = tree Command.__init__(self, 'rm', [ '-rf', tree ]) - self.run_error = 'Couldn\'t remove "%s"' % self.tree + self.run_error = 'Couldn\'t remove "%s": {err_reason}' % self.tree class Dch(Command): @@ -250,7 +257,7 @@ class Dch(Command): if msg: args.append(msg) Command.__init__(self, 'dch', args) - self.run_error = "Dch failed." + self.run_error = "Dch failed: {err_reason}" class DpkgSourceExtract(Command): @@ -262,7 +269,7 @@ class DpkgSourceExtract(Command): Command.__init__(self, 'dpkg-source', ['-x']) def __call__(self, dsc, output_dir): - self.run_error = 'Couldn\'t extract "%s"' % dsc + self.run_error = 'Couldn\'t extract "%s": {err_reason}' % dsc Command.__call__(self, [dsc, output_dir]) @@ -273,14 +280,14 @@ class UnpackZipArchive(Command): self.dir = dir Command.__init__(self, 'unzip', [ "-q", archive, '-d', dir ]) - self.run_error = 'Couldn\'t unpack "%s"' % self.archive + self.run_error = 'Couldn\'t unpack "%s": {err_reason}' % self.archive class GitCommand(Command): "Mother/Father of all git commands" def __init__(self, cmd, args=[], **kwargs): Command.__init__(self, 'git', [cmd] + args, **kwargs) - self.run_error = "Couldn't run git %s" % cmd + self.run_error = "Couldn't run git %s: {err_reason}" % cmd # vim:et:ts=4:sw=4:et:sts=4:ai:set list listchars=tab\:»·,trail\:·: diff --git a/gbp/pkg/pristinetar.py b/gbp/pkg/pristinetar.py index 03f043d..1c4eb5b 100644 --- a/gbp/pkg/pristinetar.py +++ b/gbp/pkg/pristinetar.py @@ -63,7 +63,7 @@ class PristineTar(Command): @param archive: the name of the orig archive @type archive: C{str} """ - self.run_error = 'Couldn\'t checkout "%s"' % os.path.basename(archive) + self.run_error = 'Couldn\'t checkout "%s": {err_reason}' % os.path.basename(archive) self.__call__(['checkout', archive]) def commit(self, archive, upstream): @@ -76,7 +76,7 @@ class PristineTar(Command): @param upstream: the upstream branch to diff against @type upstream: C{str} """ - self.run_error = ("Couldn't commit to '%s' with upstream '%s'" % + self.run_error = ("Couldn't commit to '%s' with upstream '%s': {err_reason}" % (self.branch, upstream)) self.__call__(['commit', archive, upstream]) diff --git a/tests/21_test_command_wrappers.py b/tests/21_test_command_wrappers.py new file mode 100644 index 0000000..4dfbb48 --- /dev/null +++ b/tests/21_test_command_wrappers.py @@ -0,0 +1,70 @@ +# vim: set fileencoding=utf-8 : +"""Test L{gbp.command_wrappers.Command}'s tarball unpack""" + +import unittest +import mock +import functools + +from gbp.command_wrappers import Command, CommandExecFailed +from . testutils import GbpLogTester + + +def patch_popen(stdout='', stderr='', returncode=1): + """Decorator to easily set the return value of popen.communicate()""" + def patch_popen_decorator(func): + @functools.wraps(func) + def wrap(self): + with mock.patch('subprocess.Popen') as create_mock: + popen_mock = mock.Mock(**{'returncode': returncode, + 'communicate.return_value': (stdout, stderr)}) + create_mock.return_value = popen_mock + return func(self, create_mock) + return wrap + return patch_popen_decorator + + +class TestCommandWrapperFailures(unittest.TestCase, GbpLogTester): + def setUp(self): + self.false = Command('/does/not/matter') + self.log_tester = GbpLogTester() + self.log_tester._capture_log(True) + + def tearDown(self): + self.log_tester._capture_log(False) + + @patch_popen(stdout='', stderr='', returncode=1) + def test_log_default_error_msg(self, create_mock): + with self.assertRaises(CommandExecFailed): + self.false.__call__() + self.log_tester._check_log(0, "gbp:error: '/does/not/matter' failed: it exited with 1") + self.assertEqual(self.false.retcode, 1) + self.assertEqual(self.false.stderr, '') + self.assertEqual(self.false.stdout, '') + + @patch_popen(stdout='', stderr='we have a problem', returncode=1) + def test_log_use_stderr_for_err_message(self, create_mock): + self.false.capture_stderr = True + self.false.run_error = "Erpel {stderr}" + with self.assertRaises(CommandExecFailed): + self.false.__call__() + self.log_tester._check_log(0, "gbp:error: Erpel we have a problem") + self.assertEqual(self.false.retcode, 1) + self.assertEqual(self.false.stderr, 'we have a problem') + self.assertEqual(self.false.stdout, '') + + @patch_popen(stdout='we have a problem', stderr='', returncode=1) + def test_log_use_stdout_for_err_message(self, create_mock): + self.false.capture_stdout = True + self.false.run_error = "Erpel {stdout}" + with self.assertRaises(CommandExecFailed): + self.false.__call__() + self.log_tester._check_log(0, "gbp:error: Erpel we have a problem") + self.assertEqual(self.false.retcode, 1) + self.assertEqual(self.false.stderr, '') + self.assertEqual(self.false.stdout, 'we have a problem') + + @patch_popen(returncode=0) + def test_no_log_on_success(self, create_mock): + self.false.__call__() + self.log_tester._check_log_empty() + self.assertEqual(self.false.retcode, 0) -- cgit v1.2.3