Bug 1436802 - [lint] Add some tests for the flake8 linter integration
This essentially tests tools/lint/python/flake8.py. Though it also adds
a basic framework for testing all the other linters as well. Getting this
added now will allow others to collaborate on adding more tests without
needing to get to 100% coverage for all linters right off the bat.
All python tests under tools/lint/test will run as part of the 'ml' task
on Linux, and the build task on Windows (OSX coverage is currently missing
for python tests).
The flake8 linter currently has a bug where the 'exclude' argument is
ignored. This is why we need to also exclude 'tools/lint/test/files' in
topsrcdir/.flake8, even though it is already listed in the
'mach_commands.py'. Other linters shouldn't need to do this, the exclusion
in 'mach_commands.py' should be good enough. See
bug 1277851 for more
details.
MozReview-Commit-ID: 9ho8C83eeuj
--- a/.flake8
+++ b/.flake8
@@ -1,6 +1,7 @@
[flake8]
# See http://pep8.readthedocs.io/en/latest/intro.html#configuration
ignore = E121, E123, E126, E129, E133, E226, E241, E242, E704, W503, E402, E741
max-line-length = 99
exclude =
testing/mochitest/pywebsocket,
+ tools/lint/test/files,
--- a/taskcluster/ci/source-test/python.yml
+++ b/taskcluster/ci/source-test/python.yml
@@ -102,16 +102,17 @@ mozlint:
description: python/mozlint unit tests
treeherder:
symbol: py(ml)
run:
mach: python-test --subsuite mozlint
when:
files-changed:
- 'python/mozlint/**'
+ - 'tools/lint/**'
mozterm:
description: python/mozterm unit tests
treeherder:
symbol: py(term)
run:
mach: python-test --subsuite mozterm
when:
--- a/tools/lint/mach_commands.py
+++ b/tools/lint/mach_commands.py
@@ -33,17 +33,17 @@ class MachCommands(MachCommandBase):
@Command(
'lint', category='devenv',
description='Run linters.',
parser=setup_argument_parser)
def lint(self, *runargs, **lintargs):
"""Run linters."""
from mozlint import cli
lintargs.setdefault('root', self.topsrcdir)
- lintargs['exclude'] = ['obj*']
+ lintargs['exclude'] = ['obj*', 'tools/lint/test/files']
cli.SEARCH_PATHS.append(here)
self._activate_virtualenv()
return cli.run(*runargs, **lintargs)
@Command('eslint', category='devenv',
description='Run eslint or help configure eslint for optimal development.')
@CommandArgument('paths', default=None, nargs='*',
help="Paths to file or directories to lint, like "
--- a/tools/lint/python/flake8.py
+++ b/tools/lint/python/flake8.py
@@ -136,16 +136,20 @@ def run_process(config, cmd):
def setup(root):
if not reinstall_flake8():
print(FLAKE8_INSTALL_ERROR)
return 1
def lint(paths, config, **lintargs):
+ # TODO don't store results in a global
+ global results
+ results = []
+
cmdargs = [
os.path.join(bindir, 'flake8'),
'--format', '{"path":"%(path)s","lineno":%(row)s,'
'"column":%(col)s,"rule":"%(code)s","message":"%(text)s"}',
]
# Run any paths with a .flake8 file in the directory separately so
# it gets picked up. This means only .flake8 files that live in
new file mode 100644
--- /dev/null
+++ b/tools/lint/test/conftest.py
@@ -0,0 +1,98 @@
+import os
+import sys
+from collections import defaultdict
+
+from mozbuild.base import MozbuildObject
+from mozlint.pathutils import findobject
+from mozlint.parser import Parser
+
+import pytest
+
+here = os.path.abspath(os.path.dirname(__file__))
+build = MozbuildObject.from_environment(cwd=here)
+
+lintdir = os.path.dirname(here)
+sys.path.insert(0, lintdir)
+
+
+@pytest.fixture(scope='module')
+def root(request):
+ """Return the root directory for the files of the linter under test.
+
+ For example, with LINTER=flake8 this would be tools/lint/test/files/flake8.
+ """
+ if not hasattr(request.module, 'LINTER'):
+ pytest.fail("'root' fixture used from a module that didn't set the LINTER variable")
+ return os.path.join(here, 'files', request.module.LINTER)
+
+
+@pytest.fixture(scope='module')
+def paths(root):
+ """Return a function that can resolve file paths relative to the linter
+ under test.
+
+ Can be used like `paths('foo.py', 'bar/baz')`. This will return a list of
+ absolute paths under the `root` files directory.
+ """
+ def _inner(*paths):
+ if not paths:
+ return [root]
+ return [os.path.normpath(os.path.join(root, p)) for p in paths]
+ return _inner
+
+
+@pytest.fixture(scope='module')
+def config(request):
+ """Finds, loads and returns the config for the linter name specified by the
+ LINTER global variable in the calling module.
+
+ This implies that each test file (that uses this fixture) should only be
+ used to test a single linter. If no LINTER variable is defined, the test
+ will fail.
+ """
+ if not hasattr(request.module, 'LINTER'):
+ pytest.fail("'config' fixture used from a module that didn't set the LINTER variable")
+
+ name = request.module.LINTER
+ config_path = os.path.join(lintdir, '{}.yml'.format(name))
+ parser = Parser()
+ # TODO Don't assume one linter per yaml file
+ return parser.parse(config_path)[0]
+
+
+@pytest.fixture(scope='module', autouse=True)
+def run_setup(config):
+ """Make sure that if the linter named in the LINTER global variable has a
+ setup function, it gets called before running the tests.
+ """
+ if 'setup' not in config:
+ return
+
+ func = findobject(config['setup'])
+ func(build.topsrcdir)
+
+
+@pytest.fixture(scope='module')
+def lint(config, root):
+ """Find and return the 'lint' function for the external linter named in the
+ LINTER global variable.
+
+ This will automatically pass in the 'config' and 'root' arguments if not
+ specified.
+ """
+ try:
+ func = findobject(config['payload'])
+ except (ImportError, ValueError):
+ pytest.fail("could not resolve a lint function from '{}'".format(config['payload']))
+
+ def wrapper(paths, config=config, root=root, collapse_results=False, **lintargs):
+ results = func(paths, config, root=root, **lintargs)
+ if not collapse_results:
+ return results
+
+ ret = defaultdict(list)
+ for r in results:
+ ret[r.path].append(r)
+ return ret
+
+ return wrapper
new file mode 100644
--- /dev/null
+++ b/tools/lint/test/files/flake8/bad.py
@@ -0,0 +1,5 @@
+# Unused import
+import distutils
+
+print("This is a line that is over 80 characters but under 100. It shouldn't fail.")
+print("This is a line that is over not only 80, but 100 characters. It should most certainly cause a failure.")
new file mode 100644
--- /dev/null
+++ b/tools/lint/test/files/flake8/custom/.flake8
@@ -0,0 +1,4 @@
+[flake8]
+max-line-length=110
+ignore=
+ F401
new file mode 100644
--- /dev/null
+++ b/tools/lint/test/files/flake8/custom/good.py
@@ -0,0 +1,5 @@
+# Unused import
+import distutils
+
+print("This is a line that is over 80 characters but under 100. It shouldn't fail.")
+print("This is a line that is over not only 80, but 100 characters. It should also not cause a failure.")
new file mode 100644
--- /dev/null
+++ b/tools/lint/test/python.ini
@@ -0,0 +1,4 @@
+[DEFAULT]
+subsuite=mozlint, os == "linux"
+
+[test_flake8.py]
new file mode 100644
--- /dev/null
+++ b/tools/lint/test/test_flake8.py
@@ -0,0 +1,45 @@
+import mozunit
+import pytest
+
+LINTER = 'flake8'
+
+
+def test_lint_single_file(lint, paths):
+ results = lint(paths('bad.py'))
+ assert len(results) == 2
+ assert results[0].rule == 'F401'
+ assert results[1].rule == 'E501'
+ assert results[1].lineno == 5
+
+ # run lint again to make sure the previous results aren't counted twice
+ results = lint(paths('bad.py'))
+ assert len(results) == 2
+
+
+def test_lint_custom_config(lint, paths):
+ results = lint(paths('custom'))
+ assert len(results) == 0
+
+ results = lint(paths('custom/good.py'))
+ assert len(results) == 0
+
+ results = lint(paths('custom', 'bad.py'))
+ assert len(results) == 2
+
+
+@pytest.mark.xfail(
+ strict=True, reason="Bug 1277851 - custom configs are ignored if specifying a parent path")
+def test_lint_custom_config_from_parent_path(lint, paths):
+ results = lint(paths(), collapse_results=True)
+ assert paths('custom/good.py')[0] not in results
+
+
+@pytest.mark.xfail(strict=True, reason="Bug 1277851 - 'exclude' argument is ignored")
+def test_lint_excluded_file(lint, paths):
+ paths = paths('bad.py')
+ results = lint(paths, exclude=paths)
+ assert len(results) == 0
+
+
+if __name__ == '__main__':
+ mozunit.main()
--- a/tools/moz.build
+++ b/tools/moz.build
@@ -59,10 +59,11 @@ SPHINX_TREES['try'] = 'tryselect/docs'
with Files('tryselect/docs/**'):
SCHEDULES.exclusive = ['docs']
CRAMTEST_MANIFESTS += [
'tryselect/test/cram.ini',
]
PYTHON_UNITTEST_MANIFESTS += [
+ 'lint/test/python.ini',
'tryselect/test/python.ini',
]