From 091e3ea8fd3541e0edb98e31f9484cc08b0e5413 Mon Sep 17 00:00:00 2001 From: melanie witt Date: Tue, 16 May 2023 01:34:00 +0000 Subject: [PATCH] testing: Use inspect.isfunction() to check signatures We recently discovered that nova.test.SubclassSignatureTestCase has not worked properly since support for python 2.7 was dropped. The reason is the use of the inspect.ismethod() function; in python 2.7 it returned true if the object was a bound or an unbound method [2] but as of python 3, it only returns true if the object is a bound method [2]. Because of this, LibvirtBaseVolumeDriverSubclassSignatureTestCase was not actually checking any signatures. This replaces the use of inspect.ismethod() with inspect.isfunction() to fix the issue. It also adds a skip for subclass private functions as these are expected to be different among subclasses. One subclass method signature caught by the test is also updated to match the base class. [1] https://docs.python.org/2.7/library/inspect.html#inspect.ismethod [2] https://docs.python.org/3.5/library/inspect.html#inspect.ismethod Closes-Bug: #2019772 Change-Id: I5644effac7206035de77e0d1cd450a9fb96c3a0d --- nova/test.py | 6 +++++- nova/tests/unit/test_test.py | 19 +++++++++++++++++++ nova/virt/libvirt/volume/lightos.py | 2 +- 3 files changed, 25 insertions(+), 2 deletions(-) diff --git a/nova/test.py b/nova/test.py index e37967b06d83..cc2f5e6f03c5 100644 --- a/nova/test.py +++ b/nova/test.py @@ -718,11 +718,15 @@ class SubclassSignatureTestCase(testtools.TestCase, metaclass=abc.ABCMeta): # getmembers returns all members, including members inherited from # the base class. It's redundant for us to test these, but as # they'll always pass it's not worth the complexity to filter them out. - for (name, method) in inspect.getmembers(cls, inspect.ismethod): + for (name, method) in inspect.getmembers(cls, inspect.isfunction): # Subclass __init__ methods can usually be legitimately different if name == '__init__': continue + # Skip subclass private functions + if name.startswith('_'): + continue + while hasattr(method, '__wrapped__'): # This is a wrapped function. The signature we're going to # see here is that of the wrapper, which is almost certainly diff --git a/nova/tests/unit/test_test.py b/nova/tests/unit/test_test.py index 1042153b10b0..caf3f0b98874 100644 --- a/nova/tests/unit/test_test.py +++ b/nova/tests/unit/test_test.py @@ -18,6 +18,7 @@ import os.path import tempfile +import unittest from unittest import mock import uuid @@ -415,3 +416,21 @@ class PatchOpenTestCase(test.NoDBTestCase): without changing other file operations. """ self._test_patched_open() + + +class SubclassSignatureTestCaseTestCase(test.SubclassSignatureTestCase): + class Baseclass(): + def method(self): + pass + + class Subclass(Baseclass): + def method(self, different): + pass + + def _get_base_class(self): + return SubclassSignatureTestCaseTestCase.Baseclass + + @unittest.expectedFailure + def test_signatures(self): + # Test that the signature check fails when it's supposed to fail. + super().test_signatures() diff --git a/nova/virt/libvirt/volume/lightos.py b/nova/virt/libvirt/volume/lightos.py index 6a22bf6dc63c..c48890997b7f 100644 --- a/nova/virt/libvirt/volume/lightos.py +++ b/nova/virt/libvirt/volume/lightos.py @@ -52,7 +52,7 @@ class LibvirtLightOSVolumeDriver(libvirt_volume.LibvirtVolumeDriver): super(LibvirtLightOSVolumeDriver, self).disconnect_volume( connection_info, instance, force=force) - def extend_volume(self, connection_info, instance, requested_size=None): + def extend_volume(self, connection_info, instance, requested_size): """Extend the volume.""" LOG.debug("calling os-brick to extend LightOS Volume." "instance:%s, volume_id:%s",