Merge lp:~frankban/charms/oneiric/buildbot-master/run-helper into lp:~yellow/charms/oneiric/buildbot-master/trunk

[Francesco Banconi]

Changes:
========

A new run helper accepting `verbose` and `ignore_errors` keyword arguments.
That function is compatible with the previous one, but can be used by both charms and lpsetup.

Tests:
======

$ python hooks/tests.py
........................
----------------------------------------------------------------------
Ran 24 tests in 0.088s

OK

$ python -m doctest hooks/helpers.py

[Gary Poster (gary)]

Hi Francesco. Thank you. As we discussed on the phone, let's see if we can drastically simplify this. I'll return to this if you tell me I should.

review: Needs Information

Unmerged revisions

33. By Francesco Banconi on 2012-02-22

Updated run helper.

=== modified file 'hooks/helpers.py'
--- hooks/helpers.py	2012-02-14 17:00:53 +0000
+++ hooks/helpers.py	2012-02-22 13:11:43 +0000
@@ -44,19 +44,76 @@
 Env = namedtuple('Env', 'uid gid home')
 
 
-def run(*args):
+def run(*args, **kwargs):
     """Run the command with the given arguments.
 
     The first argument is the path to the command to run, subsequent arguments
     are command-line arguments to be passed.
+
+    Usually the output is captured by a pipe and returned::
+
+        >>> run('echo', 'output')
+        'output\\n'
+
+    If the keyword argument `verbose` is True, the output and error are
+    printed to stdout and stderr, and the return code is returned::
+
+        >>> run('ls', '-l', verbose=True, stdout=subprocess.PIPE)
+        0
+
+    A `subprocess.CalledProcessError` exception is raised if the return
+    code is not zero::
+
+        >>> run('ls', '--not a valid switch') # doctest: +ELLIPSIS
+        Traceback (most recent call last):
+        CalledProcessError: Command 'ls --not a valid switch' ...
+
+    A keyword argument `exception` can be passed to raise another exception.
+    However, the new exception must accept retcode, cmd and output args::
+
+        >>> class MyException(subprocess.CalledProcessError): pass
+        >>> run('ls', '--not a valid switch',
+        ...     exception=MyException) # doctest: +ELLIPSIS
+        Traceback (most recent call last):
+        MyException: Command 'ls --not a valid switch' ...
+
+    If the keyword argument `ignore_errors` is True, then the stderr is
+    returned in case of errors::
+
+        >>> run('ls', '--not a valid switch',
+        ...     ignore_errors=True) # doctest: +ELLIPSIS
+        "ls: unrecognized option '--not a valid switch'..."
+
+    It is possible to combine `ignore_errors` and `verbose` to get the
+    error code::
+
+        >>> recode = run(
+        ...     'ls', '--not a valid switch',
+        ...     ignore_errors=True, verbose=True, stderr=subprocess.PIPE)
+        >>> recode == 2
+        True
+
+    None arguments are ignored::
+
+        >>> run(None, 'ls', None, '-l', verbose=True, stdout=subprocess.PIPE)
+        0
     """
-    process = subprocess.Popen(args, stdout=subprocess.PIPE,
-        stderr=subprocess.PIPE, close_fds=True)
+    exception = kwargs.pop('exception', subprocess.CalledProcessError)
+    ignore_errors = kwargs.pop('ignore_errors', False)
+    verbose = kwargs.pop('verbose', False)
+    default = None if verbose else subprocess.PIPE
+    args = [i for i in args if i is not None]
+    process = subprocess.Popen(
+        args, stdout=kwargs.pop('stdout', default),
+        stderr=kwargs.pop('stderr', default), close_fds=True,
+        **kwargs)
     stdout, stderr = process.communicate()
