diff --git a/lower-constraints.txt b/lower-constraints.txt index 885dd92e0e75..aa1d8ac52dab 100644 --- a/lower-constraints.txt +++ b/lower-constraints.txt @@ -53,7 +53,7 @@ Mako==1.0.7 MarkupSafe==1.0 mccabe==0.2.1 microversion-parse==0.2.1 -mock==2.0.0 +mock==3.0.0 monotonic==1.4 mox3==0.20.0 msgpack==0.5.6 diff --git a/nova/test.py b/nova/test.py index 0bca282d3b79..048d0a2e0c27 100644 --- a/nova/test.py +++ b/nova/test.py @@ -48,6 +48,7 @@ from oslo_versionedobjects import fixture as ovo_fixture from oslotest import mock_fixture from oslotest import moxstubout import six +from six.moves import builtins import testtools from nova.compute import resource_tracker @@ -394,6 +395,17 @@ class TestCase(testtools.TestCase): """ return patch_exists(patched_path, result) + @staticmethod + def patch_open(patched_path, read_data): + """Provide a static method version of patch_open() which is easier to + use as a context manager within a test method via: + + def test_something(self): + with self.patch_open(path, "fake contents of file"): + ... + """ + return patch_open(patched_path, read_data) + def flags(self, **kw): """Override flag variables for a test.""" group = kw.pop('group', None) @@ -823,3 +835,52 @@ def patch_exists(patched_path, result): with mock.patch.object(os.path, "exists") as mock_exists: mock_exists.side_effect = fake_exists yield mock_exists + + +@contextlib.contextmanager +def patch_open(patched_path, read_data): + """Selectively patch open() so that if it's called with patched_path, + return a mock which makes it look like the file contains + read_data. Calls with any other path are passed through to the + real open() function. + + Either import and use as a decorator, or use the + nova.TestCase.patch_open() static method as a context manager. + + Currently it is *not* recommended to use this if any of the + following apply: + + - The code under test will attempt to write to patched_path. + + - 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 faked file contents to be determined + programmatically (e.g. by matching substrings of patched_path). + + - You expect open() to be called multiple times on the same path + and return different file contents 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 + open() 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(six.moves.builtins, 'open') + def test_my_code(self, mock_open): + ... + mock_open.assert_called_once_with(path) + """ + real_open = builtins.open + m = mock.mock_open(read_data=read_data) + + def selective_fake_open(path, *args, **kwargs): + if path == patched_path: + return m(patched_path) + return real_open(path, *args, **kwargs) + + with mock.patch.object(builtins, 'open') as mock_open: + mock_open.side_effect = selective_fake_open + yield m diff --git a/nova/tests/unit/pci/test_utils.py b/nova/tests/unit/pci/test_utils.py index 6675be5c2471..c1105bd226e8 100644 --- a/nova/tests/unit/pci/test_utils.py +++ b/nova/tests/unit/pci/test_utils.py @@ -108,8 +108,7 @@ class GetFunctionByIfnameTestCase(test.NoDBTestCase): totalvf_path = "/sys/class/net/%s/device/%s" % (ifname, utils._SRIOV_TOTALVFS) mock_readlink.return_value = '../../../0000:00:00.1' - with mock.patch.object( - builtins, 'open', mock.mock_open(read_data='4')) as mock_open: + with self.patch_open(totalvf_path, '4') as mock_open: address, physical_function = utils.get_function_by_ifname('eth0') self.assertEqual(address, '0000:00:00.1') self.assertTrue(physical_function) diff --git a/nova/tests/unit/test_test.py b/nova/tests/unit/test_test.py index 633158b5ae61..1ef3b311e905 100644 --- a/nova/tests/unit/test_test.py +++ b/nova/tests/unit/test_test.py @@ -17,6 +17,8 @@ """Tests for the testing base code.""" import os.path +import tempfile +import uuid import mock from oslo_log import log as logging @@ -374,3 +376,58 @@ class PatchExistsTestCase(test.NoDBTestCase): # Check non-patched parameters self.assertTrue(os.path.exists(os.path.dirname(__file__))) self.assertFalse(os.path.exists('non-existent/file')) + + +class PatchOpenTestCase(test.NoDBTestCase): + fake_contents = "These file contents don't really exist" + + def _test_patched_open(self): + """Test that a selectively patched open can fake the contents of a + file while still allowing normal, real file operations. + """ + self.assertFalse(os.path.exists('fake_file')) + + with open('fake_file') as f: + self.assertEqual(self.fake_contents, f.read()) + + # Test we can still open and read this file from within the + # same context. NOTE: We have to make sure we open the .py + # file not the corresponding .pyc file. + with open(__file__.rstrip('c')) as f: + this_file_contents = f.read() + self.assertIn("class %s(" % self.__class__.__name__, + this_file_contents) + self.assertNotIn("magic concatenated" "string", + this_file_contents) + + # Test we can still create, write to, and then read from a + # temporary file, from within the same context. + tmp = tempfile.NamedTemporaryFile() + tmp_contents = str(uuid.uuid1()) + with open(tmp.name, 'w') as f: + f.write(tmp_contents) + with open(tmp.name) as f: + self.assertEqual(tmp_contents, f.read()) + + return tmp.name + + def test_with_patch_open(self): + """Test that "with patch_open" can fake the contents of a file + without changing other file operations, and that calls can + be asserted on the mocked method. + """ + with self.patch_open('fake_file', self.fake_contents) as mock_open: + tmp_name = self._test_patched_open() + + # Test we can make assertions about how the mock_open was called. + self.assertIn(mock.call('fake_file'), mock_open.mock_calls) + # The mock_open should get bypassed for non-patched path values: + self.assertNotIn(mock.call(__file__), mock_open.mock_calls) + self.assertNotIn(mock.call(tmp_name), mock_open.mock_calls) + + @test.patch_open('fake_file', fake_contents) + def test_patch_open_decorator(self): + """Test that @patch_open can fake the contents of a file + without changing other file operations. + """ + self._test_patched_open() diff --git a/nova/tests/unit/test_versions.py b/nova/tests/unit/test_versions.py index 9a2ecfda0b54..86522de2566f 100644 --- a/nova/tests/unit/test_versions.py +++ b/nova/tests/unit/test_versions.py @@ -13,8 +13,6 @@ # under the License. from oslo_config import cfg -import six -from six.moves import builtins from nova import test from nova import version @@ -32,9 +30,13 @@ class VersionTestCase(test.NoDBTestCase): self.assertEqual("5.5.5.5-g9ec3421", version.version_string_with_package()) + @test.patch_open("/etc/nova/release", read_data= + "[Nova]\n" + "vendor = ACME Corporation\n" + "product = ACME Nova\n" + "package = 1337\n") def test_release_file(self): version.loaded = False - real_open = builtins.open real_find_file = cfg.CONF.find_file def fake_find_file(self, name): @@ -42,17 +44,6 @@ class VersionTestCase(test.NoDBTestCase): return "/etc/nova/release" return real_find_file(self, name) - def fake_open(path, *args, **kwargs): - if path == "/etc/nova/release": - data = """[Nova] -vendor = ACME Corporation -product = ACME Nova -package = 1337""" - return six.StringIO(data) - - return real_open(path, *args, **kwargs) - - self.stub_out('six.moves.builtins.open', fake_open) self.stub_out('oslo_config.cfg.ConfigOpts.find_file', fake_find_file) self.assertEqual(version.vendor_string(), "ACME Corporation") diff --git a/nova/tests/unit/virt/libvirt/test_driver.py b/nova/tests/unit/virt/libvirt/test_driver.py index c014dac36a72..adbff9c1ee82 100644 --- a/nova/tests/unit/virt/libvirt/test_driver.py +++ b/nova/tests/unit/virt/libvirt/test_driver.py @@ -55,7 +55,6 @@ from oslo_utils import units from oslo_utils import uuidutils from oslo_utils import versionutils import six -from six.moves import builtins from six.moves import range from nova.api.metadata import base as instance_metadata @@ -18289,24 +18288,10 @@ class TestGuestConfigSysinfoSerialOS(test.NoDBTestCase): self._test_get_guest_config_sysinfo_serial(self.theuuid) @test.patch_exists("/etc/machine-id", True) + @test.patch_open("/etc/machine-id", theuuid) def test_get_guest_config_sysinfo_serial_auto_os(self): self.flags(sysinfo_serial="auto", group="libvirt") - - real_open = builtins.open - with mock.patch.object(builtins, "open") as mock_open: - theuuid = "56b40135-a973-4eb3-87bb-a2382a3e6dbc" - - def fake_open(filename, *args, **kwargs): - if filename == "/etc/machine-id": - h = mock.MagicMock() - h.read.return_value = theuuid - h.__enter__.return_value = h - return h - return real_open(filename, *args, **kwargs) - - mock_open.side_effect = fake_open - - self._test_get_guest_config_sysinfo_serial(theuuid) + self._test_get_guest_config_sysinfo_serial(self.theuuid) def test_get_guest_config_sysinfo_serial_unique(self): self.flags(sysinfo_serial="unique", group="libvirt") diff --git a/test-requirements.txt b/test-requirements.txt index 318863b7ccff..60f8fb3acd5e 100644 --- a/test-requirements.txt +++ b/test-requirements.txt @@ -7,7 +7,7 @@ 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 -mock>=2.0.0 # BSD +mock>=3.0.0 # BSD mox3>=0.20.0 # Apache-2.0 psycopg2>=2.7 # LGPL/ZPL PyMySQL>=0.7.6 # MIT License