Add spack flake8 command. (#2186)
- Ported old run-flake8-tests qa script to `spack flake8` command. - New command does not modify files in the source tree - Copies files to a temp stage modifies them there, and runs tests. - Updated docs and `run-flake8-tests` script to call `spack flake8`.
This commit is contained in:
Todd Gamblin
@@ -83,10 +83,11 @@ adding new unit tests or strengthening existing tests.
|
||||
|
||||
.. note::
|
||||
|
||||
There is also a ``run-unit-tests`` script in ``share/spack/qa`` that runs the
|
||||
unit tests. Afterwards, it reports back to Coverage with the percentage of Spack
|
||||
that is covered by unit tests. This script is designed for Travis CI. If you
|
||||
want to run the unit tests yourself, we suggest you use ``spack test``.
|
||||
There is also a ``run-unit-tests`` script in ``share/spack/qa`` that
|
||||
runs the unit tests. Afterwards, it reports back to Coverage with the
|
||||
percentage of Spack that is covered by unit tests. This script is
|
||||
designed for Travis CI. If you want to run the unit tests yourself, we
|
||||
suggest you use ``spack test``.
|
||||
|
||||
^^^^^^^^^^^^
|
||||
Flake8 Tests
|
||||
@@ -99,24 +100,25 @@ from variable naming to indentation. In order to limit the number of PRs that
|
||||
were mostly style changes, we decided to enforce PEP 8 conformance. Your PR
|
||||
needs to comply with PEP 8 in order to be accepted.
|
||||
|
||||
Testing for PEP 8 compliance is easy. Simply add the quality assurance
|
||||
directory to your ``PATH`` and run the flake8 script:
|
||||
Testing for PEP 8 compliance is easy. Simply run the ``spack flake8``
|
||||
command:
|
||||
|
||||
.. code-block:: console
|
||||
|
||||
$ export PATH+=":$SPACK_ROOT/share/spack/qa"
|
||||
$ run-flake8-tests
|
||||
$ spack flake8
|
||||
|
||||
``run-flake8-tests`` has a couple advantages over running ``flake8`` by hand:
|
||||
``spack flake8`` has a couple advantages over running ``flake8`` by hand:
|
||||
|
||||
#. It only tests files that you have modified since branching off of develop.
|
||||
#. It only tests files that you have modified since branching off of
|
||||
``develop``.
|
||||
|
||||
#. It works regardless of what directory you are in.
|
||||
|
||||
#. It automatically adds approved exemptions from the flake8 checks. For example,
|
||||
URLs are often longer than 80 characters, so we exempt them from the line
|
||||
length checks. We also exempt lines that start with "homepage", "url", "version",
|
||||
"variant", "depends_on", and "extends" in the ``package.py`` files.
|
||||
#. It automatically adds approved exemptions from the ``flake8``
|
||||
checks. For example, URLs are often longer than 80 characters, so we
|
||||
exempt them from line length checks. We also exempt lines that start
|
||||
with "homepage", "url", "version", "variant", "depends_on", and
|
||||
"extends" in ``package.py`` files.
|
||||
|
||||
More approved flake8 exemptions can be found
|
||||
`here <https://github.com/LLNL/spack/blob/develop/.flake8>`_.
|
||||
@@ -518,4 +520,3 @@ and create a PR:
|
||||
.. code-block:: console
|
||||
|
||||
$ git push origin --set-upstream
|
||||
|
||||
|
||||
144
lib/spack/spack/cmd/flake8.py
Executable file
144
lib/spack/spack/cmd/flake8.py
Executable file
@@ -0,0 +1,144 @@
|
||||
##############################################################################
|
||||
# Copyright (c) 2013-2016, Lawrence Livermore National Security, LLC.
|
||||
# Produced at the Lawrence Livermore National Laboratory.
|
||||
#
|
||||
# This file is part of Spack.
|
||||
# Created by Todd Gamblin, tgamblin@llnl.gov, All rights reserved.
|
||||
# LLNL-CODE-647188
|
||||
#
|
||||
# For details, see https://github.com/llnl/spack
|
||||
# Please also see the LICENSE file for our notice and the LGPL.
|
||||
#
|
||||
# This program is free software; you can redistribute it and/or modify
|
||||
# it under the terms of the GNU Lesser General Public License (as
|
||||
# published by the Free Software Foundation) version 2.1, February 1999.
|
||||
#
|
||||
# This program is distributed in the hope that it will be useful, but
|
||||
# WITHOUT ANY WARRANTY; without even the IMPLIED WARRANTY OF
|
||||
# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the terms and
|
||||
# conditions of the GNU Lesser General Public License for more details.
|
||||
#
|
||||
# You should have received a copy of the GNU Lesser General Public
|
||||
# License along with this program; if not, write to the Free Software
|
||||
# Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
|
||||
##############################################################################
|
||||
import re
|
||||
import os
|
||||
import sys
|
||||
import shutil
|
||||
import tempfile
|
||||
|
||||
from llnl.util.filesystem import *
|
||||
|
||||
import spack
|
||||
from spack.util.executable import *
|
||||
|
||||
description = "Runs source code style checks on Spack. Requires flake8."
|
||||
|
||||
changed_files_path = os.path.join(spack.share_path, 'qa', 'changed_files')
|
||||
changed_files = Executable(changed_files_path)
|
||||
flake8 = None
|
||||
|
||||
#
|
||||
# This is a dict that maps:
|
||||
# filename pattern ->
|
||||
# a flake8 exemption code ->
|
||||
# list of patterns, for which matching lines should have codes applied.
|
||||
#
|
||||
exemptions = {
|
||||
# exemptions applied only to package.py files.
|
||||
r'package.py$': {
|
||||
# Exempt lines with urls and descriptions from overlong line errors.
|
||||
501: [r'^\s*homepage\s*=',
|
||||
r'^\s*url\s*=',
|
||||
r'^\s*version\(.*\)',
|
||||
r'^\s*variant\(.*\)',
|
||||
r'^\s*depends_on\(.*\)',
|
||||
r'^\s*extends\(.*\)'],
|
||||
# Exempt '@when' decorated functions from redefinition errors.
|
||||
811: [r'^\s*\@when\(.*\)'],
|
||||
},
|
||||
|
||||
# exemptions applied to all files.
|
||||
r'.py$': {
|
||||
# Exempt lines with URLs from overlong line errors.
|
||||
501: [r'^(https?|file)\:']
|
||||
},
|
||||
}
|
||||
|
||||
# compile all regular expressions.
|
||||
exemptions = dict((re.compile(file_pattern),
|
||||
dict((code, [re.compile(p) for p in patterns])
|
||||
for code, patterns in error_dict.items()))
|
||||
for file_pattern, error_dict in exemptions.items())
|
||||
|
||||
|
||||
def filter_file(source, dest):
|
||||
"""Filter a single file through all the patterns in exemptions."""
|
||||
for file_pattern, errors in exemptions.items():
|
||||
if not file_pattern.search(source):
|
||||
continue
|
||||
|
||||
with open(source) as infile:
|
||||
parent = os.path.dirname(dest)
|
||||
mkdirp(parent)
|
||||
|
||||
with open(dest, 'w') as outfile:
|
||||
for line in infile:
|
||||
line = line.rstrip()
|
||||
for code, patterns in errors.items():
|
||||
for pattern in patterns:
|
||||
if pattern.search(line):
|
||||
line += (" # NOQA: ignore=%d" % code)
|
||||
break
|
||||
outfile.write(line + '\n')
|
||||
|
||||
|
||||
def setup_parser(subparser):
|
||||
subparser.add_argument(
|
||||
'-k', '--keep-temp', action='store_true',
|
||||
help="Do not delete temporary directory where flake8 runs. "
|
||||
"Use for debugging, to see filtered files.")
|
||||
|
||||
|
||||
def flake8(parser, args):
|
||||
# Just use this to check for flake8 -- we actually execute it with Popen.
|
||||
global flake8
|
||||
flake8 = which('flake8', required=True)
|
||||
|
||||
temp = tempfile.mkdtemp()
|
||||
try:
|
||||
with working_dir(spack.prefix):
|
||||
changed = changed_files('*.py', output=str)
|
||||
changed = [x for x in changed.split('\n') if x]
|
||||
shutil.copy('.flake8', os.path.join(temp, '.flake8'))
|
||||
|
||||
print '======================================================='
|
||||
print 'flake8: running flake8 code checks on spack.'
|
||||
print
|
||||
print 'Modified files:'
|
||||
for filename in changed:
|
||||
print " %s" % filename.strip()
|
||||
print('=======================================================')
|
||||
|
||||
# filter files into a temporary directory with exemptions added.
|
||||
for filename in changed:
|
||||
src_path = os.path.join(spack.prefix, filename)
|
||||
dest_path = os.path.join(temp, filename)
|
||||
filter_file(src_path, dest_path)
|
||||
|
||||
# run flake8 on the temporary tree.
|
||||
with working_dir(temp):
|
||||
flake8('--format', 'pylint', *changed, fail_on_error=False)
|
||||
|
||||
if flake8.returncode != 0:
|
||||
print "Flake8 found errors."
|
||||
sys.exit(1)
|
||||
else:
|
||||
print "Flake8 checks were clean."
|
||||
|
||||
finally:
|
||||
if args.keep_temp:
|
||||
print "temporary files are in ", temp
|
||||
else:
|
||||
shutil.rmtree(temp, ignore_errors=True)
|
||||
@@ -23,67 +23,7 @@ deps=(
|
||||
# Check for dependencies
|
||||
"$QA_DIR/check_dependencies" "${deps[@]}" || exit 1
|
||||
|
||||
# Gather array of changed files
|
||||
changed=($("$QA_DIR/changed_files" "*.py"))
|
||||
# Add Spack to the PATH.
|
||||
export PATH="$SPACK_ROOT/bin:$PATH"
|
||||
|
||||
# Exit if no Python files were modified
|
||||
if [[ ! "${changed[@]}" ]]; then
|
||||
echo "No Python files were modified."
|
||||
exit 0
|
||||
fi
|
||||
|
||||
# Move to root directory of Spack
|
||||
# Allows script to be run from anywhere
|
||||
cd "$SPACK_ROOT"
|
||||
|
||||
function cleanup {
|
||||
# Restore original package files after modifying them.
|
||||
for file in "${changed[@]}"; do
|
||||
if [[ -e "${file}.sbak~" ]]; then
|
||||
mv "${file}.sbak~" "${file}"
|
||||
fi
|
||||
done
|
||||
}
|
||||
|
||||
# Cleanup temporary files upon exit or when script is killed
|
||||
trap cleanup EXIT SIGINT SIGTERM
|
||||
|
||||
# Add approved style exemptions to the changed packages.
|
||||
for file in "${changed[@]}"; do
|
||||
# Make a backup to restore later
|
||||
cp "$file" "$file.sbak~"
|
||||
|
||||
#
|
||||
# Exemptions for package.py files
|
||||
#
|
||||
if [[ $file = *package.py ]]; then
|
||||
# Exempt lines with urls and descriptions from overlong line errors.
|
||||
perl -i -pe 's/^(\s*homepage\s*=.*)$/\1 # NOQA: ignore=E501/' "$file"
|
||||
perl -i -pe 's/^(\s*url\s*=.*)$/\1 # NOQA: ignore=E501/' "$file"
|
||||
perl -i -pe 's/^(\s*version\(.*\).*)$/\1 # NOQA: ignore=E501/' "$file"
|
||||
perl -i -pe 's/^(\s*variant\(.*\).*)$/\1 # NOQA: ignore=E501/' "$file"
|
||||
perl -i -pe 's/^(\s*depends_on\(.*\).*)$/\1 # NOQA: ignore=E501/' "$file"
|
||||
perl -i -pe 's/^(\s*extends\(.*\).*)$/\1 # NOQA: ignore=E501/' "$file"
|
||||
|
||||
# Exempt '@when' decorated functions from redefinition errors.
|
||||
perl -i -pe 's/^(\s*\@when\(.*\).*)$/\1 # NOQA: ignore=F811/' "$file"
|
||||
fi
|
||||
|
||||
#
|
||||
# Exemptions for all files
|
||||
#
|
||||
perl -i -pe 's/^(.*(https?|file)\:.*)$/\1 # NOQA: ignore=E501/' $file
|
||||
done
|
||||
|
||||
echo =======================================================
|
||||
echo flake8: running flake8 code checks on spack.
|
||||
echo
|
||||
echo Modified files:
|
||||
echo "${changed[@]}" | perl -pe 's/^/ /;s/ +/\n /g'
|
||||
echo =======================================================
|
||||
if flake8 --format pylint "${changed[@]}"; then
|
||||
echo "Flake8 checks were clean."
|
||||
else
|
||||
echo "Flake8 found errors."
|
||||
exit 1
|
||||
fi
|
||||
exec spack flake8
|
||||
|
||||
Reference in New Issue
Block a user