Browse Source

Switch to hacking 3.0.1

In hacking 2.0 or later, local-check-factory was removed as it is not
compatible with flake8 3.x and it is advised to use flake8's local
plugins [1]. neutron-lib provided a factory to register common hacking
rules, but it no longer works with hacking 2, so we need to define rules
defined in neutron-lib as flake8 local check plugin [2] explicitly.
This needs to be done in each neutron related project, so it is the
downside of the migration to hacking 2.x (I explored a way to continue
to use the factory but failed to find a good way to achieve this) but
I believe it is good to migrate the newer libraries.

* flake8ext decorator in neutron/hacking/checks.py is also replaced with
  hacking.core.flake8ext to avoid the copy-and-paste code.
* neutron-lib dependency is updated as neutron-lib 2.3 added hacking 3 support.
* Python modules related to coding style checks (listed in blacklist.txt in
  openstack/requirements repo) are dropped from lower-constraints.txt
  as they are not actually used in tests (other than pep8).
* HackingDocTestCase is now converted into normal test cases.
  HackingDocTestCase depends on the internal of hacking and pycodestyle
  so it looks better to use normal style of writing tests.

[1] https://docs.openstack.org/releasenotes/hacking/unreleased.html#relnotes-2-0-0
[2] https://flake8.pycqa.org/en/3.7.0/user/configuration.html#using-local-plugins

Change-Id: I92cf50a84bb587a0649a7cffee15cce4ce37d086
changes/66/708966/10
Akihiro Motoki 1 year ago
committed by Brian Haley
parent
commit
52e3fee5ef
6 changed files with 89 additions and 168 deletions
  1. +1
    -11
      lower-constraints.txt
  2. +14
    -55
      neutron/hacking/checks.py
  3. +57
    -98
      neutron/tests/unit/hacking/test_checks.py
  4. +1
    -1
      requirements.txt
  5. +1
    -2
      test-requirements.txt
  6. +15
    -1
      tox.ini

+ 1
- 11
lower-constraints.txt View File

@ -2,9 +2,7 @@ alabaster==0.7.10
alembic==0.8.10
amqp==2.1.1
appdirs==1.4.3
astroid==2.1.0
Babel==2.3.4
bandit==1.1.0
bashate==0.5.1
beautifulsoup4==4.6.0
cachetools==2.0.0
@ -25,14 +23,11 @@ eventlet==0.18.2
extras==1.0.0
fasteners==0.7.0
fixtures==3.0.0
flake8-import-order==0.12
flake8==2.6.2
future==0.16.0
futurist==1.2.0
gitdb==0.6.4
GitPython==1.0.1
greenlet==0.4.10
hacking==1.1.0
httplib2==0.9.1
imagesize==0.7.1
iso8601==0.1.11
@ -49,7 +44,6 @@ logilab-common==1.4.1
logutils==0.3.5
Mako==0.4.0
MarkupSafe==1.0
mccabe==0.2.1
mock==3.0.0
monotonic==0.6;python_version<'3.3'
mox3==0.20.0
@ -57,7 +51,7 @@ msgpack-python==0.4.0
munch==2.1.0
netaddr==0.7.18
netifaces==0.10.4
neutron-lib==2.2.0
neutron-lib==2.3.0
openstackdocstheme==1.30.0
openstacksdk==0.31.2
os-client-config==1.28.0
@ -92,19 +86,15 @@ Paste==2.0.2
PasteDeploy==1.5.0
pbr==4.0.0
pecan==1.3.2
pep8==1.5.7
pika-pool==0.1.3
pika==0.10.0
positional==1.2.1
prettytable==0.7.2
psutil==3.2.2
pycadf==1.1.0
pycodestyle==2.4.0
pycparser==2.18
pyflakes==0.8.1
Pygments==2.2.0
pyinotify==0.9.6
pylint==2.2.0
PyMySQL==0.7.6
pyOpenSSL==17.1.0
pyparsing==2.1.0


+ 14
- 55
neutron/hacking/checks.py View File

