Remove unnecessary hacking checks
The B319 check to ensure no use of xrange is not an issue in a Python 3-only codebase, while the unittest assert-focused B311, B312 and B321 checks duplicate H203, H204 and H203 (again) from hacking respectively. An unnecessary 'py3pep8' tox environment is removed since the standard 'pep8' environment uses Python 3 now. Change-Id: I8eb8c6accd1c2f2c7851a08b372235699a971ad9 Signed-off-by: Stephen Finucane <stephenfin@redhat.com>
This commit is contained in:
parent
33d42acb04
commit
029233a1a4
@ -32,16 +32,10 @@ Guidelines for writing new hacking checks
|
|||||||
on the B3xx value.
|
on the B3xx value.
|
||||||
- List the new rule in the top level HACKING.rst file
|
- List the new rule in the top level HACKING.rst file
|
||||||
- Add test cases for each new rule to barbican/tests/test_hacking.py
|
- Add test cases for each new rule to barbican/tests/test_hacking.py
|
||||||
|
|
||||||
"""
|
"""
|
||||||
|
|
||||||
oslo_namespace_imports = re.compile(r"from[\s]*oslo[.](.*)")
|
oslo_namespace_imports = re.compile(r"from[\s]*oslo[.](.*)")
|
||||||
dict_constructor_with_list_copy_re = re.compile(r".*\bdict\((\[)?(\(|\[)")
|
dict_constructor_with_list_copy_re = re.compile(r".*\bdict\((\[)?(\(|\[)")
|
||||||
assert_no_xrange_re = re.compile(r"\s*xrange\s*\(")
|
|
||||||
assert_True = re.compile(r".*assertEqual\(True, .*\)")
|
|
||||||
assert_None = re.compile(r".*assertEqual\(None, .*\)")
|
|
||||||
assert_Not_Equal = re.compile(r".*assertNotEqual\(None, .*\)")
|
|
||||||
assert_Is_Not = re.compile(r".*assertIsNot\(None, .*\)")
|
|
||||||
no_log_warn = re.compile(r".*LOG.warn\(.*\)")
|
no_log_warn = re.compile(r".*LOG.warn\(.*\)")
|
||||||
|
|
||||||
|
|
||||||
@ -179,47 +173,14 @@ def dict_constructor_with_list_copy(logical_line):
|
|||||||
|
|
||||||
B318
|
B318
|
||||||
"""
|
"""
|
||||||
msg = ("B318: Must use a dict comprehension instead of a dict constructor"
|
msg = (
|
||||||
|
"B318: Must use a dict comprehension instead of a dict constructor "
|
||||||
"with a sequence of key-value pairs."
|
"with a sequence of key-value pairs."
|
||||||
)
|
)
|
||||||
if dict_constructor_with_list_copy_re.match(logical_line):
|
if dict_constructor_with_list_copy_re.match(logical_line):
|
||||||
yield (0, msg)
|
yield (0, msg)
|
||||||
|
|
||||||
|
|
||||||
@core.flake8ext
|
|
||||||
def no_xrange(logical_line):
|
|
||||||
"""Do not use 'xrange'
|
|
||||||
|
|
||||||
B319
|
|
||||||
"""
|
|
||||||
if assert_no_xrange_re.match(logical_line):
|
|
||||||
yield(0, "B319: Do not use xrange().")
|
|
||||||
|
|
||||||
|
|
||||||
@core.flake8ext
|
|
||||||
def validate_assertTrue(logical_line):
|
|
||||||
"""Use 'assertTrue' instead of 'assertEqual'
|
|
||||||
|
|
||||||
B312
|
|
||||||
"""
|
|
||||||
if re.match(assert_True, logical_line):
|
|
||||||
msg = ("B312: Unit tests should use assertTrue(value) instead"
|
|
||||||
" of using assertEqual(True, value).")
|
|
||||||
yield(0, msg)
|
|
||||||
|
|
||||||
|
|
||||||
@core.flake8ext
|
|
||||||
def validate_assertIsNone(logical_line):
|
|
||||||
"""Use 'assertIsNone' instead of 'assertEqual'
|
|
||||||
|
|
||||||
B311
|
|
||||||
"""
|
|
||||||
if re.match(assert_None, logical_line):
|
|
||||||
msg = ("B311: Unit tests should use assertIsNone(value) instead"
|
|
||||||
" of using assertEqual(None, value).")
|
|
||||||
yield(0, msg)
|
|
||||||
|
|
||||||
|
|
||||||
@core.flake8ext
|
@core.flake8ext
|
||||||
def no_log_warn_check(logical_line):
|
def no_log_warn_check(logical_line):
|
||||||
"""Disallow 'LOG.warn'
|
"""Disallow 'LOG.warn'
|
||||||
@ -229,17 +190,3 @@ def no_log_warn_check(logical_line):
|
|||||||
msg = ("B320: LOG.warn is deprecated, please use LOG.warning!")
|
msg = ("B320: LOG.warn is deprecated, please use LOG.warning!")
|
||||||
if re.match(no_log_warn, logical_line):
|
if re.match(no_log_warn, logical_line):
|
||||||
yield(0, msg)
|
yield(0, msg)
|
||||||
|
|
||||||
|
|
||||||
@core.flake8ext
|
|
||||||
def validate_assertIsNotNone(logical_line):
|
|
||||||
"""Use 'assertIsNotNone'
|
|
||||||
|
|
||||||
B321
|
|
||||||
"""
|
|
||||||
if re.match(assert_Not_Equal, logical_line) or \
|
|
||||||
re.match(assert_Is_Not, logical_line):
|
|
||||||
msg = ("B321: Unit tests should use assertIsNotNone(value) instead"
|
|
||||||
" of using assertNotEqual(None, value) or"
|
|
||||||
" assertIsNot(None, value).")
|
|
||||||
yield(0, msg)
|
|
||||||
|
@ -118,34 +118,6 @@ class HackingTestCase(oslotest.base.BaseTestCase):
|
|||||||
self.assertEqual(0, len(list(checks.dict_constructor_with_list_copy(
|
self.assertEqual(0, len(list(checks.dict_constructor_with_list_copy(
|
||||||
" self._render_dict(xml, data_el, data.__dict__)"))))
|
" self._render_dict(xml, data_el, data.__dict__)"))))
|
||||||
|
|
||||||
def test_no_xrange(self):
|
|
||||||
self.assertEqual(1, len(list(checks.no_xrange("xrange(45)"))))
|
|
||||||
|
|
||||||
self.assertEqual(0, len(list(checks.no_xrange("range(45)"))))
|
|
||||||
|
|
||||||
def test_validate_assertTrue(self):
|
|
||||||
test_value = True
|
|
||||||
self.assertEqual(0, len(list(checks.validate_assertTrue(
|
|
||||||
"assertTrue(True)"))))
|
|
||||||
self.assertEqual(1, len(list(checks.validate_assertTrue(
|
|
||||||
"assertEqual(True, %s)" % test_value))))
|
|
||||||
|
|
||||||
def test_validate_assertIsNone(self):
|
|
||||||
test_value = None
|
|
||||||
self.assertEqual(0, len(list(checks.validate_assertIsNone(
|
|
||||||
"assertIsNone(None)"))))
|
|
||||||
self.assertEqual(1, len(list(checks.validate_assertIsNone(
|
|
||||||
"assertEqual(None, %s)" % test_value))))
|
|
||||||
|
|
||||||
def test_validate_assertIsNotNone(self):
|
|
||||||
test_value = None
|
|
||||||
self.assertEqual(0, len(list(checks.validate_assertIsNotNone(
|
|
||||||
"assertIsNotNone(NotNone)"))))
|
|
||||||
self.assertEqual(1, len(list(checks.validate_assertIsNotNone(
|
|
||||||
"assertNotEqual(None, %s)" % test_value))))
|
|
||||||
self.assertEqual(1, len(list(checks.validate_assertIsNotNone(
|
|
||||||
"assertIsNot(None, %s)" % test_value))))
|
|
||||||
|
|
||||||
def test_no_log_warn_check(self):
|
def test_no_log_warn_check(self):
|
||||||
self.assertEqual(0, len(list(checks.no_log_warn_check(
|
self.assertEqual(0, len(list(checks.no_log_warn_check(
|
||||||
"LOG.warning('This should not trigger LOG.warn"
|
"LOG.warning('This should not trigger LOG.warn"
|
||||||
|
12
tox.ini
12
tox.ini
@ -64,14 +64,6 @@ commands = {posargs}
|
|||||||
[testenv:debug]
|
[testenv:debug]
|
||||||
commands = oslo_debug_helper -t barbican/tests {posargs}
|
commands = oslo_debug_helper -t barbican/tests {posargs}
|
||||||
|
|
||||||
[testenv:py3pep8]
|
|
||||||
# This hack is in place to allow us to run py3 based flake8
|
|
||||||
# without installing barbican.
|
|
||||||
install_command = /bin/echo {packages}
|
|
||||||
commands =
|
|
||||||
pip install "hacking>=3.0,<3.1.0"
|
|
||||||
flake8 barbican setup.py
|
|
||||||
|
|
||||||
[testenv:docs]
|
[testenv:docs]
|
||||||
# This environment is called from CI scripts to test and publish
|
# This environment is called from CI scripts to test and publish
|
||||||
# the main docs to https://docs.openstack.org/barbican
|
# the main docs to https://docs.openstack.org/barbican
|
||||||
@ -175,11 +167,7 @@ commands = oslopolicy-sample-generator --config-file=etc/oslo-config-generator/p
|
|||||||
[flake8:local-plugins]
|
[flake8:local-plugins]
|
||||||
extension =
|
extension =
|
||||||
B310 = checks:CheckLoggingFormatArgs
|
B310 = checks:CheckLoggingFormatArgs
|
||||||
B311 = checks:validate_assertIsNone
|
|
||||||
B312 = checks:validate_assertTrue
|
|
||||||
B317 = checks:check_oslo_namespace_imports
|
B317 = checks:check_oslo_namespace_imports
|
||||||
B318 = checks:dict_constructor_with_list_copy
|
B318 = checks:dict_constructor_with_list_copy
|
||||||
B319 = checks:no_xrange
|
|
||||||
B320 = checks:no_log_warn_check
|
B320 = checks:no_log_warn_check
|
||||||
B321 = checks:validate_assertIsNotNone
|
|
||||||
paths = ./barbican/hacking
|
paths = ./barbican/hacking
|
||||||
|
Loading…
Reference in New Issue
Block a user