diff options
| -rw-r--r-- | lower-constraints.txt | 12 | ||||
| -rw-r--r-- | neutron/hacking/checks.py | 69 | ||||
| -rw-r--r-- | neutron/tests/unit/hacking/test_checks.py | 155 | ||||
| -rw-r--r-- | requirements.txt | 2 | ||||
| -rw-r--r-- | test-requirements.txt | 3 | ||||
| -rw-r--r-- | tox.ini | 16 |
6 files changed, 89 insertions, 168 deletions
diff --git a/lower-constraints.txt b/lower-constraints.txt index 693ffa0386..a6e9a37786 100644 --- a/lower-constraints.txt +++ b/lower-constraints.txt @@ -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 diff --git a/neutron/hacking/checks.py b/neutron/hacking/checks.py index eea4d97961..a9b7d1e6c9 100644 --- a/neutron/hacking/checks.py +++ b/neutron/hacking/checks.py @@ -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) diff --git a/neutron/tests/unit/hacking/test_checks.py b/neutron/tests/unit/hacking/test_checks.py index 82943068d5..7e6ecfb896 100644 --- a/neutron/tests/unit/hacking/test_checks.py +++ b/neutron/tests/unit/hacking/test_checks.py @@ -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) diff --git a/requirements.txt b/requirements.txt index 875fc4ba82..727ecd43cf 100644 --- a/requirements.txt +++ b/requirements.txt @@ -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 diff --git a/test-requirements.txt b/test-requirements.txt index b3101e6ebf..6a8a310089 100644 --- a/test-requirements.txt +++ b/test-requirements.txt @@ -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 @@ -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 |