[Branch,~linaro-maintainers/linaro-image-tools/trunk] Rev 265: Fix install_package_providing() to wait() for its child. Also change MockCmdRunnerPopenFixture to...

Message ID 20110127184728.28712.8767.launchpad@loganberry.canonical.com
State Accepted
Headers show

Commit Message

Guilherme Salgado Jan. 27, 2011, 6:47 p.m.
Merge authors:
  Guilherme Salgado (salgado)
Related merge proposals:
  https://code.launchpad.net/~salgado/linaro-image-tools/misc/+merge/47700
  proposed by: Guilherme Salgado (salgado)
  review: Approve - James Westby (james-w)
------------------------------------------------------------
revno: 265 [merge]
committer: Guilherme Salgado <salgado@canonical.com>
branch nick: trunk
timestamp: Thu 2011-01-27 16:43:55 -0200
message:
  Fix install_package_providing() to wait() for its child. Also change MockCmdRunnerPopenFixture to assert that any callsite that invokes Popen.__call__() also calls .wait()
modified:
  README
  linaro_media_create/tests/fixtures.py
  linaro_media_create/tests/test_media_create.py
  linaro_media_create/utils.py


--
lp:linaro-image-tools
https://code.launchpad.net/~linaro-maintainers/linaro-image-tools/trunk

You are subscribed to branch lp:linaro-image-tools.
To unsubscribe from this branch go to https://code.launchpad.net/~linaro-maintainers/linaro-image-tools/trunk/+edit-subscription

Patch

=== modified file 'README'
--- README	2010-12-10 21:39:04 +0000
+++ README	2011-01-27 17:54:57 +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 17:54:57 +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-27 17:19:03 +0000
+++ linaro_media_create/tests/test_media_create.py	2011-01-27 18:43:55 +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 17:54:57 +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):