From 0a93b563e975e1815922852c2eacb32caa8faaf2 Mon Sep 17 00:00:00 2001 From: Rick Harris Date: Wed, 13 Feb 2013 23:33:20 +0000 Subject: [PATCH] Harmonize PEP8 checking between tox and run_tests.sh Tox and run_tests.sh were running PEP8 checks against different file-sets. This patch refactors the logic to determine which files to run PEP8 checks on into `tools/run_pep8.sh` where it can be called by both tox and `run_tests.sh`. Additional fixes: Some of our Python XenAPI Dom0 plugins don't end in *.py but should still be checked by PEP8. This patches fixes the hacking.py violations in the files and adds them back to the srcfiles list. Merged tools/unused_imports.sh into tools/run_pep8.sh Change-Id: Id5edd1acb644ab938beffc3473494a179d9d8cda --- .../xenapi/etc/xapi.d/plugins/kernel | 2 +- .../xenapi/etc/xapi.d/plugins/migration | 4 +- .../xenapi/etc/xapi.d/plugins/xenhost | 2 +- run_tests.sh | 40 +------------------ tools/run_pep8.sh | 25 ++++++++++++ tools/unused_imports.sh | 4 -- tox.ini | 13 +----- 7 files changed, 31 insertions(+), 59 deletions(-) create mode 100755 tools/run_pep8.sh delete mode 100755 tools/unused_imports.sh diff --git a/plugins/xenserver/xenapi/etc/xapi.d/plugins/kernel b/plugins/xenserver/xenapi/etc/xapi.d/plugins/kernel index 9ce6902d7de8..32e253cde882 100755 --- a/plugins/xenserver/xenapi/etc/xapi.d/plugins/kernel +++ b/plugins/xenserver/xenapi/etc/xapi.d/plugins/kernel @@ -116,7 +116,7 @@ def _remove_file(filepath): def remove_kernel_ramdisk(session, args): - """Removes kernel and/or ramdisk from dom0's file system""" + """Removes kernel and/or ramdisk from dom0's file system.""" kernel_file = optional(args, 'kernel-file') ramdisk_file = optional(args, 'ramdisk-file') if kernel_file: diff --git a/plugins/xenserver/xenapi/etc/xapi.d/plugins/migration b/plugins/xenserver/xenapi/etc/xapi.d/plugins/migration index b9e9da2e27fb..4b6bf8811bd0 100755 --- a/plugins/xenserver/xenapi/etc/xapi.d/plugins/migration +++ b/plugins/xenserver/xenapi/etc/xapi.d/plugins/migration @@ -25,7 +25,7 @@ configure_logging('migration') def move_vhds_into_sr(session, instance_uuid, sr_path, uuid_stack): - """Moves the VHDs from their copied location to the SR""" + """Moves the VHDs from their copied location to the SR.""" staging_path = "/images/instance%s" % instance_uuid imported_vhds = utils.import_vhds(sr_path, staging_path, uuid_stack) utils.cleanup_staging_area(staging_path) @@ -47,7 +47,7 @@ def _rsync_vhds(instance_uuid, host, staging_path, user="root"): def transfer_vhd(session, instance_uuid, host, vdi_uuid, sr_path, seq_num): - """Rsyncs a VHD to an adjacent host""" + """Rsyncs a VHD to an adjacent host.""" staging_path = utils.make_staging_area(sr_path) try: utils.prepare_staging_area( diff --git a/plugins/xenserver/xenapi/etc/xapi.d/plugins/xenhost b/plugins/xenserver/xenapi/etc/xapi.d/plugins/xenhost index 4d219390860b..0319af4d29ba 100755 --- a/plugins/xenserver/xenapi/etc/xapi.d/plugins/xenhost +++ b/plugins/xenserver/xenapi/etc/xapi.d/plugins/xenhost @@ -33,9 +33,9 @@ import subprocess import tempfile import time +import pluginlib_nova as pluginlib import XenAPI import XenAPIPlugin -import pluginlib_nova as pluginlib pluginlib.configure_logging("xenhost") diff --git a/run_tests.sh b/run_tests.sh index 1f269fbfe166..eafdb26b8632 100755 --- a/run_tests.sh +++ b/run_tests.sh @@ -93,8 +93,6 @@ export venv_name export tools_dir export venv=${venv_path}/${venv_dir} -SCRIPT_ROOT=$(dirname $(readlink -f "$0")) - if [ $no_site_packages -eq 1 ]; then installvenvopts="--no-site-packages" fi @@ -149,43 +147,7 @@ function copy_subunit_log { function run_pep8 { echo "Running PEP8 and HACKING compliance check..." - - # Files of interest - # NOTE(lzyeval): Avoid selecting nova-api-paste.ini and nova.conf in nova/bin - # when running on devstack. - # NOTE(lzyeval): Avoid selecting *.pyc files to reduce pep8 check-up time - # when running on devstack. - srcfiles=`find nova -type f -name "*.py" ! -wholename "nova\/openstack*"` - srcfiles+=" `find bin -type f ! -name "nova.conf*" ! -name "*api-paste.ini*" ! -name "*~"`" - srcfiles+=" `find tools -type f -name "*.py"`" - srcfiles+=" `find smoketests -type f -name "*.py"`" - srcfiles+=" setup.py" - - # Until all these issues get fixed, ignore. - ignore='--ignore=E12,E711,E721,E712,N403,N404,N303' - - # First run the hacking selftest, to make sure it's right - echo "Running hacking.py self test" - ${wrapper} python tools/hacking.py --doctest - - # Then actually run it - echo "Running pep8" - ${wrapper} python tools/hacking.py ${ignore} ${srcfiles} - - PYTHONPATH=$SCRIPT_ROOT/plugins/xenserver/networking/etc/xensource/scripts ${wrapper} python tools/hacking.py ${ignore} ./plugins/xenserver/networking - - PYTHONPATH=$SCRIPT_ROOT/plugins/xenserver/xenapi/etc/xapi.d/plugins ${wrapper} python tools/hacking.py ${ignore} ./plugins/xenserver/xenapi - - ${wrapper} bash tools/unused_imports.sh - # NOTE(sdague): as of grizzly-2 these are passing however leaving the comment - # in here in case we need to break it out when we get more of our hacking working - # again. - # - # NOTE(sirp): Dom0 plugins are written for Python 2.4, meaning some HACKING - # checks are too strict. - # pep8onlyfiles=`find plugins -type f -name "*.py"` - # pep8onlyfiles+=" `find plugins/xenserver/xenapi/etc/xapi.d/plugins/ -type f -perm +111`" - # ${wrapper} pep8 ${ignore} ${pep8onlyfiles} + bash tools/run_pep8.sh } diff --git a/tools/run_pep8.sh b/tools/run_pep8.sh new file mode 100755 index 000000000000..4e7212e089fa --- /dev/null +++ b/tools/run_pep8.sh @@ -0,0 +1,25 @@ +#!/bin/bash +# This is used by run_tests.sh and tox.ini +python tools/hacking.py --doctest + +# Until all these issues get fixed, ignore. +PEP8='python tools/hacking.py --ignore=E12,E711,E721,E712,N303,N403,N404' + +EXCLUDE='--exclude=.venv,.git,.tox,dist,doc,*openstack/common*,*lib/python*' +EXCLUDE+=',*egg,build,./plugins/xenserver/networking/etc/xensource/scripts' +EXCLUDE+=',./plugins/xenserver/xenapi/etc/xapi.d/plugins' +${PEP8} ${EXCLUDE} . + +${PEP8} --filename=nova* bin + +SCRIPT_ROOT=$(echo $(dirname $(readlink -f "$0")) | sed s/\\/tools//) + +SCRIPTS_PATH=${SCRIPT_ROOT}/plugins/xenserver/networking/etc/xensource/scripts +PYTHONPATH=${SCRIPTS_PATH} ${PEP8} ./plugins/xenserver/networking + +# NOTE(sirp): Also check Dom0 plugins w/o .py extension +PLUGINS_PATH=${SCRIPT_ROOT}/plugins/xenserver/xenapi/etc/xapi.d/plugins +PYTHONPATH=${PLUGINS_PATH} ${PEP8} ./plugins/xenserver/xenapi \ + `find plugins/xenserver/xenapi/etc/xapi.d/plugins/ -type f -perm +111` + +! pyflakes nova/ | grep "imported but unused" diff --git a/tools/unused_imports.sh b/tools/unused_imports.sh deleted file mode 100755 index 0e0294517d27..000000000000 --- a/tools/unused_imports.sh +++ /dev/null @@ -1,4 +0,0 @@ -#!/bin/sh - -#snakefood sfood-checker detects even more unused imports -! pyflakes nova/ | grep "imported but unused" diff --git a/tox.ini b/tox.ini index eed3ae5338c4..a34315a7f471 100644 --- a/tox.ini +++ b/tox.ini @@ -18,18 +18,7 @@ downloadcache = ~/cache/pip deps= pep8==1.3.3 pyflakes -commands = - python tools/hacking.py --doctest - python tools/hacking.py --ignore=E12,E711,E721,E712,N403,N404,N303 \ - --show-source \ - --exclude=.venv,.git,.tox,dist,doc,*openstack/common*,*lib/python*,*egg,build,./plugins/xenserver/networking/etc/xensource/scripts,./plugins/xenserver/xenapi/etc/xapi.d/plugins . - python tools/hacking.py --ignore=E12,E711,E721,E712,N403,N404,N303,N304 \ - --show-source \ - ./plugins/xenserver/networking/etc/xensource/scripts \ - ./plugins/xenserver/xenapi/etc/xapi.d/plugins - python tools/hacking.py --ignore=E12,E711,E721,E712,N403,N404,N303 \ - --show-source --filename=nova* bin - bash tools/unused_imports.sh +commands = bash tools/run_pep8.sh [testenv:pylint] setenv = VIRTUAL_ENV={envdir}