Poison /sys access via various calls in test
Unit test should never depend on the file system of the machine it runs on. So this patch injects a poison to various std lib calls to catch these cases. We only selectively poison access to /sys fs as there are legitimate cases to access temp files. When I isolated the poisoned test cases I tried to add a meaningful mocking around the sysfs reads based on the meaning of the test. Change-Id: I66636ebfe627cba8c4470cd84faa78f69249aa05
This commit is contained in:
parent
a93092e0d5
commit
ebae3c2081
|
@ -285,6 +285,7 @@ class TestCase(base.BaseTestCase):
|
|||
quota.UID_QFD_POPULATED_CACHE_ALL = False
|
||||
|
||||
self.useFixture(nova_fixtures.GenericPoisonFixture())
|
||||
self.useFixture(nova_fixtures.SysFsPoisonFixture())
|
||||
|
||||
# make sure that the wsgi app is fully initialized for all testcase
|
||||
# instead of only once initialized for test worker
|
||||
|
|
|
@ -31,6 +31,7 @@ from nova.objects import fields as obj_fields
|
|||
from nova.tests.fixtures import libvirt_data as fake_libvirt_data
|
||||
from nova.virt.libvirt import config as vconfig
|
||||
from nova.virt.libvirt import driver as libvirt_driver
|
||||
from nova.virt.libvirt import host
|
||||
|
||||
|
||||
# Allow passing None to the various connect methods
|
||||
|
@ -2267,6 +2268,15 @@ class LibvirtFixture(fixtures.Fixture):
|
|||
self.mock_uname = self.useFixture(
|
||||
fixtures.MockPatch('os.uname', return_value=fake_uname)).mock
|
||||
|
||||
real_exists = os.path.exists
|
||||
|
||||
def fake_exists(path):
|
||||
if path == host.SEV_KERNEL_PARAM_FILE:
|
||||
return False
|
||||
return real_exists(path)
|
||||
|
||||
self.useFixture(fixtures.MonkeyPatch('os.path.exists', fake_exists))
|
||||
|
||||
# ...and on all machine types
|
||||
fake_loaders = [
|
||||
{
|
||||
|
|
|
@ -1739,3 +1739,62 @@ class ReaderWriterLock(lockutils.ReaderWriterLock):
|
|||
'threading.current_thread', eventlet.getcurrent)
|
||||
with mpatch if eventlet_patched else contextlib.ExitStack():
|
||||
super().__init__(*a, **kw)
|
||||
|
||||
|
||||
class SysFsPoisonFixture(fixtures.Fixture):
|
||||
|
||||
def inject_poison(self, module_name, function_name):
|
||||
import importlib
|
||||
mod = importlib.import_module(module_name)
|
||||
orig_f = getattr(mod, function_name)
|
||||
if (
|
||||
isinstance(orig_f, mock.Mock) or
|
||||
# FIXME(gibi): Is this a bug in unittest.mock? If I remove this
|
||||
# then LibvirtReportSevTraitsTests fails as builtins.open is mocked
|
||||
# there at import time via @test.patch_open. That injects a
|
||||
# MagicMock instance to builtins.open which we check here against
|
||||
# Mock (or even MagicMock) via isinstance and that check says it is
|
||||
# not a mock. More interestingly I cannot reproduce the same
|
||||
# issue with @test.patch_open and isinstance in a simple python
|
||||
# interpreter. So to make progress I'm checking the class name
|
||||
# here instead as that works.
|
||||
orig_f.__class__.__name__ == "MagicMock"
|
||||
):
|
||||
# the target is already mocked, probably via a decorator run at
|
||||
# import time, so we don't need to inject our poison
|
||||
return
|
||||
|
||||
full_name = module_name + "." + function_name
|
||||
|
||||
def toxic_wrapper(*args, **kwargs):
|
||||
path = args[0]
|
||||
if isinstance(path, bytes):
|
||||
pattern = b'/sys'
|
||||
elif isinstance(path, str):
|
||||
pattern = '/sys'
|
||||
else:
|
||||
# we ignore the rest of the potential pathlike types for now
|
||||
pattern = None
|
||||
|
||||
if pattern and path.startswith(pattern):
|
||||
raise Exception(
|
||||
'This test invokes %s on %s. It is bad, you '
|
||||
'should mock it.'
|
||||
% (full_name, path)
|
||||
)
|
||||
else:
|
||||
return orig_f(*args, **kwargs)
|
||||
|
||||
self.useFixture(fixtures.MonkeyPatch(full_name, toxic_wrapper))
|
||||
|
||||
def setUp(self):
|
||||
super().setUp()
|
||||
self.inject_poison("os.path", "isdir")
|
||||
self.inject_poison("builtins", "open")
|
||||
self.inject_poison("glob", "iglob")
|
||||
self.inject_poison("os", "listdir")
|
||||
self.inject_poison("glob", "glob")
|
||||
# TODO(gibi): Would be good to poison these too but that makes
|
||||
# a bunch of test to fail
|
||||
# self.inject_poison("os.path", "exists")
|
||||
# self.inject_poison("os", "stat")
|
||||
|
|
|
@ -190,7 +190,6 @@ class LibvirtReportNoSevTraitsTests(LibvirtReportTraitsTestBase):
|
|||
class LibvirtReportSevTraitsTests(LibvirtReportTraitsTestBase):
|
||||
STUB_INIT_HOST = False
|
||||
|
||||
@test.patch_exists(SEV_KERNEL_PARAM_FILE, True)
|
||||
@test.patch_open(SEV_KERNEL_PARAM_FILE, "1\n")
|
||||
@mock.patch.object(
|
||||
fakelibvirt.virConnect, '_domain_capability_features',
|
||||
|
@ -198,6 +197,7 @@ class LibvirtReportSevTraitsTests(LibvirtReportTraitsTestBase):
|
|||
def setUp(self):
|
||||
super(LibvirtReportSevTraitsTests, self).setUp()
|
||||
self.flags(num_memory_encrypted_guests=16, group='libvirt')
|
||||
with test.patch_exists(SEV_KERNEL_PARAM_FILE, True):
|
||||
self.start_compute()
|
||||
|
||||
def test_sev_trait_on_off(self):
|
||||
|
|
|
@ -30,7 +30,6 @@ class TestSEVInstanceReboot(base.ServersTestBase):
|
|||
"""
|
||||
microversion = 'latest'
|
||||
|
||||
@test.patch_exists(SEV_KERNEL_PARAM_FILE, True)
|
||||
@test.patch_open(SEV_KERNEL_PARAM_FILE, "1\n")
|
||||
@mock.patch.object(
|
||||
fakelibvirt.virConnect, '_domain_capability_features',
|
||||
|
@ -40,6 +39,7 @@ class TestSEVInstanceReboot(base.ServersTestBase):
|
|||
|
||||
# Configure the compute to allow SEV based instances and then start
|
||||
self.flags(num_memory_encrypted_guests=16, group='libvirt')
|
||||
with test.patch_exists(SEV_KERNEL_PARAM_FILE, True):
|
||||
self.start_compute()
|
||||
|
||||
# Create a SEV enabled image for the test
|
||||
|
|
|
@ -8075,6 +8075,10 @@ class TestAPIPortbinding(TestAPIBase):
|
|||
mock_get_instance_pci_devs.assert_called_once_with(
|
||||
instance, pci_req_id)
|
||||
|
||||
@mock.patch.object(
|
||||
pci_utils, 'is_physical_function',
|
||||
new=mock.Mock(return_value=False)
|
||||
)
|
||||
@mock.patch.object(
|
||||
pci_utils, 'get_vf_num_by_pci_address',
|
||||
new=mock.MagicMock(
|
||||
|
|
|
@ -644,6 +644,10 @@ class PciDevSpecRemoteManagedTestCase(test.NoDBTestCase):
|
|||
pci = devspec.PciDeviceSpec(pci_info)
|
||||
self.assertTrue(pci.match(self.test_dev))
|
||||
|
||||
@mock.patch(
|
||||
'nova.pci.utils.is_physical_function',
|
||||
new=mock.Mock(return_value=False)
|
||||
)
|
||||
def test_remote_managed_vf_match_by_pci_obj(self):
|
||||
pci_info = {"vendor_id": "8086", "address": "0000:0a:00.2",
|
||||
"product_id": "5057", "physical_network": "hr_net",
|
||||
|
@ -663,6 +667,10 @@ class PciDevSpecRemoteManagedTestCase(test.NoDBTestCase):
|
|||
pci_obj = objects.PciDevice.create(None, pci_dev)
|
||||
self.assertTrue(pci.match_pci_obj(pci_obj))
|
||||
|
||||
@mock.patch(
|
||||
'nova.pci.utils.is_physical_function',
|
||||
new=mock.Mock(return_value=False)
|
||||
)
|
||||
def test_remote_managed_vf_no_match_by_pci_obj(self):
|
||||
pci_info = {"vendor_id": "8086", "address": "0000:0a:00.0",
|
||||
"product_id": "5057", "physical_network": "hr_net",
|
||||
|
|
Loading…
Reference in New Issue