Added tox for linting and testing code living uder src/python. At the moment, all linting is done through the same `pylint` installation under support/.virtualenv, which requires ALL dependencies (i.e. pip-requirements.txt, requirements.in scattered in various directories) to be installed in the same virtualenv, making things really messy -- e.g. when I've changed some code under `src/python/lib`, but don't have the dev virtualenv activated, linting will fail since none of the dependencies under `src/python/lib` have been installed.
Using tox, we can solve this problem by distributing a "test spec" (tox.ini) in each of the python source directories which are aware of its local dependencies only. To test or lint the code there would be as simple as running `tox -e py27-lint <file_path>`, and the corresponding virtualenv and test dependencies would automatically be setup. This patch modifies `support/mesos-style.py` to install `tox` in `support/.virtualenv` and delegates linting to a `tox` call when it sees python directories that have tox setup for it. Linting for all other languages will not be affected. Testing Done: 1. Intentionally create a lint error, such as extra spaces before a parenthesis in a python file 2. Run the pre-commit hook and see tox in action Reviewed at https://reviews.apache.org/r/64970/ Project: http://git-wip-us.apache.org/repos/asf/mesos/repo Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/1e761268 Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/1e761268 Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/1e761268 Branch: refs/heads/master Commit: 1e7612681a8d9f30e18ca97da428e1100ff2ce3f Parents: db87a70 Author: Eric Chung <cinchu...@gmail.com> Authored: Wed May 9 03:35:28 2018 -0700 Committer: Kevin Klues <klue...@gmail.com> Committed: Wed May 9 05:13:58 2018 -0700 ---------------------------------------------------------------------- src/python/cli_new/tests/__init__.py | 15 ++++++ src/python/cli_new/tox.ini | 22 ++++++++ src/python/lib/requirements-test.in | 4 -- src/python/lib/tox.ini | 14 ++++-- support/build-virtualenv | 1 - support/mesos-style.py | 84 ++++++++++++++++++++++--------- 6 files changed, 109 insertions(+), 31 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/mesos/blob/1e761268/src/python/cli_new/tests/__init__.py ---------------------------------------------------------------------- diff --git a/src/python/cli_new/tests/__init__.py b/src/python/cli_new/tests/__init__.py new file mode 100644 index 0000000..635f0d9 --- /dev/null +++ b/src/python/cli_new/tests/__init__.py @@ -0,0 +1,15 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. http://git-wip-us.apache.org/repos/asf/mesos/blob/1e761268/src/python/cli_new/tox.ini ---------------------------------------------------------------------- diff --git a/src/python/cli_new/tox.ini b/src/python/cli_new/tox.ini new file mode 100644 index 0000000..58ca380 --- /dev/null +++ b/src/python/cli_new/tox.ini @@ -0,0 +1,22 @@ +# Tox (http://tox.testrun.org/) is a tool for running tests +# in multiple virtualenvs. This configuration file will run the +# test suite on all supported python versions. To use it, "pip install tox" +# and then run "tox" from this directory. + +[tox] +envlist = {py27}-{lint,test} +skipsdist = true + +[testenv] +basepython = + py27: python2.7 +deps = + -rpip-requirements.txt + test: coverage + mock + pytest + pytest-cov + lint: pylint==1.6.4 +commands = + test: py.test {posargs:tests -vv --cov=lib --cov=bin --cov-report=term-missing} + lint: pylint --rcfile=../../../support/pylint.config {posargs:lib/cli tests} http://git-wip-us.apache.org/repos/asf/mesos/blob/1e761268/src/python/lib/requirements-test.in ---------------------------------------------------------------------- diff --git a/src/python/lib/requirements-test.in b/src/python/lib/requirements-test.in deleted file mode 100644 index b2b73aa..0000000 --- a/src/python/lib/requirements-test.in +++ /dev/null @@ -1,4 +0,0 @@ -coverage -mock -pytest -pytest-cov http://git-wip-us.apache.org/repos/asf/mesos/blob/1e761268/src/python/lib/tox.ini ---------------------------------------------------------------------- diff --git a/src/python/lib/tox.ini b/src/python/lib/tox.ini index fd5e89c..05b633e 100644 --- a/src/python/lib/tox.ini +++ b/src/python/lib/tox.ini @@ -4,11 +4,19 @@ # and then run "tox" from this directory. [tox] -envlist = py27 +envlist = {py27}-{lint,test} skipsdist = true [testenv] -commands = py.test tests -vv --cov=mesos --cov-report=term-missing +basepython = + py27: python2.7 deps = -rrequirements.in - -rrequirements-test.in + test: coverage + mock + pytest + pytest-cov + lint: pylint==1.6.4 +commands = + test: py.test {posargs:tests -vv --cov=mesos --cov-report=term-missing} + lint: pylint --rcfile=../../../support/pylint.config {posargs:mesos tests setup.py} http://git-wip-us.apache.org/repos/asf/mesos/blob/1e761268/support/build-virtualenv ---------------------------------------------------------------------- diff --git a/support/build-virtualenv b/support/build-virtualenv index 248b991..850af89 100755 --- a/support/build-virtualenv +++ b/support/build-virtualenv @@ -73,7 +73,6 @@ pip install -r ${CURRDIR}/pip-requirements.txt # https://issues.apache.org/jira/browse/MESOS-8206 pip install -r ${CURRDIR}/../src/python/cli_new/pip-requirements.txt pip install -r ${CURRDIR}/../src/python/lib/requirements.in -pip install -r ${CURRDIR}/../src/python/lib/requirements-test.in # Add Node.js virtual environment to the existing virtual environment. nodeenv -p http://git-wip-us.apache.org/repos/asf/mesos/blob/1e761268/support/mesos-style.py ---------------------------------------------------------------------- diff --git a/support/mesos-style.py b/support/mesos-style.py index 47ec369..07074da 100755 --- a/support/mesos-style.py +++ b/support/mesos-style.py @@ -351,7 +351,9 @@ class PyLinter(LinterBase): cli_dir = os.path.join('src', 'python', 'cli_new') lib_dir = os.path.join('src', 'python', 'lib') support_dir = 'support' - source_dirs = [cli_dir, lib_dir, support_dir] + source_dirs_to_lint_with_venv = [support_dir] + source_dirs_to_lint_with_tox = [cli_dir, lib_dir] + source_dirs = source_dirs_to_lint_with_tox + source_dirs_to_lint_with_venv exclude_files = '(' \ r'protobuf\-2\.4\.1|' \ @@ -366,38 +368,74 @@ class PyLinter(LinterBase): comment_prefix = '#' - def run_lint(self, source_paths): + pylint_config = os.path.abspath(os.path.join('support', 'pylint.config')) + + def run_tox(self, configfile, args, tox_env=None, recreate=False): """ - Runs pylint over given files. + Runs tox with given configfile and args. Optionally set tox env + and/or recreate the tox-managed virtualenv. + """ + cmd = [os.path.join(os.path.dirname(__file__), + '.virtualenv', 'bin', 'tox')] + cmd += ['-qq'] + cmd += ['-c', configfile] + if tox_env is not None: + cmd += ['-e', tox_env] + if recreate: + cmd += ['--recreate'] + cmd += ['--'] + cmd += args + + return subprocess.Popen(cmd, stdout=subprocess.PIPE) + + def filter_source_files(self, source_dir, source_files): + """ + Filters out files starting with source_dir. + """ + return [f for f in source_files if f.startswith(source_dir)] - https://google.github.io/styleguide/pyguide.html + def lint_source_files_under_source_dir(self, source_dir, source_files): + """ + Runs pylint directly or indirectly throgh tox on source_files which + are under source_dir. If tox is to be used, it must be configured + in source_dir, i.e. a tox.ini must be present. """ + filtered_source_files = self.filter_source_files( + source_dir, source_files) - num_errors = 0 + if len(filtered_source_files) == 0: + return 0 - pylint_config = os.path.join('support', 'pylint.config') + if source_dir in self.source_dirs_to_lint_with_tox: + process = self.run_tox( + configfile=os.path.join(source_dir, 'tox.ini'), + args=['--rcfile='+self.pylint_config] + filtered_source_files, + tox_env='py27-lint') + else: + process = self.run_command_in_virtualenv( + 'pylint --rcfile={rcfile} {files}'.format( + rcfile=self.pylint_config, + files=' '.join(filtered_source_files))) - source_files = '' + num_errors = 0 + for line in process.stdout: + if re.match(r'^[RCWEF]: *[\d]+', line): + num_errors += 1 + sys.stderr.write(line) - for source_dir in self.source_dirs: - source_dir_files = [] - for source_path in source_paths: - if source_path.startswith(source_dir): - source_dir_files.append(source_path) + return num_errors - source_files = ' '.join([source_files, ' '.join(source_dir_files)]) + def run_lint(self, source_paths): + """ + Runs pylint over given files. - process = self.run_command_in_virtualenv( - 'pylint --rcfile={rcfile} {files}'.format( - rcfile=pylint_config, - files=source_files - ) - ) + https://google.github.io/styleguide/pyguide.html + """ + num_errors = 0 - for line in process.stdout: - if not line.startswith('*'): - num_errors += 1 - sys.stderr.write(line) + for source_dir in self.source_dirs: + num_errors += self.lint_source_files_under_source_dir( + source_dir, source_paths) return num_errors