@ -15,17 +15,7 @@
import os
import re
from neutron_lib.hacking import checks
def flake8ext(f):
"""Decorator to indicate flake8 extension.
This is borrowed from hacking.core.flake8ext(), but at now it is used
only for unit tests to know which are neutron flake8 extensions.
"""
f.name = __name__
return f
from hacking import core
# Guidelines for writing new hacking checks
@ -50,7 +40,7 @@ tests_imports_from2 = re.compile(r"\bfrom[\s]+neutron[\s]+import[\s]+tests\b")
import_mock = re.compile(r"\bimport[\s]+mock\b")
@flake8ext
@core.flake8ext
def check_assert_called_once_with(logical_line, filename):
"""N322 - Try to detect unintended calls of nonexistent mock methods like:
assertCalledOnceWith
@ -74,7 +64,7 @@ def check_assert_called_once_with(logical_line, filename):
yield (0, msg)
@flake8ext
@core.flake8ext
def check_asserttruefalse(logical_line, filename):
"""N328 - Don't use assertEqual(True/False, observed)."""
if 'neutron/tests/' in filename:
@ -96,7 +86,7 @@ def check_asserttruefalse(logical_line, filename):
yield (0, msg)
@flake8ext
@core.flake8ext
def check_assertempty(logical_line, filename):
"""N330 - Enforce using assertEqual parameter ordering in case of empty
objects.
@ -111,7 +101,7 @@ def check_assertempty(logical_line, filename):
yield (0, msg)
@flake8ext
@core.flake8ext
def check_assertisinstance(logical_line, filename):
"""N331 - Enforce using assertIsInstance."""
if 'neutron/tests/' in filename:
@ -122,7 +112,7 @@ def check_assertisinstance(logical_line, filename):
yield (0, msg)
@flake8ext
@core.flake8ext
def check_assertequal_for_httpcode(logical_line, filename):
"""N332 - Enforce correct oredering for httpcode in assertEqual."""
msg = ("N332: Use assertEqual(expected_http_code, observed_http_code) "
@ -133,18 +123,9 @@ def check_assertequal_for_httpcode(logical_line, filename):
yield (0, msg)
@flake8ext
@core.flake8ext
def check_oslo_i18n_wrapper(logical_line, filename, noqa):
"""N340 - Check for neutron.i18n usage.
Okay(neutron/foo/bar.py): from neutron._i18n import _
Okay(neutron_fwaas/foo/bar.py): from neutron_fwaas._i18n import _
N340(neutron/foo/bar.py): from neutron.i18n import _
N340(neutron_fwaas/foo/bar.py): from neutron_fwaas.i18n import _
N340(neutron_fwaas/foo/bar.py): from neutron.i18n import _
N340(neutron_fwaas/foo/bar.py): from neutron._i18n import _
Okay(neutron/foo/bar.py): from neutron.i18n import _ # noqa
"""
"""N340 - Check for neutron.i18n usage."""
if noqa:
return
@ -162,16 +143,9 @@ def check_oslo_i18n_wrapper(logical_line, filename, noqa):
yield (0, msg)
@flake8ext
@core.flake8ext
def check_builtins_gettext(logical_line, tokens, filename, lines, noqa):
"""N341 - Check usage of builtins gettext _().
Okay(neutron/foo.py): from neutron._i18n import _\n_('foo')
N341(neutron/foo.py): _('foo')
Okay(neutron/_i18n.py): _('foo')
Okay(neutron/i18n.py): _('foo')
Okay(neutron/foo.py): _('foo') # noqa
"""
"""N341 - Check usage of builtins gettext _()."""
if noqa:
return
@ -202,7 +176,7 @@ def check_builtins_gettext(logical_line, tokens, filename, lines, noqa):
yield (0, msg)
@flake8ext
@core.flake8ext
def check_no_imports_from_tests(logical_line, filename, noqa):
"""N343 - Production code must not import from neutron.tests.*
"""
@ -219,7 +193,7 @@ def check_no_imports_from_tests(logical_line, filename, noqa):
yield(0, msg)
@flake8ext
@core.flake8ext
def check_python3_no_filter(logical_line):
"""N344 - Use list comprehension instead of filter(lambda)."""
@ -231,7 +205,7 @@ def check_python3_no_filter(logical_line):
# TODO(boden): rehome this check to neutron-lib
@flake8ext
@core.flake8ext
def check_no_sqlalchemy_event_import(logical_line, filename, noqa):
"""N346 - Use neutron_lib.db.api.sqla_listen rather than sqlalchemy."""
if noqa:
@ -248,7 +222,7 @@ def check_no_sqlalchemy_event_import(logical_line, filename, noqa):
"between unit tests")
@flake8ext
@core.flake8ext
def check_no_import_mock(logical_line, filename, noqa):
"""N347 - Test code must not import mock library
"""
@ -262,18 +236,3 @@ def check_no_import_mock(logical_line, filename, noqa):
if re.match(import_mock, logical_line):
yield(0, msg)
def factory(register):
checks.factory(register)
register(check_assert_called_once_with)
register(check_asserttruefalse)
register(check_assertempty)
register(check_assertisinstance)
register(check_assertequal_for_httpcode)
register(check_oslo_i18n_wrapper)
register(check_builtins_gettext)
register(check_no_imports_from_tests)
register(check_python3_no_filter)
register(check_no_sqlalchemy_event_import)
register(check_no_import_mock)

+ 57
- 98
neutron/tests/unit/hacking/test_checks.py View File

@ -10,16 +10,11 @@
# License for the specific language governing permissions and limitations
# under the License.
import io
import re
import tokenize
from flake8 import engine
from hacking.tests import test_doctest as hacking_doctest
import pkg_resources
import pycodestyle
import testscenarios
import testtools
from testtools import content
from testtools import matchers
from neutron.hacking import checks
from neutron.tests import base
@ -30,12 +25,14 @@ CREATE_DUMMY_MATCH_OBJECT = re.compile('a')
class HackingTestCase(base.BaseTestCase):
def assertLinePasses(self, func, line):
def assertLinePasses(self, func, line, *args, **kwargs):
with testtools.ExpectedException(StopIteration):
next(func(line))
next(func(line, *args, **kwargs))
def assertLineFails(self, func, line):
self.assertIsInstance(next(func(line)), tuple)
def assertLineFails(self, expected_code, func, line, *args, **kwargs):
value = next(func(line, *args, **kwargs))
self.assertIsInstance(value, tuple)
self.assertIn(expected_code, value[1])
def test_assert_called_once_with(self):
fail_code2 = """
@ -201,9 +198,9 @@ class HackingTestCase(base.BaseTestCase):
checks.check_no_imports_from_tests(
fail_code, "neutron/tests/test_fake.py", None))))
def test_check_python3_filter(self):
def test_check_python3_no_filter(self):
f = checks.check_python3_no_filter
self.assertLineFails(f, "filter(lambda obj: test(obj), data)")
self.assertLineFails('N344', f, "filter(lambda obj: test(obj), data)")
self.assertLinePasses(f, "[obj for obj in data if test(obj)]")
self.assertLinePasses(f, "filter(function, range(0,10))")
self.assertLinePasses(f, "lambda x, y: x+y")
@ -226,88 +223,50 @@ class HackingTestCase(base.BaseTestCase):
checks.check_no_import_mock(
fail_line, "neutron/tests/test_fake.py", None))))
# The following is borrowed from hacking/tests/test_doctest.py.
# Tests defined in docstring is easier to understand
# in some cases, for example, hacking rules which take tokens as argument.
# TODO(amotoki): Migrate existing unit tests above to docstring tests.
# NOTE(amotoki): Is it better to enhance HackingDocTestCase in hacking repo to
# pass filename to pycodestyle.Checker so that we can reuse it in this test.
# I am not sure whether unit test class is public.
SELFTEST_REGEX = re.compile(r'\b(Okay|N\d{3})(\((\S+)\))?:\s(.*)')
# Each scenario is (name, dict(filename=..., lines=.., options=..., code=...))
file_cases = []
class HackingDocTestCase(hacking_doctest.HackingTestCase):
scenarios = file_cases
def test_pycodestyle(self):
# NOTE(jecarey): Add tests marked as off_by_default to enable testing
turn_on = set(['H106'])
if self.options.select:
turn_on.update(self.options.select)
self.options.select = tuple(turn_on)
self.options.ignore = ('N530',)
report = pycodestyle.BaseReport(self.options)
checker = pycodestyle.Checker(filename=self.filename, lines=self.lines,
options=self.options, report=report)
checker.check_all()
self.addDetail('doctest', content.text_content(self.raw))
if self.code == 'Okay':
self.assertThat(
len(report.counters),
matchers.Not(matchers.GreaterThan(
len(self.options.benchmark_keys))),
"incorrectly found %s" % ', '.join(
[key for key in report.counters
if key not in self.options.benchmark_keys]))
else:
self.addDetail('reason',
content.text_content("Failed to trigger rule %s" %
self.code))
self.assertIn(self.code, report.counters)
def _get_lines(check):
for line in check.__doc__.splitlines():
line = line.lstrip()
match = SELFTEST_REGEX.match(line)
if match is None:
continue
yield (line, match.groups())
def load_tests(loader, tests, pattern):
default_checks = [e.name for e
in pkg_resources.iter_entry_points('flake8.extension')]
flake8_style = engine.get_style_guide(
parse_argv=False,
# We are testing neutron-specific hacking rules, so there is no need
# to run the checks registered by hacking or other flake8 extensions.
ignore=default_checks)
options = flake8_style.options
for name, check in checks.__dict__.items():
if not hasattr(check, 'name'):
continue
if check.name != checks.__name__:
continue
if not check.__doc__:
continue
for (lineno, (raw, line)) in enumerate(_get_lines(check)):
code, __, filename, source = line
lines = [part.replace(r'\t', '\t') + '\n'
for part in source.split(r'\n')]
file_cases.append(("%s-line-%s" % (name, lineno),
dict(lines=lines, raw=raw, options=options,
code=code, filename=filename)))
return testscenarios.load_tests_apply_scenarios(loader, tests, pattern)
def test_check_oslo_i18n_wrapper(self):
def _pass(line, filename, noqa=False):
self.assertLinePasses(
checks.check_oslo_i18n_wrapper,
line, filename, noqa)
def _fail(line, filename):
self.assertLineFails(
"N340", checks.check_oslo_i18n_wrapper,
line, filename, noqa=False)
_pass("from neutron._i18n import _", "neutron/foo/bar.py")
_pass("from neutron_fwaas._i18n import _", "neutron_fwaas/foo/bar.py")
_fail("from neutron.i18n import _", "neutron/foo/bar.py")
_fail("from neutron_fwaas.i18n import _", "neutron_fwaas/foo/bar.py")
_fail("from neutron.i18n import _", "neutron_fwaas/foo/bar.py")
_fail("from neutron._i18n import _", "neutron_fwaas/foo/bar.py")
_pass("from neutron.i18n import _", "neutron/foo/bar.py", noqa=True)
def test_check_builtins_gettext(self):
# NOTE: check_builtins_gettext() takes two additional arguments,
# "tokens" and "lines". "tokens" is a list of tokens from the target
# logical line, and "lines" is a list of lines of the input file.
# Considering this, test functions (_pass and _fail) take "lines"
# as an argument and calls the hacking check function line by line
# after generating tokens from the target line.
def _get_tokens(line):
return tokenize.tokenize(io.BytesIO(line.encode('utf-8')).readline)
def _pass(lines, filename, noqa=False):
for line in lines:
self.assertLinePasses(
checks.check_builtins_gettext,
line, _get_tokens(line), filename, lines, noqa)
def _fail(lines, filename):
for line in lines:
self.assertLineFails(
"N341", checks.check_builtins_gettext,
line, _get_tokens(line), filename, lines, noqa=False)
_pass(["from neutron._i18n import _", "_('foo')"], "neutron/foo.py")
_fail(["_('foo')"], "neutron/foo.py")
_pass(["_('foo')"], "neutron/_i18n.py")
_pass(["_('foo')"], "neutron/i18n.py")
_pass(["_('foo')"], "neutron/foo.py", noqa=True)

