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

Proposed by Francesco Banconi on 2012-03-19
Status: Merged
Approved by: Gary Poster on 2012-03-19
Approved revision: 15
Merged at revision: 15
Proposed branch: lp:~frankban/python-shelltoolbox/apt-and-run
Merge into: lp:python-shelltoolbox
Diff against target: 97 lines (+37/-3)
3 files modified
setup.py (+1/-1)
shelltoolbox/__init__.py (+4/-2)
tests.py (+32/-0)
To merge this branch: bzr merge lp:~frankban/python-shelltoolbox/apt-and-run
Reviewer Review Type Date Requested Status
Gary Poster (community) 2012-03-19 Approve on 2012-03-19
Review via email: mp+98188@code.launchpad.net

Description of the change

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

To post a comment you must log in.
Gary Poster (gary) wrote :

Nice changes. Thank you.

review: Approve
Francesco Banconi (frankban) wrote :

Thanks Gary.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'setup.py'
2--- setup.py 2012-03-05 15:16:17 +0000
3+++ setup.py 2012-03-19 11:52:20 +0000
4@@ -9,7 +9,7 @@
5 ez_setup.use_setuptools()
6
7
8-__version__ = '0.2.0'
9+__version__ = '0.2.1'
10
11 from setuptools import setup
12
13
14=== modified file 'shelltoolbox/__init__.py'
15--- shelltoolbox/__init__.py 2012-03-07 10:08:35 +0000
16+++ shelltoolbox/__init__.py 2012-03-19 11:52:20 +0000
17@@ -71,10 +71,11 @@
18
19 :raises: subprocess.CalledProcessError
20 """
21+ caller = kwargs.pop('caller', run)
22 debian_frontend = kwargs.pop('DEBIAN_FRONTEND', 'noninteractive')
23 with environ(DEBIAN_FRONTEND=debian_frontend, **kwargs):
24 cmd = ('apt-get', '-y', 'install') + args
25- return run(*cmd)
26+ return caller(*cmd)
27
28
29 def bzr_whois(user):
30@@ -417,8 +418,9 @@
31 close_fds=kwargs.pop('close_fds', True), **kwargs)
32 stdout, stderr = process.communicate()
33 if process.returncode:
34+ output = ''.join(filter(None, [stdout, stderr]))
35 raise subprocess.CalledProcessError(
36- process.returncode, repr(args), output=stdout+stderr)
37+ process.returncode, repr(args), output=output)
38 return stdout
39
40
41
42=== modified file 'tests.py'
43--- tests.py 2012-03-07 10:08:35 +0000
44+++ tests.py 2012-03-19 11:52:20 +0000
45@@ -13,6 +13,7 @@
46 import unittest
47
48 from shelltoolbox import (
49+ apt_get_install,
50 cd,
51 command,
52 DictDiffer,
53@@ -34,6 +35,33 @@
54 )
55
56
57+class TestAptGetInstall(unittest.TestCase):
58+
59+ packages = ('package1', 'package2')
60+
61+ def _get_caller(self, **kwargs):
62+ def caller(*args):
63+ for k, v in kwargs.items():
64+ self.assertEqual(v, os.getenv(k))
65+ return caller
66+
67+ def test_caller(self):
68+ # Ensure the correct command line is passed to caller.
69+ cmd = apt_get_install(*self.packages, caller=lambda *args: args)
70+ expected = ('apt-get', '-y', 'install') + self.packages
71+ self.assertTupleEqual(expected, cmd)
72+
73+ def test_non_interactive_dpkg(self):
74+ # Ensure dpkg is called in non interactive mode.
75+ caller = self._get_caller(DEBIAN_FRONTEND='noninteractive')
76+ apt_get_install(*self.packages, caller=caller)
77+
78+ def test_env_vars(self):
79+ # Ensure apt can be run using custom environment variables.
80+ caller = self._get_caller(DEBIAN_FRONTEND='noninteractive', LANG='C')
81+ apt_get_install(*self.packages, caller=caller, LANG='C')
82+
83+
84 class TestCdContextManager(unittest.TestCase):
85
86 def test_cd(self):
87@@ -335,6 +363,10 @@
88 self.assertEqual("['ls', '--not a valid switch']", exception.cmd)
89 self.assertIn('unrecognized option', exception.output)
90
91+ def testErrorRaisedStdoutNotRedirected(self):
92+ with self.assertRaises(CalledProcessError):
93+ run('ls', '--not a valid switch', stdout=None)
94+
95 def testNoneArguments(self):
96 # Ensure None is ignored when passed as positional argument.
97 self.assertIn('Usage:', run('/bin/ls', None, '--help', None))

Subscribers

People subscribed via source and target branches