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

[Guilherme Salgado]

Use a fixture instead of a contextmanager to mock do_run in tests.

This is to make sure the mocking is always undone at the end of the test.

[James Westby (james-w)]

Hi,

I think this is a good change, thanks.

47 +class MockSomethingFixture(object):

Maybe MockAttributeFixture?

We might also want a specific subclass for do_run, as I expect that will
be used a lot?

Thanks,

James

review: Approve

[Guilherme Salgado (salgado)]

On Fri, 2010-12-03 at 18:11 +0000, James Westby wrote:
> Review: Approve
> Hi,
>
> I think this is a good change, thanks.
>
> 47 +class MockSomethingFixture(object):
>
> Maybe MockAttributeFixture?

I thought about that but then it may mislead one into thinking it can
not mock functions/objects.

>
> We might also want a specific subclass for do_run, as I expect that will
> be used a lot?

Yes, that makes sense. I've created that.

Thanks for the review!

=== modified file 'media_create/tests/fixtures.py'
--- media_create/tests/fixtures.py	2010-11-30 06:08:21 +0000
+++ media_create/tests/fixtures.py	2010-12-03 17:24:08 +0000
@@ -1,10 +1,9 @@
-#!/usr/bin/python
-
 import os
 import shutil
 import subprocess
 import tempfile
 
+
 class CreateTempDirFixture(object):
 
     def __init__(self):
@@ -16,7 +15,7 @@
     def tearDown(self):
         if os.path.exists(self.tempdir):
             shutil.rmtree(self.tempdir)
-            
+
     def get_temp_dir(self):
         return self.tempdir
 
@@ -26,16 +25,36 @@
     def __init__(self, dir):
         self.dir = dir
         self.tarball = os.path.join(self.dir + 'tarball.tar.gz')
-        
+
     def setUp(self):
         # Create gzipped tar archive.
         args = ['tar', '-czf', self.tarball, self.dir]
         proc = subprocess.Popen(args)
         proc.wait()
-        
+
     def tearDown(self):
         if os.path.exists(self.tarball):
             os.remove(self.tarball)
-            
+
     def get_tarball(self):
         return self.tarball
+
+
+class MockSomethingFixture(object):
+    """A fixture which mocks something on the given object.
+
+    Replaces attr_name on obj with the given mock, undoing that upon
+    tearDown().
+    """
+
+    def __init__(self, obj, attr_name, mock):
+        self.obj = obj
+        self.attr_name = attr_name
+        self.mock = mock
+        self.orig_attr = getattr(obj, attr_name)
+
+    def setUp(self):
+        setattr(self.obj, self.attr_name, self.mock)
+
+    def tearDown(self):
+        setattr(self.obj, self.attr_name, self.orig_attr)

=== modified file 'media_create/tests/test_media_create.py'
--- media_create/tests/test_media_create.py	2010-12-01 19:08:25 +0000
+++ media_create/tests/test_media_create.py	2010-12-03 17:24:08 +0000
@@ -17,6 +17,7 @@
 from media_create.tests.fixtures import (
     CreateTempDirFixture,
     CreateTarballFixture,
+    MockSomethingFixture,
     )
 
 
@@ -104,14 +105,6 @@
         self.assertEqual(rc, 0)
 
 
-@contextmanager
-def do_run_mocked(mock):
-    orig_func = cmd_runner.do_run
-    cmd_runner.do_run = mock
-    yield
-    cmd_runner.do_run = orig_func
-
-
 class MockDoRun(object):
     """A mock for do_run() which just stores the args given to it."""
     args = None
@@ -120,20 +113,20 @@
         return 0
 
 
-class TestCmdRunner(TestCase):
+class TestCmdRunner(TestCaseWithFixtures):
 
     def test_run(self):
-        mock = MockDoRun()
-        with do_run_mocked(mock):
-            return_code = cmd_runner.run(['foo', 'bar', 'baz'])
+        fixture = MockSomethingFixture(cmd_runner, 'do_run', MockDoRun())
+        self.useFixture(fixture)
+        return_code = cmd_runner.run(['foo', 'bar', 'baz'])
         self.assertEqual(0, return_code)
-        self.assertEqual(['foo', 'bar', 'baz'], mock.args)
+        self.assertEqual(['foo', 'bar', 'baz'], fixture.mock.args)
 
     def test_run_as_root(self):
-        mock = MockDoRun()
-        with do_run_mocked(mock):
-            cmd_runner.run(['foo', 'bar'], as_root=True)
-        self.assertEqual(['sudo', 'foo', 'bar'], mock.args)
+        fixture = MockSomethingFixture(cmd_runner, 'do_run', MockDoRun())
+        self.useFixture(fixture)
+        cmd_runner.run(['foo', 'bar'], as_root=True)
+        self.assertEqual(['sudo', 'foo', 'bar'], fixture.mock.args)
 
     def test_run_succeeds_on_zero_return_code(self):
         return_code = cmd_runner.run(['true'])