Merge lp:~salgado/linaro-image-tools/misc into lp:linaro-image-tools/11.11

[Guilherme Salgado]

When testing lmc in a chroot I noticed install_package_providing() would not wait for its child so we could end up running multiple 'apt-get install' in parallel, which will fail.

In the process of fixing that it occurred to me that it'd be worthwhile to assert in MockCmdRunnerPopenFixture (which is used by most of our tests that run subprocesses) that any code that invokes __call__() on a Popen object must also call wait() (either directly or via communicate()) on it. So I did that, and although there were no callsites (other than install_package_providing) that were not waiting for their child, I'm sure this new assertion will help prevent regressions.

[James Westby (james-w)]

Nice. I like the extra check. We can add a way to avoid the check if
we ever need to when we encounter it.

Thanks,

James

review: Approve

=== modified file 'README'
--- README	2010-12-10 21:39:04 +0000
+++ README	2011-01-27 18:01:50 +0000
@@ -13,6 +13,8 @@
   - dpkg-dev
   - python-parted
   - python-dbus
+  - python-apt
+  - qemu-kvm
 
 And run the following command:
 

=== modified file 'linaro_media_create/tests/fixtures.py'
--- linaro_media_create/tests/fixtures.py	2011-01-17 17:51:13 +0000
+++ linaro_media_create/tests/fixtures.py	2011-01-27 18:01:50 +0000
@@ -66,7 +66,13 @@
 class MockCmdRunnerPopen(object):
     """A mock for cmd_runner.Popen() which stores the args given to it."""
     calls = None
+    # A variable that is set to False in __call__() and only set back to True
+    # when wait() is called, to indicate that tht subprocess has finished. Is
+    # used in tests to make sure all callsites wait for their child.
+    child_finished = True
+
     def __call__(self, cmd, *args, **kwargs):
+        self.child_finished = False
         if self.calls is None:
             self.calls = []
         if isinstance(cmd, basestring):
@@ -83,6 +89,7 @@
         return '', ''
 
     def wait(self):
+        self.child_finished = True
         return self.returncode
 
     @property
@@ -96,11 +103,16 @@
     If no mock is given, a MockCmdRunnerPopen instance is used.
     """
 
-    def __init__(self, mock=None):
-        if mock is None:
-            mock = MockCmdRunnerPopen()
+    def __init__(self):
         super(MockCmdRunnerPopenFixture, self).__init__(
-            cmd_runner, 'Popen', mock)
+            cmd_runner, 'Popen', MockCmdRunnerPopen())
+
+    def tearDown(self):
+        super(MockCmdRunnerPopenFixture, self).tearDown()
+        if not self.mock.child_finished:
+            raise AssertionError(
+                "You should call wait() or communicate() to ensure "
+                "the subprocess is finished before proceeding.")
 
 
 class MockCallableWithPositionalArgs(object):

=== modified file 'linaro_media_create/tests/test_media_create.py'
--- linaro_media_create/tests/test_media_create.py	2011-01-26 18:03:50 +0000
+++ linaro_media_create/tests/test_media_create.py	2011-01-27 18:01:50 +0000
@@ -354,16 +354,17 @@
 class TestCmdRunner(TestCaseWithFixtures):
 
     def test_run(self):
-        fixture = MockCmdRunnerPopenFixture()
-        self.useFixture(fixture)
+        fixture = self.useFixture(MockCmdRunnerPopenFixture())
         proc = cmd_runner.run(['foo', 'bar', 'baz'])
+        # Call wait or else MockCmdRunnerPopenFixture() raises an
+        # AssertionError().
+        proc.wait()
         self.assertEqual(0, proc.returncode)
         self.assertEqual([['foo', 'bar', 'baz']], fixture.mock.calls)
 
     def test_run_as_root(self):
-        fixture = MockCmdRunnerPopenFixture()
-        self.useFixture(fixture)
-        cmd_runner.run(['foo', 'bar'], as_root=True)
+        fixture = self.useFixture(MockCmdRunnerPopenFixture())
+        cmd_runner.run(['foo', 'bar'], as_root=True).wait()
         self.assertEqual([['sudo', 'foo', 'bar']], fixture.mock.calls)
 
     def test_run_succeeds_on_zero_return_code(self):

=== modified file 'linaro_media_create/utils.py'
--- linaro_media_create/utils.py	2011-01-18 12:43:13 +0000
+++ linaro_media_create/utils.py	2011-01-27 18:01:50 +0000
@@ -28,7 +28,7 @@
     package, _ = packages[0]
     print ("Installing required command %s from package %s"
            % (command, package))
-    cmd_runner.run(['apt-get', 'install', package], as_root=True)
+    cmd_runner.run(['apt-get', 'install', package], as_root=True).wait()
 
 
 def ensure_command(command):