+ 1
- 1
requirements.txt View File

@ -16,7 +16,7 @@ Jinja2>=2.10 # BSD License (3 clause)
keystonemiddleware>=4.17.0 # Apache-2.0
netaddr>=0.7.18 # BSD
netifaces>=0.10.4 # MIT
neutron-lib>=2.2.0 # Apache-2.0
neutron-lib>=2.3.0 # Apache-2.0
python-neutronclient>=6.7.0 # Apache-2.0
tenacity>=4.4.0 # Apache-2.0
SQLAlchemy>=1.2.0 # MIT


+ 1
- 2
test-requirements.txt View File

@ -1,12 +1,11 @@
# 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.1,<3.1.0 # Apache-2.0
bandit!=1.6.0,>=1.1.0 # Apache-2.0
coverage!=4.4,>=4.0 # Apache-2.0
fixtures>=3.0.0 # Apache-2.0/BSD
flake8-import-order==0.12 # LGPLv3
pycodestyle>=2.0.0 # MIT
python-subunit>=1.0.0 # Apache-2.0/BSD
testtools>=2.2.0 # MIT
testresources>=2.0.0 # Apache-2.0/BSD


+ 15
- 1
tox.ini View File

@ -169,9 +169,23 @@ show-source = true
exclude = ./.*,build,dist,doc
import-order-style = pep8
[flake8:local-plugins]
extension =
# Checks specific to neutron repo
N340 = neutron.hacking.checks:check_oslo_i18n_wrapper
N341 = neutron.hacking.checks:check_builtins_gettext
# Checks from neutron-lib
N521 = neutron_lib.hacking.checks:use_jsonutils
N524 = neutron_lib.hacking.checks:check_no_contextlib_nested
N529 = neutron_lib.hacking.checks:no_mutable_default_args
N530 = neutron_lib.hacking.checks:check_neutron_namespace_imports
N532 = neutron_lib.hacking.translation_checks:check_log_warn_deprecated
N534 = neutron_lib.hacking.translation_checks:check_raised_localized_exceptions
N536 = neutron_lib.hacking.checks:assert_equal_none
N537 = neutron_lib.hacking.translation_checks:no_translate_logs
[hacking]
import_exceptions = neutron._i18n
local-check-factory = neutron.hacking.checks.factory
[testenv:bandit]
envdir = {toxworkdir}/shared


Loading…
Cancel
Save