Add mock to avoid loading guestfs in unit test

We recently discovered that when the perfect conditions are present
where:

  * libguestfs-dev/el and guestfs python bindings are installed

and

  * unit tests are not being run in a venv or guestfs python bindings
    are installed in the tox venv

the test will end up loading the guestfs module and try to call the
real guestfs and possibly libvirt and fail because of it.

Our unit tests shouldn't be loading modules like guestfs, so this adds
proper mocking to the test along with a poison fixture that will
prevent future accidental imports of such modules.

Closes-Bug: #1994913

Change-Id: I676ee1fd33cf053681a07448759c28f0f2ad79d1
This commit is contained in:
melanie witt
2022-10-27 01:43:55 +00:00
parent b1958b7cfa
commit ecb11043e9
3 changed files with 55 additions and 0 deletions

View File

@@ -288,6 +288,10 @@ class TestCase(base.BaseTestCase):
self.useFixture(nova_fixtures.GenericPoisonFixture())
self.useFixture(nova_fixtures.SysFsPoisonFixture())
# Additional module names can be added to this set if needed
self.useFixture(nova_fixtures.ImportModulePoisonFixture(
set(['guestfs', 'libvirt'])))
# make sure that the wsgi app is fully initialized for all testcase
# instead of only once initialized for test worker
wsgi_app.init_global_data.reset()

View File

@@ -20,8 +20,10 @@ import collections
import contextlib
from contextlib import contextmanager
import functools
from importlib.abc import MetaPathFinder
import logging as std_logging
import os
import sys
import time
from unittest import mock
import warnings
@@ -1798,3 +1800,51 @@ class SysFsPoisonFixture(fixtures.Fixture):
# a bunch of test to fail
# self.inject_poison("os.path", "exists")
# self.inject_poison("os", "stat")
class ImportModulePoisonFixture(fixtures.Fixture):
"""Poison imports of modules unsuitable for the test environment.
Examples are guestfs and libvirt. Ordinarily, these would not be installed
in the test environment but if they _are_ present, it can result in
actual calls to libvirt, for example, which could cause tests to fail.
This fixture will inspect module imports and if they are in the disallowed
list, it will fail the test with a helpful message about mocking needed in
the test.
"""
class ForbiddenModules(MetaPathFinder):
def __init__(self, test, modules):
super().__init__()
self.test = test
self.modules = modules
def find_spec(self, fullname, path, target=None):
if fullname in self.modules:
self.test.fail_message = (
f"This test imports the '{fullname}' module, which it "
f'should not in the test environment. Please add '
f'appropriate mocking to this test.'
)
raise ImportError(fullname)
def __init__(self, module_names):
self.module_names = module_names
self.fail_message = ''
if isinstance(module_names, str):
self.module_names = set([module_names])
sys.meta_path.insert(0, self.ForbiddenModules(self, self.module_names))
def setUp(self):
super().setUp()
self.addCleanup(self.cleanup)
def cleanup(self):
# We use a flag and check it during the cleanup phase to fail the test
# if needed. This is done because some module imports occur inside of a
# try-except block that ignores all exceptions, so raising an exception
# there (which is also what self.assert* and self.fail() do underneath)
# will not work to cause a failure in the test.
if self.fail_message:
raise ImportError(self.fail_message)

View File

@@ -40,6 +40,7 @@ class FakeMount(object):
class APITestCase(test.NoDBTestCase):
@mock.patch('nova.virt.disk.vfs.guestfs.VFSGuestFS', new=mock.Mock())
def test_can_resize_need_fs_type_specified(self):
imgfile = tempfile.NamedTemporaryFile()
self.addCleanup(imgfile.close)