Merge lp:~frankban/python-shelltoolbox/apt-and-run into lp:python-shelltoolbox

[Francesco Banconi]

== Changes ==

The helper *apt_get_install* now accepts a *caller* keyword argument that defaults to *run*.
This is required by lpsetup, and this way we can easily test the function too.

Fixed a problem in *run* helper error handling: calling an invalid command and passing stdout (or stderr) = None resulted in a concatenation of str and None.

Bumped version up a little.

== Tests ==

$ python tests.py
...........................................................
----------------------------------------------------------------------
Ran 59 tests in 0.147s

OK

[Gary Poster (gary)]

Nice changes. Thank you.

review: Approve

[Francesco Banconi (frankban)]

Thanks Gary.

=== modified file 'setup.py'
--- setup.py	2012-03-05 15:16:17 +0000
+++ setup.py	2012-03-19 11:52:20 +0000
@@ -9,7 +9,7 @@
 ez_setup.use_setuptools()
 
 
-__version__ = '0.2.0'
+__version__ = '0.2.1'
 
 from setuptools import setup
 
 
=== modified file 'shelltoolbox/__init__.py'
--- shelltoolbox/__init__.py	2012-03-07 10:08:35 +0000
+++ shelltoolbox/__init__.py	2012-03-19 11:52:20 +0000
@@ -71,10 +71,11 @@
 
     :raises: subprocess.CalledProcessError
     """
+    caller = kwargs.pop('caller', run)
     debian_frontend = kwargs.pop('DEBIAN_FRONTEND', 'noninteractive')
     with environ(DEBIAN_FRONTEND=debian_frontend, **kwargs):
         cmd = ('apt-get', '-y', 'install') + args
-        return run(*cmd)
+        return caller(*cmd)
 
 
 def bzr_whois(user):
@@ -417,8 +418,9 @@
         close_fds=kwargs.pop('close_fds', True), **kwargs)
     stdout, stderr = process.communicate()
     if process.returncode:
+        output = ''.join(filter(None, [stdout, stderr]))
         raise subprocess.CalledProcessError(
-            process.returncode, repr(args), output=stdout+stderr)
+            process.returncode, repr(args), output=output)
     return stdout
 
 
 
=== modified file 'tests.py'
--- tests.py	2012-03-07 10:08:35 +0000
+++ tests.py	2012-03-19 11:52:20 +0000
@@ -13,6 +13,7 @@
 import unittest
 
 from shelltoolbox import (
+    apt_get_install,
     cd,
     command,
     DictDiffer,
@@ -34,6 +35,33 @@
     )
 
 
+class TestAptGetInstall(unittest.TestCase):
+
+    packages = ('package1', 'package2')
+
+    def _get_caller(self, **kwargs):
+        def caller(*args):
+            for k, v in kwargs.items():
+                self.assertEqual(v, os.getenv(k))
+        return caller
+
+    def test_caller(self):
+        # Ensure the correct command line is passed to caller.
+        cmd = apt_get_install(*self.packages, caller=lambda *args: args)
+        expected = ('apt-get', '-y', 'install') + self.packages
+        self.assertTupleEqual(expected, cmd)
+
+    def test_non_interactive_dpkg(self):
+        # Ensure dpkg is called in non interactive mode.
+        caller = self._get_caller(DEBIAN_FRONTEND='noninteractive')
+        apt_get_install(*self.packages, caller=caller)
+
+    def test_env_vars(self):
+        # Ensure apt can be run using custom environment variables.
+        caller = self._get_caller(DEBIAN_FRONTEND='noninteractive', LANG='C')
+        apt_get_install(*self.packages, caller=caller, LANG='C')
+
+
 class TestCdContextManager(unittest.TestCase):
 
     def test_cd(self):
@@ -335,6 +363,10 @@
         self.assertEqual("['ls', '--not a valid switch']", exception.cmd)
         self.assertIn('unrecognized option', exception.output)
 
+    def testErrorRaisedStdoutNotRedirected(self):
+        with self.assertRaises(CalledProcessError):
+            run('ls', '--not a valid switch', stdout=None)
+
     def testNoneArguments(self):
         # Ensure None is ignored when passed as positional argument.
         self.assertIn('Usage:', run('/bin/ls', None, '--help', None))