You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@mesos.apache.org by kl...@apache.org on 2018/05/09 12:15:19 UTC

[2/2] mesos git commit: Added tox for linting and testing code living uder src/python.

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 <ci...@gmail.com>
Authored: Wed May 9 03:35:28 2018 -0700
Committer: Kevin Klues <kl...@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