Merge "Move patch_exists() to nova.test.TestCase for reuse"

lower-constraints.txt

@@ -12,7 +12,7 @@ cffi==1.11.5
 cliff==2.11.0
 cmd2==0.8.1
 colorama==0.3.9
-contextlib2==0.5.5
+contextlib2==0.5.5;python_version=='2.7'
 coverage==4.0
 cryptography==2.1.4
 cursive==0.2.1

nova/test.py

@@ -29,10 +29,12 @@ import datetime
 import inspect
 import itertools
 import os
+import os.path
 import pprint
 import sys
 
 import fixtures
+import mock
 from oslo_cache import core as cache
 from oslo_concurrency import lockutils
 from oslo_config import cfg
@@ -64,6 +66,10 @@ from nova.tests.unit import policy_fixture
 from nova import utils
 from nova.virt import images
 
+if six.PY2:
+    import contextlib2 as contextlib
+else:
+    import contextlib
 
 CONF = cfg.CONF
 
@@ -376,6 +382,18 @@ class TestCase(testtools.TestCase):
         """
         self.useFixture(fixtures.MonkeyPatch(old, new))
 
+    @staticmethod
+    def patch_exists(patched_path, result):
+        """Provide a static method version of patch_exists(), which if you
+        haven't already imported nova.test can be slightly easier to
+        use as a context manager within a test method via:
+
+            def test_something(self):
+                with self.patch_exists(path, True):
+                    ...
+        """
+        return patch_exists(patched_path, result)
+
     def flags(self, **kw):
         """Override flag variables for a test."""
         group = kw.pop('group', None)
@@ -760,3 +778,48 @@ class ContainKeyValue(object):
     def __repr__(self):
         return "<ContainKeyValue: key " + str(self.wantkey) + \
                " and value " + str(self.wantvalue) + ">"
+
+
+@contextlib.contextmanager
+def patch_exists(patched_path, result):
+    """Selectively patch os.path.exists() so that if it's called with
+    patched_path, return result.  Calls with any other path are passed
+    through to the real os.path.exists() function.
+
+    Either import and use as a decorator / context manager, or use the
+    nova.TestCase.patch_exists() static method as a context manager.
+
+    Currently it is *not* recommended to use this if any of the
+    following apply:
+
+    - You want to patch via decorator *and* make assertions about how the
+      mock is called (since using it in the decorator form will not make
+      the mock available to your code).
+
+    - You want the result of the patched exists() call to be determined
+      programmatically (e.g. by matching substrings of patched_path).
+
+    - You expect exists() to be called multiple times on the same path
+      and return different values each time.
+
+    Additionally within unit tests which only test a very limited code
+    path, it may be possible to ensure that the code path only invokes
+    exists() once, in which case it's slightly overkill to do
+    selective patching based on the path.  In this case something like
+    like this may be more appropriate:
+
+        @mock.patch('os.path.exists', return_value=True)
+        def test_my_code(self, mock_exists):
+            ...
+            mock_exists.assert_called_once_with(path)
+    """
+    real_exists = os.path.exists
+
+    def fake_exists(path):
+        if path == patched_path:
+            return result
+        return real_exists(path)
+
+    with mock.patch.object(os.path, "exists") as mock_exists:
+        mock_exists.side_effect = fake_exists
+        yield mock_exists

nova/tests/unit/test_test.py

@@ -16,6 +16,9 @@
 
 """Tests for the testing base code."""
 
+import os.path
+
+import mock
 from oslo_log import log as logging
 import oslo_messaging as messaging
 import six
@@ -312,3 +315,62 @@ class NovaExceptionReraiseFormatErrorTestCase(test.NoDBTestCase):
         # not enough kwargs
         ex = self.assertRaises(KeyError, FakeImageException, image_id='image')
         self.assertIn('type', six.text_type(ex))
+
+
+class PatchExistsTestCase(test.NoDBTestCase):
+    def test_with_patch_exists_true(self):
+        """Test that "with patch_exists" can fake the existence of a file
+        without changing other file existence checks, and that calls can
+        be asserted on the mocked method.
+        """
+        self.assertFalse(os.path.exists('fake_file'))
+        with self.patch_exists('fake_file', True) as mock_exists:
+            self.assertTrue(os.path.exists('fake_file'))
+            self.assertTrue(os.path.exists(__file__))
+            self.assertFalse(os.path.exists('non-existent/file'))
+            self.assertIn(mock.call('fake_file'), mock_exists.mock_calls)
+
+    def test_with_patch_exists_false(self):
+        """Test that "with patch_exists" can fake the non-existence of a file
+        without changing other file existence checks, and that calls can
+        be asserted on the mocked method.
+        """
+        self.assertTrue(os.path.exists(__file__))
+        with self.patch_exists(__file__, False) as mock_exists:
+            self.assertFalse(os.path.exists(__file__))
+            self.assertTrue(os.path.exists(os.path.dirname(__file__)))
+            self.assertFalse(os.path.exists('non-existent/file'))
+            self.assertIn(mock.call(__file__), mock_exists.mock_calls)
+
+    @test.patch_exists('fake_file', True)
+    def test_patch_exists_decorator_true(self):
+        """Test that @patch_exists can fake the existence of a file
+        without changing other file existence checks.
+        """
+        self.assertTrue(os.path.exists('fake_file'))
+        self.assertTrue(os.path.exists(__file__))
+        self.assertFalse(os.path.exists('non-existent/file'))
+
+    @test.patch_exists(__file__, False)
+    def test_patch_exists_decorator_false(self):
+        """Test that @patch_exists can fake the non-existence of a file
+        without changing other file existence checks.
+        """
+        self.assertFalse(os.path.exists(__file__))
+        self.assertTrue(os.path.exists(os.path.dirname(__file__)))
+        self.assertFalse(os.path.exists('non-existent/file'))
+
+    @test.patch_exists('fake_file1', True)
+    @test.patch_exists('fake_file2', True)
+    @test.patch_exists(__file__, False)
+    def test_patch_exists_multiple_decorators(self):
+        """Test that @patch_exists can be used multiple times on the
+        same method.
+        """
+        self.assertTrue(os.path.exists('fake_file1'))
+        self.assertTrue(os.path.exists('fake_file2'))
+        self.assertFalse(os.path.exists(__file__))
+
+        # Check non-patched parameters
+        self.assertTrue(os.path.exists(os.path.dirname(__file__)))
+        self.assertFalse(os.path.exists('non-existent/file'))

nova/tests/unit/virt/libvirt/test_driver.py

@@ -18195,19 +18195,6 @@ class TestGuestConfigSysinfoSerialOS(test.NoDBTestCase):
         # Don't initialise the Host
         self.useFixture(fixtures.MockPatch('nova.virt.libvirt.driver.host'))
 
-    @contextlib.contextmanager
-    def patch_exists(self, result):
-        real_exists = os.path.exists
-
-        def fake_exists(filename):
-            if filename == "/etc/machine-id":
-                return result
-            return real_exists(filename)
-
-        with mock.patch.object(os.path, "exists") as mock_exists:
-            mock_exists.side_effect = fake_exists
-            yield mock_exists
-
     def _test_get_guest_config_sysinfo_serial(self, expected_serial):
         drvr = libvirt_driver.LibvirtDriver(fake.FakeVirtAPI(), True)
 
@@ -18252,7 +18239,7 @@ class TestGuestConfigSysinfoSerialOS(test.NoDBTestCase):
         with test.nested(
                 mock.patch.object(six.moves.builtins, "open",
                     mock.mock_open(read_data=theuuid)),
-                self.patch_exists(True)):
+                self.patch_exists("/etc/machine-id", True)):
             self._test_get_guest_config_sysinfo_serial(theuuid)
 
     def test_get_guest_config_sysinfo_serial_os_empty_machine_id(self):
@@ -18260,55 +18247,37 @@ class TestGuestConfigSysinfoSerialOS(test.NoDBTestCase):
         with test.nested(
                 mock.patch.object(six.moves.builtins, "open",
                                   mock.mock_open(read_data="")),
-                self.patch_exists(True)):
+                self.patch_exists("/etc/machine-id", True)):
             self.assertRaises(exception.NovaException,
                     self._test_get_guest_config_sysinfo_serial,
                     None)
 
     def test_get_guest_config_sysinfo_serial_os_no_machine_id_file(self):
         self.flags(sysinfo_serial="os", group="libvirt")
-        with self.patch_exists(False):
+        with self.patch_exists("/etc/machine-id", False):
             self.assertRaises(exception.NovaException,
                     self._test_get_guest_config_sysinfo_serial,
                     None)
 
+    theuuid = "56b40135-a973-4eb3-87bb-a2382a3e6dbc"
+
+    @test.patch_exists("/etc/machine-id", False)
     def test_get_guest_config_sysinfo_serial_auto_hardware(self):
         self.flags(sysinfo_serial="auto", group="libvirt")
 
-        real_exists = os.path.exists
+        with mock.patch.object(libvirt_driver.LibvirtDriver,
-        with test.nested(
+                               "_get_host_sysinfo_serial_hardware") \
-                mock.patch.object(os.path, "exists"),
+                as mock_uuid:
-                mock.patch.object(libvirt_driver.LibvirtDriver,
+            mock_uuid.return_value = self.theuuid
-                                  "_get_host_sysinfo_serial_hardware")
-        ) as (mock_exists, mock_uuid):
-            def fake_exists(filename):
-                if filename == "/etc/machine-id":
-                    return False
-                return real_exists(filename)
 
-            mock_exists.side_effect = fake_exists
+            self._test_get_guest_config_sysinfo_serial(self.theuuid)
-
-            theuuid = "56b40135-a973-4eb3-87bb-a2382a3e6dbc"
-            mock_uuid.return_value = theuuid
-
-            self._test_get_guest_config_sysinfo_serial(theuuid)
 
+    @test.patch_exists("/etc/machine-id", True)
     def test_get_guest_config_sysinfo_serial_auto_os(self):
         self.flags(sysinfo_serial="auto", group="libvirt")
 
-        real_exists = os.path.exists
         real_open = builtins.open
-        with test.nested(
+        with mock.patch.object(builtins, "open") as mock_open:
-                mock.patch.object(os.path, "exists"),
-                mock.patch.object(builtins, "open"),
-        ) as (mock_exists, mock_open):
-            def fake_exists(filename):
-                if filename == "/etc/machine-id":
-                    return True
-                return real_exists(filename)
-
-            mock_exists.side_effect = fake_exists
-
             theuuid = "56b40135-a973-4eb3-87bb-a2382a3e6dbc"
 
             def fake_open(filename, *args, **kwargs):

test-requirements.txt

@@ -3,6 +3,7 @@
 # process, which may cause wedges in the gate later.
 
 hacking>=1.1.0,<1.2.0 # Apache-2.0
+contextlib2>=0.5.5;python_version<'3.0'  # PSF License
 coverage!=4.4,>=4.0 # Apache-2.0
 ddt>=1.0.1 # MIT
 fixtures>=3.0.0 # Apache-2.0/BSD