yellow team mailing list archive

[Francesco Banconi <frankban@xxxxxxxxx>]

Francesco Banconi has proposed merging lp:~frankban/python-shell-toolbox/apt-and-run into lp:python-shell-toolbox.

Requested reviews:
  Launchpad Yellow Squad (yellow)

For more details, see:
https://code.launchpad.net/~frankban/python-shell-toolbox/apt-and-run/+merge/98188

== 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
 
-- 
https://code.launchpad.net/~frankban/python-shell-toolbox/apt-and-run/+merge/98188
Your team Launchpad Yellow Squad is requested to review the proposed merge of lp:~frankban/python-shell-toolbox/apt-and-run into lp:python-shell-toolbox.

=== 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))