From 9aa23f307dd22c10dc519ece42c2599f83cc612e Mon Sep 17 00:00:00 2001 From: Jordan Pittier Date: Tue, 14 Feb 2017 17:30:31 +0100 Subject: [PATCH] Deprecate the skip_unless_attr decorator. This decorator was used at only one place (ListServerFiltersTestJSON) and those tests are skipped in the Gate anyway (SKIPPED: Only one image found) These tests were poorly written anyway, the resource_setup() method goes against are principles in [1]: Using discovery for skipping tests is generally discouraged. This decorator encourages bad practise, like the usage of class variables. We should use the generic and well known testtools.SkipIf/Unless decorator instead. [1] : https://github.com/openstack/tempest/blob/master/HACKING.rst#skipping-tests Change-Id: I639f324d5b38cd154b3ecdb89b56ff2ee279c4ff --- ...nless_attr-decorator-450a1ed727494724.yaml | 5 ++ .../servers/test_list_server_filters.py | 46 ++++++------------- tempest/lib/decorators.py | 2 + 3 files changed, 22 insertions(+), 31 deletions(-) create mode 100644 releasenotes/notes/deprecate-skip_unless_attr-decorator-450a1ed727494724.yaml diff --git a/releasenotes/notes/deprecate-skip_unless_attr-decorator-450a1ed727494724.yaml b/releasenotes/notes/deprecate-skip_unless_attr-decorator-450a1ed727494724.yaml new file mode 100644 index 0000000000..4d8b941570 --- /dev/null +++ b/releasenotes/notes/deprecate-skip_unless_attr-decorator-450a1ed727494724.yaml @@ -0,0 +1,5 @@ +--- +deprecations: + - The ``skip_unless_attr`` decorator in lib/decorators.py has been deprecated, + please use the standard ``testtools.skipUnless`` and ``testtools.skipIf`` + decorators. diff --git a/tempest/api/compute/servers/test_list_server_filters.py b/tempest/api/compute/servers/test_list_server_filters.py index c0a8eaec11..d774f7b7c7 100644 --- a/tempest/api/compute/servers/test_list_server_filters.py +++ b/tempest/api/compute/servers/test_list_server_filters.py @@ -12,13 +12,17 @@ # WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the # License for the specific language governing permissions and limitations # under the License. +import testtools from tempest.api.compute import base from tempest.common import fixed_network from tempest.common.utils import data_utils from tempest.common import waiters +from tempest import config from tempest.lib import decorators -from tempest.lib import exceptions as lib_exc + + +CONF = config.CONF class ListServerFiltersTestJSON(base.BaseV2ComputeTest): @@ -37,31 +41,6 @@ class ListServerFiltersTestJSON(base.BaseV2ComputeTest): def resource_setup(cls): super(ListServerFiltersTestJSON, cls).resource_setup() - # Check to see if the alternate image ref actually exists... - images_client = cls.compute_images_client - images = images_client.list_images()['images'] - - if cls.image_ref != cls.image_ref_alt and \ - any([image for image in images - if image['id'] == cls.image_ref_alt]): - cls.multiple_images = True - else: - cls.image_ref_alt = cls.image_ref - - # Do some sanity checks here. If one of the images does - # not exist, fail early since the tests won't work... - try: - cls.compute_images_client.show_image(cls.image_ref) - except lib_exc.NotFound: - raise RuntimeError("Image %s (image_ref) was not found!" % - cls.image_ref) - - try: - cls.compute_images_client.show_image(cls.image_ref_alt) - except lib_exc.NotFound: - raise RuntimeError("Image %s (image_ref_alt) was not found!" % - cls.image_ref_alt) - network = cls.get_tenant_network() if network: cls.fixed_network_name = network.get('name') @@ -74,9 +53,12 @@ class ListServerFiltersTestJSON(base.BaseV2ComputeTest): **network_kwargs) cls.s2_name = data_utils.rand_name(cls.__name__ + '-instance') - cls.s2 = cls.create_test_server(name=cls.s2_name, - image_id=cls.image_ref_alt, - wait_until='ACTIVE') + # If image_ref_alt is "" or None then we still want to boot a server + # but we rely on `testtools.skipUnless` decorator to actually skip + # the irrelevant tests. + cls.s2 = cls.create_test_server( + name=cls.s2_name, image_id=cls.image_ref_alt or cls.image_ref, + wait_until='ACTIVE') cls.s3_name = data_utils.rand_name(cls.__name__ + '-instance') cls.s3 = cls.create_test_server(name=cls.s3_name, @@ -84,7 +66,8 @@ class ListServerFiltersTestJSON(base.BaseV2ComputeTest): wait_until='ACTIVE') @decorators.idempotent_id('05e8a8e7-9659-459a-989d-92c2f501f4ba') - @decorators.skip_unless_attr('multiple_images', 'Only one image found') + @testtools.skipUnless(CONF.compute.image_ref != CONF.compute.image_ref_alt, + "Need distinct images to run this test") def test_list_servers_filter_by_image(self): # Filter the list of servers by image params = {'image': self.image_ref} @@ -169,7 +152,8 @@ class ListServerFiltersTestJSON(base.BaseV2ComputeTest): len([x for x in servers['servers'] if 'id' in x])) @decorators.idempotent_id('b3304c3b-97df-46d2-8cd3-e2b6659724e7') - @decorators.skip_unless_attr('multiple_images', 'Only one image found') + @testtools.skipUnless(CONF.compute.image_ref != CONF.compute.image_ref_alt, + "Need distinct images to run this test") def test_list_servers_detailed_filter_by_image(self): # Filter the detailed list of servers by image params = {'image': self.image_ref} diff --git a/tempest/lib/decorators.py b/tempest/lib/decorators.py index 6ed99b4ea4..92f96985f6 100644 --- a/tempest/lib/decorators.py +++ b/tempest/lib/decorators.py @@ -15,6 +15,7 @@ import functools import uuid +import debtcollector.removals import six import testtools @@ -61,6 +62,7 @@ def idempotent_id(id): return decorator +@debtcollector.removals.remove(removal_version='Queen') class skip_unless_attr(object): """Decorator to skip tests if a specified attr does not exists or False""" def __init__(self, attr, msg=None):