-    if process.returncode:
-        raise subprocess.CalledProcessError(
-            process.returncode, repr(args), output=stdout+stderr)
-    return stdout
+    retcode = process.poll()
+    if retcode and not ignore_errors:
+        cmd = ' '.join(i if i.strip() else "'{}'".format(i) for i in args)
+        output = None if verbose else stdout + stderr
+        raise exception(retcode, cmd, output=output)
+    return retcode if verbose else stdout + stderr
 
 
 def command(*base_args):

=== modified file 'hooks/tests.py'
--- hooks/tests.py	2012-02-10 19:43:46 +0000
+++ hooks/tests.py	2012-02-22 13:11:43 +0000
@@ -1,5 +1,6 @@
 import os
-from subprocess import CalledProcessError
+import subprocess
+import tempfile
 import unittest
 
 from helpers import (
@@ -10,10 +11,12 @@
     unit_info,
     )
 
-from subprocess import CalledProcessError
 
 class TestRun(unittest.TestCase):
 
+    invalid_command = ('/bin/ls', '--not a valid switch')
+    valid_command = ('/bin/ls', '--help')
+
     def testSimpleCommand(self):
         # Running a simple command (ls) works and running the command
         # produces a string.
@@ -22,18 +25,55 @@
     def testStdoutReturned(self):
         # Running a simple command (ls) works and running the command
         # produces a string.
-        self.assertIn('Usage:', run('/bin/ls', '--help'))
+        self.assertIn('Usage:', run(*self.valid_command))
 
-    def testSimpleCommand(self):
+    def testCommandError(self):
         # If an error occurs a CalledProcessError is raised with the return
         # code, command executed, and the output of the command.
-        with self.assertRaises(CalledProcessError) as info:
-            run('ls', '--not a valid switch')
+        with self.assertRaises(subprocess.CalledProcessError) as info:
+            run(*self.invalid_command)
         exception = info.exception
         self.assertEqual(2, exception.returncode)
-        self.assertEqual("('ls', '--not a valid switch')", exception.cmd)
+        self.assertEqual(' '.join(self.invalid_command), exception.cmd)
         self.assertIn('unrecognized option', exception.output)
 
+    def testCustomException(self):
+        # In case of errors a custom exception is raised if provided as
+        # `exception`.
+        class MyException(subprocess.CalledProcessError): pass
+        self.assertRaises(
+            MyException, run, *self.invalid_command, exception=MyException)
+
+    def testVerbose(self):
+        # If `verbose` keyword argument is given, the output is printed to
+        # stdout and the function returns command retcode.
+        with tempfile.TemporaryFile(mode='r+') as stdout:
+            retcode = run(*self.valid_command, verbose=True, stdout=stdout)
+            stdout.seek(0)
+            self.assertIn('Usage:', stdout.read())
+        self.assertEqual(0, retcode)
+
+    def testIgnoreErrors(self):
+        # If `ignore_errors` keyword argument is given, if an error occurs
+        # the function returns stdout + stderr and no exception is raised.
+        error = run(*self.invalid_command, ignore_errors=True)
+        self.assertIn('unrecognized option', error)
+
+    def testVerboseIgnoreErrors(self):
+        # If an error occurs it is possible to print it and obtain the error
+        # code combining `verbose` and `ignore_errors` keyword arguments.
+        with tempfile.TemporaryFile(mode='r+') as stderr:
+            retcode = run(
+                *self.invalid_command, verbose=True,
+                ignore_errors=True, stderr=stderr)
+            stderr.seek(0)
+            self.assertIn('unrecognized option', stderr.read())
+        self.assertEqual(2, retcode)
+
+    def testNoneArguments(self):
+        # None arguments are ignored.
+        self.assertIn('Usage:', run(None, '/bin/ls', None, '--help'))
+
 
 class TestCommand(unittest.TestCase):
 
@@ -59,7 +99,7 @@
     def testError(self):
         # If the command returns a non-zero exit code, an exception is raised.
         ls = command('/bin/ls')
-        with self.assertRaises(CalledProcessError):
+        with self.assertRaises(subprocess.CalledProcessError):
             ls('--not a valid switch')
 
     def testBakedInArguments(self):