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
This commit is contained in:
Andreas Jaeger 2020-03-29 10:21:39 +02:00
parent b47d1d2b8a
commit f27a334e21
9 changed files with 46 additions and 60 deletions

View File

@ -12,7 +12,6 @@ Tempest Specific Commandments
tempest/scenario tests tempest/scenario tests
- [T104] Scenario tests require a services decorator - [T104] Scenario tests require a services decorator
- [T105] Tests cannot use setUpClass/tearDownClass - [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 - [T107] Check that a service tag isn't in the module path
- [T108] Check no hyphen at the end of rand_name() argument - [T108] Check no hyphen at the end of rand_name() argument
- [T109] Cannot use testtools.skip decorator; instead use - [T109] Cannot use testtools.skip decorator; instead use

View File

@ -23,7 +23,7 @@ CONF = config.CONF
# NOTE(zhufl): This inherits from BaseVolumeAdminTest because # NOTE(zhufl): This inherits from BaseVolumeAdminTest because
# it requires force_tenant_isolation=True, which need admin # it requires force_tenant_isolation=True, which need admin
# credentials to create non-admin users for the tests. # 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 # avoid existing volumes of pre-defined tenant
force_tenant_isolation = True force_tenant_isolation = True

View File

@ -109,7 +109,7 @@ class VolumesSnapshotListTestJSON(base.BaseVolumeTest):
snap_list = self.snapshots_client.list_snapshots( snap_list = self.snapshots_client.list_snapshots(
sort_key=sort_key, sort_dir=sort_dir)['snapshots'] sort_key=sort_key, sort_dir=sort_dir)['snapshots']
self.assertNotEmpty(snap_list) self.assertNotEmpty(snap_list)
if sort_key is 'display_name': if sort_key == 'display_name':
sort_key = 'name' sort_key = 'name'
# Note: On Cinder API, 'display_name' works as a sort key # Note: On Cinder API, 'display_name' works as a sort key
# on a request, a volume name appears as 'name' on the response. # on a request, a volume name appears as 'name' on the response.

View File

@ -15,6 +15,7 @@
import os import os
import re import re
from hacking import core
import pycodestyle 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.*') TEST_DEFINITION = re.compile(r'^\s*def test.*')
SETUP_TEARDOWN_CLASS_DEFINITION = re.compile(r'^\s+def (setUp|tearDown)Class') SETUP_TEARDOWN_CLASS_DEFINITION = re.compile(r'^\s+def (setUp|tearDown)Class')
SCENARIO_DECORATOR = re.compile(r'\s*@.*services\((.*)\)') SCENARIO_DECORATOR = re.compile(r'\s*@.*services\((.*)\)')
VI_HEADER_RE = re.compile(r"^#\s+vim?:.+")
RAND_NAME_HYPHEN_RE = re.compile(r".*rand_name\(.+[\-\_][\"\']\)") RAND_NAME_HYPHEN_RE = re.compile(r".*rand_name\(.+[\-\_][\"\']\)")
mutable_default_args = re.compile(r"^\s*def .+\((.+=\{\}|.+=\[\])") mutable_default_args = re.compile(r"^\s*def .+\((.+=\{\}|.+=\[\])")
TESTTOOLS_SKIP_DECORATOR = re.compile(r'\s*@testtools\.skip\((.*)\)') TESTTOOLS_SKIP_DECORATOR = re.compile(r'\s*@testtools\.skip\((.*)\)')
@ -39,6 +39,7 @@ NEGATIVE_TEST_DECORATOR = re.compile(
_HAVE_NEGATIVE_DECORATOR = False _HAVE_NEGATIVE_DECORATOR = False
@core.flake8ext
def import_no_clients_in_api_and_scenario_tests(physical_line, filename): def import_no_clients_in_api_and_scenario_tests(physical_line, filename):
"""Check for client imports from tempest/api & tempest/scenario tests """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")) " in tempest/api/* or tempest/scenario/* tests"))
@core.flake8ext
def scenario_tests_need_service_tags(physical_line, filename, def scenario_tests_need_service_tags(physical_line, filename,
previous_logical): previous_logical):
"""Check that scenario tests have service tags """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") "T104: Scenario tests require a service decorator")
@core.flake8ext
def no_setup_teardown_class_for_tests(physical_line, filename): def no_setup_teardown_class_for_tests(physical_line, filename):
if pycodestyle.noqa(physical_line): 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") "T105: (setUp|tearDown)Class can not be used in tests")
def no_vi_headers(physical_line, line_number, lines): @core.flake8ext
"""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"
def service_tags_not_in_module_path(physical_line, filename): def service_tags_not_in_module_path(physical_line, filename):
"""Check that a service tag isn't in the module path """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") "T107: service tag should not be in path")
@core.flake8ext
def no_hyphen_at_end_of_rand_name(logical_line, filename): def no_hyphen_at_end_of_rand_name(logical_line, filename):
"""Check no hyphen at the end of rand_name() argument """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 return 0, msg
@core.flake8ext
def no_mutable_default_args(logical_line): def no_mutable_default_args(logical_line):
"""Check that mutable object isn't used as default argument """Check that mutable object isn't used as default argument
@ -137,6 +129,7 @@ def no_mutable_default_args(logical_line):
yield (0, msg) yield (0, msg)
@core.flake8ext
def no_testtools_skip_decorator(logical_line): def no_testtools_skip_decorator(logical_line):
"""Check that methods do not have the testtools.skip decorator """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 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): line_number, lines):
"""Check that service client names of GET should be consistent """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) 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): line_number, lines):
"""Check that service client names of DELETE should be consistent """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) yield (0, msg)
@core.flake8ext
def dont_import_local_tempest_into_lib(logical_line, filename): def dont_import_local_tempest_into_lib(logical_line, filename):
"""Check that tempest.lib should not import local tempest code """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) yield (0, msg)
@core.flake8ext
def use_rand_uuid_instead_of_uuid4(logical_line, filename): def use_rand_uuid_instead_of_uuid4(logical_line, filename):
"""Check that tests use data_utils.rand_uuid() instead of uuid.uuid4() """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) yield (0, msg)
@core.flake8ext
def dont_use_config_in_tempest_lib(logical_line, filename): def dont_use_config_in_tempest_lib(logical_line, filename):
"""Check that tempest.lib doesn't use tempest config """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) 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): filename):
"""Check admin tests should exist under admin path """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: if 'tempest/api/' not in filename:
return return
if pycodestyle.noqa(physical_line):
return
if not re.match(r'class .*Test.*\(.*Admin.*\):', logical_line): if not re.match(r'class .*Test.*\(.*Admin.*\):', logical_line):
return return
@ -298,6 +294,7 @@ def dont_put_admin_tests_on_nonadmin_path(logical_line, physical_line,
yield(0, msg) yield(0, msg)
@core.flake8ext
def unsupported_exception_attribute_PY3(logical_line): def unsupported_exception_attribute_PY3(logical_line):
"""Check Unsupported 'message' exception attribute in PY3 """Check Unsupported 'message' exception attribute in PY3
@ -309,6 +306,7 @@ def unsupported_exception_attribute_PY3(logical_line):
yield(0, msg) yield(0, msg)
@core.flake8ext
def negative_test_attribute_always_applied_to_negative_tests(physical_line, def negative_test_attribute_always_applied_to_negative_tests(physical_line,
filename): filename):
"""Check ``@decorators.attr(type=['negative'])`` applied to negative tests. """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" " to all negative API tests"
) )
_HAVE_NEGATIVE_DECORATOR = False _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)

View File

@ -684,7 +684,7 @@ class Credentials(object):
def __str__(self): def __str__(self):
"""Represent only attributes included in self.ATTRIBUTES""" """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) _repr = dict((k, getattr(self, k)) for k in attrs)
return str(_repr) return str(_repr)
@ -741,7 +741,7 @@ class KeystoneV2Credentials(Credentials):
def __str__(self): def __str__(self):
"""Represent only attributes included in self.ATTRIBUTES""" """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) _repr = dict((k, getattr(self, k)) for k in attrs)
return str(_repr) return str(_repr)

View File

@ -69,7 +69,7 @@ class TestObjectClient(base.TestCase):
# If the expected initial status is not 100, then an exception # If the expected initial status is not 100, then an exception
# should be thrown and the connection closed # should be thrown and the connection closed
if initial_status is 100: if initial_status == 100:
status, reason = \ status, reason = \
self.object_client.create_object_continue(cnt, obj, req_data) self.object_client.create_object_continue(cnt, obj, req_data)
else: else:
@ -91,7 +91,7 @@ class TestObjectClient(base.TestCase):
mock_poc.return_value.endheaders.assert_called_once_with() mock_poc.return_value.endheaders.assert_called_once_with()
# The following steps are only taken if the initial status is 100 # 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 # Verify that the method returned what it was supposed to
self.assertEqual(status, 201) self.assertEqual(status, 201)

View File

@ -101,17 +101,6 @@ class HackingTestCase(base.TestCase):
'def test_fake:', './tempest/scenario/orchestration/test_fake.py', 'def test_fake:', './tempest/scenario/orchestration/test_fake.py',
"\n")) "\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): def test_service_tags_not_in_module_path(self):
self.assertTrue(checks.service_tags_not_in_module_path( self.assertTrue(checks.service_tags_not_in_module_path(
"@utils.services('compute')", "@utils.services('compute')",

View File

@ -1,7 +1,7 @@
# The order of packages is significant, because pip processes them in the order # 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 # of appearance. Changing the order has an impact on the overall integration
# process, which may cause wedges in the gate later. # 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 mock>=2.0.0 # BSD
coverage!=4.4,>=4.0 # Apache-2.0 coverage!=4.4,>=4.0 # Apache-2.0
oslotest>=3.2.0 # Apache-2.0 oslotest>=3.2.0 # Apache-2.0

21
tox.ini
View File

@ -313,7 +313,6 @@ commands =
check-uuid --fix check-uuid --fix
[hacking] [hacking]
local-check-factory = tempest.hacking.checks.factory
import_exceptions = tempest.services import_exceptions = tempest.services
[flake8] [flake8]
@ -327,6 +326,26 @@ exclude = .git,.venv,.tox,dist,doc,*egg,build
enable-extensions = H106,H203,H904 enable-extensions = H106,H203,H904
import-order-style = pep8 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] [testenv:releasenotes]
deps = deps =
-c{env:UPPER_CONSTRAINTS_FILE:https://releases.openstack.org/constraints/upper/master} -c{env:UPPER_CONSTRAINTS_FILE:https://releases.openstack.org/constraints/upper/master}