diff --git a/lower-constraints.txt b/lower-constraints.txt index f1fe816efd..885dd92e0e 100644 --- a/lower-constraints.txt +++ b/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 diff --git a/nova/test.py b/nova/test.py index bf98be5778..0bca282d3b 100644 --- a/nova/test.py +++ b/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 "" + + +@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 diff --git a/nova/tests/unit/test_test.py b/nova/tests/unit/test_test.py index 2a2d75e1b4..633158b5ae 100644 --- a/nova/tests/unit/test_test.py +++ b/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')) diff --git a/nova/tests/unit/virt/libvirt/test_driver.py b/nova/tests/unit/virt/libvirt/test_driver.py index 513669588e..b43ec4221f 100644 --- a/nova/tests/unit/virt/libvirt/test_driver.py +++ b/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 test.nested( - mock.patch.object(os.path, "exists"), - mock.patch.object(libvirt_driver.LibvirtDriver, - "_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) + with mock.patch.object(libvirt_driver.LibvirtDriver, + "_get_host_sysinfo_serial_hardware") \ + as mock_uuid: + mock_uuid.return_value = self.theuuid - mock_exists.side_effect = fake_exists - - theuuid = "56b40135-a973-4eb3-87bb-a2382a3e6dbc" - mock_uuid.return_value = theuuid - - self._test_get_guest_config_sysinfo_serial(theuuid) + self._test_get_guest_config_sysinfo_serial(self.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( - 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 - + with mock.patch.object(builtins, "open") as mock_open: theuuid = "56b40135-a973-4eb3-87bb-a2382a3e6dbc" def fake_open(filename, *args, **kwargs): diff --git a/test-requirements.txt b/test-requirements.txt index 87ecc3eabd..318863b7cc 100644 --- a/test-requirements.txt +++ b/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