author | Andrew Halberstadt <ahalberstadt@mozilla.com> |
Thu, 22 Mar 2018 17:24:15 -0400 | |
changeset 410601 | ad34ac3d45a62dff961bd247e36ab209af2cc0e5 |
parent 410600 | 430a1c231025cee13e68fccd07883dcceb741aad |
child 410602 | 26240d2027da0d4d010f7009ce4509e2c6969da2 |
push id | 33733 |
push user | aciure@mozilla.com |
push date | Thu, 29 Mar 2018 22:05:29 +0000 |
treeherder | mozilla-central@7ca58ce09779 [default view] [failures only] |
perfherder | [talos] [build metrics] [platform microbench] (compared to previous push) |
reviewers | andi |
bugs | 1436802, 1277851 |
milestone | 61.0a1 |
first release with | nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
|
last release without | nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
|
--- 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', ]