From f27a334e21c6ec97daab5e94cac51aac4711361c Mon Sep 17 00:00:00 2001 From: Andreas Jaeger Date: Sun, 29 Mar 2020 10:21:39 +0200 Subject: [PATCH] Update hacking for Python3 The repo is Python 3 now, so update hacking to version 3.0 which supports Python 3. Fix problems found. Update local hacking checks for new flake8, remove vi check, it's now in hacking as H106. The tempest plugin is used on older branches as well. We really only need hacking on this repo anyways, where we no longer support python 2, so here we make the requirement specific to python 3. Change-Id: I3f4216f66606fbc450a46c93de306399b7f3cd65 --- HACKING.rst | 1 - .../api/volume/test_volume_absolute_limits.py | 2 +- .../api/volume/test_volumes_snapshots_list.py | 2 +- tempest/hacking/checks.py | 59 ++++++------------- tempest/lib/auth.py | 4 +- .../object_storage/test_object_client.py | 4 +- tempest/tests/test_hacking.py | 11 ---- test-requirements.txt | 2 +- tox.ini | 21 ++++++- 9 files changed, 46 insertions(+), 60 deletions(-) diff --git a/HACKING.rst b/HACKING.rst index 204b3c793a..f8be0ad703 100644 --- a/HACKING.rst +++ b/HACKING.rst @@ -12,7 +12,6 @@ Tempest Specific Commandments tempest/scenario tests - [T104] Scenario tests require a services decorator - [T105] Tests cannot use setUpClass/tearDownClass -- [T106] vim configuration should not be kept in source files. - [T107] Check that a service tag isn't in the module path - [T108] Check no hyphen at the end of rand_name() argument - [T109] Cannot use testtools.skip decorator; instead use diff --git a/tempest/api/volume/test_volume_absolute_limits.py b/tempest/api/volume/test_volume_absolute_limits.py index 00a3375458..4d64a954e3 100644 --- a/tempest/api/volume/test_volume_absolute_limits.py +++ b/tempest/api/volume/test_volume_absolute_limits.py @@ -23,7 +23,7 @@ CONF = config.CONF # NOTE(zhufl): This inherits from BaseVolumeAdminTest because # it requires force_tenant_isolation=True, which need admin # credentials to create non-admin users for the tests. -class AbsoluteLimitsTests(base.BaseVolumeAdminTest): # noqa +class AbsoluteLimitsTests(base.BaseVolumeAdminTest): # noqa: T115 # avoid existing volumes of pre-defined tenant force_tenant_isolation = True diff --git a/tempest/api/volume/test_volumes_snapshots_list.py b/tempest/api/volume/test_volumes_snapshots_list.py index 8a416eabd0..f4f039c3cc 100644 --- a/tempest/api/volume/test_volumes_snapshots_list.py +++ b/tempest/api/volume/test_volumes_snapshots_list.py @@ -109,7 +109,7 @@ class VolumesSnapshotListTestJSON(base.BaseVolumeTest): snap_list = self.snapshots_client.list_snapshots( sort_key=sort_key, sort_dir=sort_dir)['snapshots'] self.assertNotEmpty(snap_list) - if sort_key is 'display_name': + if sort_key == 'display_name': sort_key = 'name' # Note: On Cinder API, 'display_name' works as a sort key # on a request, a volume name appears as 'name' on the response. diff --git a/tempest/hacking/checks.py b/tempest/hacking/checks.py index 2c40cb1871..6a97a004a7 100644 --- a/tempest/hacking/checks.py +++ b/tempest/hacking/checks.py @@ -15,6 +15,7 @@ import os import re +from hacking import core import pycodestyle @@ -25,7 +26,6 @@ PYTHON_CLIENT_RE = re.compile('import (%s)client' % '|'.join(PYTHON_CLIENTS)) TEST_DEFINITION = re.compile(r'^\s*def test.*') SETUP_TEARDOWN_CLASS_DEFINITION = re.compile(r'^\s+def (setUp|tearDown)Class') SCENARIO_DECORATOR = re.compile(r'\s*@.*services\((.*)\)') -VI_HEADER_RE = re.compile(r"^#\s+vim?:.+") RAND_NAME_HYPHEN_RE = re.compile(r".*rand_name\(.+[\-\_][\"\']\)") mutable_default_args = re.compile(r"^\s*def .+\((.+=\{\}|.+=\[\])") TESTTOOLS_SKIP_DECORATOR = re.compile(r'\s*@testtools\.skip\((.*)\)') @@ -39,6 +39,7 @@ NEGATIVE_TEST_DECORATOR = re.compile( _HAVE_NEGATIVE_DECORATOR = False +@core.flake8ext def import_no_clients_in_api_and_scenario_tests(physical_line, filename): """Check for client imports from tempest/api & tempest/scenario tests @@ -53,6 +54,7 @@ def import_no_clients_in_api_and_scenario_tests(physical_line, filename): " in tempest/api/* or tempest/scenario/* tests")) +@core.flake8ext def scenario_tests_need_service_tags(physical_line, filename, previous_logical): """Check that scenario tests have service tags @@ -67,6 +69,7 @@ def scenario_tests_need_service_tags(physical_line, filename, "T104: Scenario tests require a service decorator") +@core.flake8ext def no_setup_teardown_class_for_tests(physical_line, filename): if pycodestyle.noqa(physical_line): @@ -80,20 +83,7 @@ def no_setup_teardown_class_for_tests(physical_line, filename): "T105: (setUp|tearDown)Class can not be used in tests") -def no_vi_headers(physical_line, line_number, lines): - """Check for vi editor configuration in source files. - - By default vi modelines can only appear in the first or - last 5 lines of a source file. - - T106 - """ - # NOTE(gilliard): line_number is 1-indexed - if line_number <= 5 or line_number > len(lines) - 5: - if VI_HEADER_RE.match(physical_line): - return 0, "T106: Don't put vi configuration in source files" - - +@core.flake8ext def service_tags_not_in_module_path(physical_line, filename): """Check that a service tag isn't in the module path @@ -117,6 +107,7 @@ def service_tags_not_in_module_path(physical_line, filename): "T107: service tag should not be in path") +@core.flake8ext def no_hyphen_at_end_of_rand_name(logical_line, filename): """Check no hyphen at the end of rand_name() argument @@ -127,6 +118,7 @@ def no_hyphen_at_end_of_rand_name(logical_line, filename): return 0, msg +@core.flake8ext def no_mutable_default_args(logical_line): """Check that mutable object isn't used as default argument @@ -137,6 +129,7 @@ def no_mutable_default_args(logical_line): yield (0, msg) +@core.flake8ext def no_testtools_skip_decorator(logical_line): """Check that methods do not have the testtools.skip decorator @@ -170,7 +163,8 @@ def _common_service_clients_check(logical_line, physical_line, filename, return True -def get_resources_on_service_clients(logical_line, physical_line, filename, +@core.flake8ext +def get_resources_on_service_clients(physical_line, logical_line, filename, line_number, lines): """Check that service client names of GET should be consistent @@ -197,7 +191,8 @@ def get_resources_on_service_clients(logical_line, physical_line, filename, yield (0, msg) -def delete_resources_on_service_clients(logical_line, physical_line, filename, +@core.flake8ext +def delete_resources_on_service_clients(physical_line, logical_line, filename, line_number, lines): """Check that service client names of DELETE should be consistent @@ -223,6 +218,7 @@ def delete_resources_on_service_clients(logical_line, physical_line, filename, yield (0, msg) +@core.flake8ext def dont_import_local_tempest_into_lib(logical_line, filename): """Check that tempest.lib should not import local tempest code @@ -244,6 +240,7 @@ def dont_import_local_tempest_into_lib(logical_line, filename): yield (0, msg) +@core.flake8ext def use_rand_uuid_instead_of_uuid4(logical_line, filename): """Check that tests use data_utils.rand_uuid() instead of uuid.uuid4() @@ -260,6 +257,7 @@ def use_rand_uuid_instead_of_uuid4(logical_line, filename): yield (0, msg) +@core.flake8ext def dont_use_config_in_tempest_lib(logical_line, filename): """Check that tempest.lib doesn't use tempest config @@ -277,7 +275,8 @@ def dont_use_config_in_tempest_lib(logical_line, filename): yield(0, msg) -def dont_put_admin_tests_on_nonadmin_path(logical_line, physical_line, +@core.flake8ext +def dont_put_admin_tests_on_nonadmin_path(logical_line, filename): """Check admin tests should exist under admin path @@ -287,9 +286,6 @@ def dont_put_admin_tests_on_nonadmin_path(logical_line, physical_line, if 'tempest/api/' not in filename: return - if pycodestyle.noqa(physical_line): - return - if not re.match(r'class .*Test.*\(.*Admin.*\):', logical_line): return @@ -298,6 +294,7 @@ def dont_put_admin_tests_on_nonadmin_path(logical_line, physical_line, yield(0, msg) +@core.flake8ext def unsupported_exception_attribute_PY3(logical_line): """Check Unsupported 'message' exception attribute in PY3 @@ -309,6 +306,7 @@ def unsupported_exception_attribute_PY3(logical_line): yield(0, msg) +@core.flake8ext def negative_test_attribute_always_applied_to_negative_tests(physical_line, filename): """Check ``@decorators.attr(type=['negative'])`` applied to negative tests. @@ -330,22 +328,3 @@ def negative_test_attribute_always_applied_to_negative_tests(physical_line, " to all negative API tests" ) _HAVE_NEGATIVE_DECORATOR = False - - -def factory(register): - register(import_no_clients_in_api_and_scenario_tests) - register(scenario_tests_need_service_tags) - register(no_setup_teardown_class_for_tests) - register(no_vi_headers) - register(service_tags_not_in_module_path) - register(no_hyphen_at_end_of_rand_name) - register(no_mutable_default_args) - register(no_testtools_skip_decorator) - register(get_resources_on_service_clients) - register(delete_resources_on_service_clients) - register(dont_import_local_tempest_into_lib) - register(dont_use_config_in_tempest_lib) - register(use_rand_uuid_instead_of_uuid4) - register(dont_put_admin_tests_on_nonadmin_path) - register(unsupported_exception_attribute_PY3) - register(negative_test_attribute_always_applied_to_negative_tests) diff --git a/tempest/lib/auth.py b/tempest/lib/auth.py index 8e6d3d5fe9..3fee489c15 100644 --- a/tempest/lib/auth.py +++ b/tempest/lib/auth.py @@ -684,7 +684,7 @@ class Credentials(object): def __str__(self): """Represent only attributes included in self.ATTRIBUTES""" - attrs = [attr for attr in self.ATTRIBUTES if attr is not 'password'] + attrs = [attr for attr in self.ATTRIBUTES if attr != 'password'] _repr = dict((k, getattr(self, k)) for k in attrs) return str(_repr) @@ -741,7 +741,7 @@ class KeystoneV2Credentials(Credentials): def __str__(self): """Represent only attributes included in self.ATTRIBUTES""" - attrs = [attr for attr in self.ATTRIBUTES if attr is not 'password'] + attrs = [attr for attr in self.ATTRIBUTES if attr != 'password'] _repr = dict((k, getattr(self, k)) for k in attrs) return str(_repr) diff --git a/tempest/tests/lib/services/object_storage/test_object_client.py b/tempest/tests/lib/services/object_storage/test_object_client.py index a16d1d77ba..1749b03f05 100644 --- a/tempest/tests/lib/services/object_storage/test_object_client.py +++ b/tempest/tests/lib/services/object_storage/test_object_client.py @@ -69,7 +69,7 @@ class TestObjectClient(base.TestCase): # If the expected initial status is not 100, then an exception # should be thrown and the connection closed - if initial_status is 100: + if initial_status == 100: status, reason = \ self.object_client.create_object_continue(cnt, obj, req_data) else: @@ -91,7 +91,7 @@ class TestObjectClient(base.TestCase): mock_poc.return_value.endheaders.assert_called_once_with() # The following steps are only taken if the initial status is 100 - if initial_status is 100: + if initial_status == 100: # Verify that the method returned what it was supposed to self.assertEqual(status, 201) diff --git a/tempest/tests/test_hacking.py b/tempest/tests/test_hacking.py index 83c1abbe7e..7c31185f4f 100644 --- a/tempest/tests/test_hacking.py +++ b/tempest/tests/test_hacking.py @@ -101,17 +101,6 @@ class HackingTestCase(base.TestCase): 'def test_fake:', './tempest/scenario/orchestration/test_fake.py', "\n")) - def test_no_vi_headers(self): - # NOTE(mtreinish) The lines parameter is used only for finding the - # line location in the file. So these tests just pass a list of an - # arbitrary length to use for verifying the check function. - self.assertTrue(checks.no_vi_headers( - '# vim: tabstop=4 shiftwidth=4 softtabstop=4', 1, range(250))) - self.assertTrue(checks.no_vi_headers( - '# vim: tabstop=4 shiftwidth=4 softtabstop=4', 249, range(250))) - self.assertFalse(checks.no_vi_headers( - '# vim: tabstop=4 shiftwidth=4 softtabstop=4', 149, range(250))) - def test_service_tags_not_in_module_path(self): self.assertTrue(checks.service_tags_not_in_module_path( "@utils.services('compute')", diff --git a/test-requirements.txt b/test-requirements.txt index 196387cb75..a50905fb7c 100644 --- a/test-requirements.txt +++ b/test-requirements.txt @@ -1,7 +1,7 @@ # The order of packages is significant, because pip processes them in the order # of appearance. Changing the order has an impact on the overall integration # process, which may cause wedges in the gate later. -hacking>=1.1.0,<1.2.0 # Apache-2.0 +hacking>=3.0,<3.1.0;python_version>='3.5' # Apache-2.0 mock>=2.0.0 # BSD coverage!=4.4,>=4.0 # Apache-2.0 oslotest>=3.2.0 # Apache-2.0 diff --git a/tox.ini b/tox.ini index 64921ef497..e861c84824 100644 --- a/tox.ini +++ b/tox.ini @@ -313,7 +313,6 @@ commands = check-uuid --fix [hacking] -local-check-factory = tempest.hacking.checks.factory import_exceptions = tempest.services [flake8] @@ -327,6 +326,26 @@ exclude = .git,.venv,.tox,dist,doc,*egg,build enable-extensions = H106,H203,H904 import-order-style = pep8 +[flake8:local-plugins] +extension = + T102 = checks:import_no_clients_in_api_and_scenario_tests + T104 = checks:scenario_tests_need_service_tags + T105 = checks:no_setup_teardown_class_for_tests + T107 = checks:service_tags_not_in_module_path + T108 = checks:no_hyphen_at_end_of_rand_name + N322 = checks:no_mutable_default_args + T109 = checks:no_testtools_skip_decorator + T110 = checks:get_resources_on_service_clients + T111 = checks:delete_resources_on_service_clients + T112 = checks:dont_import_local_tempest_into_lib + T113 = checks:use_rand_uuid_instead_of_uuid4 + T114 = checks:dont_use_config_in_tempest_lib + T115 = checks:dont_put_admin_tests_on_nonadmin_path + T116 = checks:unsupported_exception_attribute_PY3 + T117 = checks:negative_test_attribute_always_applied_to_negative_tests +paths = + ./tempest/hacking + [testenv:releasenotes] deps = -c{env:UPPER_CONSTRAINTS_FILE:https://releases.openstack.org/constraints/upper/master}