From a14439549f15ad48bb9bc4465a04b87b6fd14826 Mon Sep 17 00:00:00 2001 From: Victor Morales Date: Tue, 31 Jan 2017 16:52:42 -0600 Subject: [PATCH] Add bashate support Bashate is a style checker program for bash scripts. This addition improves the quality of the current bash scripts and ensures that any future change will follow the same standards. Change-Id: Ia346f77632d4ac7beb288fa3aacea221d7969c87 --- .../agent/xenapi/contrib/build-rpm.sh | 4 +- neutron/tests/contrib/gate_hook.sh | 2 +- neutron/tests/contrib/post_test_hook.sh | 3 +- run_tests.sh | 285 +++++++++--------- test-requirements.txt | 1 + tools/abandon_old_reviews.sh | 6 +- tools/coding-checks.sh | 42 +-- tools/configure_for_func_testing.sh | 10 +- tools/deploy_rootwrap.sh | 4 +- tools/misc-sanity-checks.sh | 4 +- tools/ostestr_compat_shim.sh | 6 +- tools/split.sh | 4 +- tox.ini | 12 + 13 files changed, 197 insertions(+), 186 deletions(-) diff --git a/neutron/plugins/ml2/drivers/openvswitch/agent/xenapi/contrib/build-rpm.sh b/neutron/plugins/ml2/drivers/openvswitch/agent/xenapi/contrib/build-rpm.sh index 750bdde2b0a..c3455a1393b 100755 --- a/neutron/plugins/ml2/drivers/openvswitch/agent/xenapi/contrib/build-rpm.sh +++ b/neutron/plugins/ml2/drivers/openvswitch/agent/xenapi/contrib/build-rpm.sh @@ -8,8 +8,8 @@ export PYTHONPATH=$NEUTRON_ROOT cd $NEUTRON_ROOT VERSION=$(sh -c "(cat $NEUTRON_ROOT/neutron/version.py; \ - echo 'print version_info.release_string()') | \ - python") + echo 'print version_info.release_string()') | \ + python") cd - PACKAGE=openstack-neutron-xen-plugins diff --git a/neutron/tests/contrib/gate_hook.sh b/neutron/tests/contrib/gate_hook.sh index 1911ec0f729..5ddfd596158 100644 --- a/neutron/tests/contrib/gate_hook.sh +++ b/neutron/tests/contrib/gate_hook.sh @@ -39,7 +39,7 @@ ${config} # Tweak gate configuration for our rally scenarios function load_rc_for_rally { for file in $(ls $RALLY_EXTRA_DIR/*.setup); do - local config=$(cat $file) + config=$(cat $file) export DEVSTACK_LOCAL_CONFIG+=" # generated from hook '$file' ${config} diff --git a/neutron/tests/contrib/post_test_hook.sh b/neutron/tests/contrib/post_test_hook.sh index 8e3fe7a842f..fd878f7e7c5 100644 --- a/neutron/tests/contrib/post_test_hook.sh +++ b/neutron/tests/contrib/post_test_hook.sh @@ -20,8 +20,7 @@ function generate_testr_results { fi } -if [[ "$venv" == dsvm-functional* ]] || [[ "$venv" == dsvm-fullstack* ]] -then +if [[ "$venv" == dsvm-functional* ]] || [[ "$venv" == dsvm-fullstack* ]]; then owner=stack sudo_env= diff --git a/run_tests.sh b/run_tests.sh index 397dc183bb3..a2d29292a2d 100755 --- a/run_tests.sh +++ b/run_tests.sh @@ -3,71 +3,71 @@ set -eu function usage { - echo "NOTE: This script is deprecated." - echo "Usage: $0 [OPTION]..." - echo "Run Neutron's test suite(s)" - echo "" - echo " -V, --virtual-env Always use virtualenv. Install automatically if not present" - echo " -N, --no-virtual-env Don't use virtualenv. Run tests in local environment" - echo " -s, --no-site-packages Isolate the virtualenv from the global Python environment" - echo " -r, --recreate-db Recreate the test database (deprecated, as this is now the default)." - echo " -n, --no-recreate-db Don't recreate the test database." - echo " -f, --force Force a clean re-build of the virtual environment. Useful when dependencies have been added." - echo " -u, --update Update the virtual environment with any newer package versions" - echo " -p, --pep8 Just run PEP8 and HACKING compliance check" - echo " -8, --pep8-only-changed []" - echo " Just run PEP8 and HACKING compliance check on files changed since HEAD~1 (or )" - echo " -P, --no-pep8 Don't run static code checks" - echo " -c, --coverage Generate coverage report" - echo " -d, --debug Run tests with testtools instead of testr. This allows you to use the debugger." - echo " -h, --help Print this usage message" - echo " --virtual-env-path Location of the virtualenv directory" - echo " Default: \$(pwd)" - echo " --virtual-env-name Name of the virtualenv directory" - echo " Default: .venv" - echo " --tools-path Location of the tools directory" - echo " Default: \$(pwd)" - echo "" - echo "Note: with no options specified, the script will try to run the tests in a virtual environment," - echo " If no virtualenv is found, the script will ask if you would like to create one. If you " - echo " prefer to run tests NOT in a virtual environment, simply pass the -N option." - exit + echo "NOTE: This script is deprecated." + echo "Usage: $0 [OPTION]..." + echo "Run Neutron's test suite(s)" + echo "" + echo " -V, --virtual-env Always use virtualenv. Install automatically if not present" + echo " -N, --no-virtual-env Don't use virtualenv. Run tests in local environment" + echo " -s, --no-site-packages Isolate the virtualenv from the global Python environment" + echo " -r, --recreate-db Recreate the test database (deprecated, as this is now the default)." + echo " -n, --no-recreate-db Don't recreate the test database." + echo " -f, --force Force a clean re-build of the virtual environment. Useful when dependencies have been added." + echo " -u, --update Update the virtual environment with any newer package versions" + echo " -p, --pep8 Just run PEP8 and HACKING compliance check" + echo " -8, --pep8-only-changed []" + echo " Just run PEP8 and HACKING compliance check on files changed since HEAD~1 (or )" + echo " -P, --no-pep8 Don't run static code checks" + echo " -c, --coverage Generate coverage report" + echo " -d, --debug Run tests with testtools instead of testr. This allows you to use the debugger." + echo " -h, --help Print this usage message" + echo " --virtual-env-path Location of the virtualenv directory" + echo " Default: \$(pwd)" + echo " --virtual-env-name Name of the virtualenv directory" + echo " Default: .venv" + echo " --tools-path Location of the tools directory" + echo " Default: \$(pwd)" + echo "" + echo "Note: with no options specified, the script will try to run the tests in a virtual environment," + echo " If no virtualenv is found, the script will ask if you would like to create one. If you " + echo " prefer to run tests NOT in a virtual environment, simply pass the -N option." + exit } function process_options { - i=1 - while [ $i -le $# ]; do + i=1 + while [ $i -le $# ]; do case "${!i}" in - -h|--help) usage;; - -V|--virtual-env) always_venv=1; never_venv=0;; - -N|--no-virtual-env) always_venv=0; never_venv=1;; - -s|--no-site-packages) no_site_packages=1;; - -r|--recreate-db) recreate_db=1;; - -n|--no-recreate-db) recreate_db=0;; - -f|--force) force=1;; - -u|--update) update=1;; - -p|--pep8) just_pep8=1;; - -8|--pep8-only-changed) just_pep8_changed=1;; - -P|--no-pep8) no_pep8=1;; - -c|--coverage) coverage=1;; - -d|--debug) debug=1;; - --virtual-env-path) - (( i++ )) + -h|--help) usage;; + -V|--virtual-env) always_venv=1; never_venv=0;; + -N|--no-virtual-env) always_venv=0; never_venv=1;; + -s|--no-site-packages) no_site_packages=1;; + -r|--recreate-db) recreate_db=1;; + -n|--no-recreate-db) recreate_db=0;; + -f|--force) force=1;; + -u|--update) update=1;; + -p|--pep8) just_pep8=1;; + -8|--pep8-only-changed) just_pep8_changed=1;; + -P|--no-pep8) no_pep8=1;; + -c|--coverage) coverage=1;; + -d|--debug) debug=1;; + --virtual-env-path) + $(( i++ )) venv_path=${!i} ;; - --virtual-env-name) - (( i++ )) + --virtual-env-name) + $(( i++ )) venv_dir=${!i} ;; - --tools-path) - (( i++ )) + --tools-path) + $(( i++ )) tools_path=${!i} ;; - -*) testopts="$testopts ${!i}";; - *) testargs="$testargs ${!i}" + -*) testopts="$testopts ${!i}";; + *) testargs="$testargs ${!i}" esac - (( i++ )) - done + $(( i++ )) + done } tool_path=${tools_path:-$(pwd)} @@ -107,84 +107,84 @@ export tools_dir export venv=${venv_path}/${venv_dir} if [ $no_site_packages -eq 1 ]; then - installvenvopts="--no-site-packages" + installvenvopts="--no-site-packages" fi function run_tests { - # Cleanup *pyc - ${wrapper} find . -type f -name "*.pyc" -delete + # Cleanup *pyc + ${wrapper} find . -type f -name "*.pyc" -delete - if [ $debug -eq 1 ]; then - if [ "$testopts" = "" ] && [ "$testargs" = "" ]; then - # Default to running all tests if specific test is not - # provided. - testargs="discover ./neutron/tests" + if [ $debug -eq 1 ]; then + if [ "$testopts" = "" ] && [ "$testargs" = "" ]; then + # Default to running all tests if specific test is not + # provided. + testargs="discover ./neutron/tests" + fi + ${wrapper} python -m testtools.run $testopts $testargs + + # Short circuit because all of the testr and coverage stuff + # below does not make sense when running testtools.run for + # debugging purposes. + return $? fi - ${wrapper} python -m testtools.run $testopts $testargs - # Short circuit because all of the testr and coverage stuff - # below does not make sense when running testtools.run for - # debugging purposes. - return $? - fi + if [ $coverage -eq 1 ]; then + TESTRTESTS="$TESTRTESTS --coverage" + else + TESTRTESTS="$TESTRTESTS --slowest" + fi - if [ $coverage -eq 1 ]; then - TESTRTESTS="$TESTRTESTS --coverage" - else - TESTRTESTS="$TESTRTESTS --slowest" - fi + # Just run the test suites in current environment + set +e + testargs=`echo "$testargs" | sed -e's/^\s*\(.*\)\s*$/\1/'` + TESTRTESTS="$TESTRTESTS --testr-args='--subunit $testopts $testargs'" + OS_TEST_PATH=`echo $testargs|grep -o 'neutron\.tests[^[:space:]:]\+'|tr . /` + if [ -n "$OS_TEST_PATH" ]; then + os_test_dir=$(dirname "$OS_TEST_PATH") + else + os_test_dir='' + fi + if [ -d "$OS_TEST_PATH" ]; then + wrapper="OS_TEST_PATH=$OS_TEST_PATH $wrapper" + elif [ -d "$os_test_dir" ]; then + wrapper="OS_TEST_PATH=$os_test_dir $wrapper" + fi + echo "Running \`${wrapper} $TESTRTESTS\`" + bash -c "${wrapper} $TESTRTESTS | ${wrapper} subunit2pyunit" + RESULT=$? + set -e - # Just run the test suites in current environment - set +e - testargs=`echo "$testargs" | sed -e's/^\s*\(.*\)\s*$/\1/'` - TESTRTESTS="$TESTRTESTS --testr-args='--subunit $testopts $testargs'" - OS_TEST_PATH=`echo $testargs|grep -o 'neutron\.tests[^[:space:]:]\+'|tr . /` - if [ -n "$OS_TEST_PATH" ]; then - os_test_dir=$(dirname "$OS_TEST_PATH") - else - os_test_dir='' - fi - if [ -d "$OS_TEST_PATH" ]; then - wrapper="OS_TEST_PATH=$OS_TEST_PATH $wrapper" - elif [ -d "$os_test_dir" ]; then - wrapper="OS_TEST_PATH=$os_test_dir $wrapper" - fi - echo "Running \`${wrapper} $TESTRTESTS\`" - bash -c "${wrapper} $TESTRTESTS | ${wrapper} subunit2pyunit" - RESULT=$? - set -e + copy_subunit_log - copy_subunit_log + if [ $coverage -eq 1 ]; then + echo "Generating coverage report in covhtml/" + # Don't compute coverage for common code, which is tested elsewhere + ${wrapper} coverage combine + ${wrapper} coverage html --include='neutron/*' -d covhtml -i + fi - if [ $coverage -eq 1 ]; then - echo "Generating coverage report in covhtml/" - # Don't compute coverage for common code, which is tested elsewhere - ${wrapper} coverage combine - ${wrapper} coverage html --include='neutron/*' -d covhtml -i - fi - - return $RESULT + return $RESULT } function copy_subunit_log { - LOGNAME=`cat .testrepository/next-stream` - LOGNAME=$(($LOGNAME - 1)) - LOGNAME=".testrepository/${LOGNAME}" - cp $LOGNAME subunit.log + LOGNAME=`cat .testrepository/next-stream` + LOGNAME=$(($LOGNAME - 1)) + LOGNAME=".testrepository/${LOGNAME}" + cp $LOGNAME subunit.log } function warn_on_flake8_without_venv { - if [ $never_venv -eq 1 ]; then - echo "**WARNING**:" - echo "Running flake8 without virtual env may miss OpenStack HACKING detection" - fi + if [ $never_venv -eq 1 ]; then + echo "**WARNING**:" + echo "Running flake8 without virtual env may miss OpenStack HACKING detection" + fi } function run_pep8 { - echo "Running flake8 ..." - warn_on_flake8_without_venv - ${wrapper} flake8 + echo "Running flake8 ..." + warn_on_flake8_without_venv + ${wrapper} flake8 } function run_pep8_changed { @@ -194,7 +194,7 @@ function run_pep8_changed { # --diff argument behaves surprisingly as well, because although you feed it a # diff, it actually checks the file on disk anyway. local target=${testargs:-HEAD~1} - local files=$(git diff --name-only $target | tr '\n' ' ') + files=$(git diff --name-only $target | tr '\n' ' ') echo "Running flake8 on ${files}" warn_on_flake8_without_venv diff -u --from-file /dev/null ${files} | ${wrapper} flake8 --diff @@ -203,34 +203,33 @@ function run_pep8_changed { TESTRTESTS="python setup.py testr" -if [ $never_venv -eq 0 ] -then - # Remove the virtual environment if --force used - if [ $force -eq 1 ]; then - echo "Cleaning virtualenv..." - rm -rf ${venv} - fi - if [ $update -eq 1 ]; then - echo "Updating virtualenv..." - python tools/install_venv.py $installvenvopts - fi - if [ -e ${venv} ]; then - wrapper="${with_venv}" - else - if [ $always_venv -eq 1 ]; then - # Automatically install the virtualenv - python tools/install_venv.py $installvenvopts - wrapper="${with_venv}" - else - echo -e "No virtual environment found...create one? (Y/n) \c" - read use_ve - if [ "x$use_ve" = "xY" -o "x$use_ve" = "x" -o "x$use_ve" = "xy" ]; then - # Install the virtualenv and run the test suite in it - python tools/install_venv.py $installvenvopts - wrapper=${with_venv} - fi +if [ $never_venv -eq 0 ]; then + # Remove the virtual environment if --force used + if [ $force -eq 1 ]; then + echo "Cleaning virtualenv..." + rm -rf ${venv} + fi + if [ $update -eq 1 ]; then + echo "Updating virtualenv..." + python tools/install_venv.py $installvenvopts + fi + if [ -e ${venv} ]; then + wrapper="${with_venv}" + else + if [ $always_venv -eq 1 ]; then + # Automatically install the virtualenv + python tools/install_venv.py $installvenvopts + wrapper="${with_venv}" + else + echo -e "No virtual environment found...create one? (Y/n) \c" + read use_ve + if [ "x$use_ve" = "xY" -o "x$use_ve" = "x" -o "x$use_ve" = "xy" ]; then + # Install the virtualenv and run the test suite in it + python tools/install_venv.py $installvenvopts + wrapper=${with_venv} + fi + fi fi - fi fi # Delete old coverage data from previous runs @@ -259,7 +258,7 @@ run_tests # distinguish between options (testopts), which begin with a '-', and # arguments (testargs). if [ -z "$testargs" ]; then - if [ $no_pep8 -eq 0 ]; then - run_pep8 - fi + if [ $no_pep8 -eq 0 ]; then + run_pep8 + fi fi diff --git a/test-requirements.txt b/test-requirements.txt index 0904c959fe5..f8c181c38c2 100644 --- a/test-requirements.txt +++ b/test-requirements.txt @@ -22,3 +22,4 @@ reno>=1.8.0 # Apache-2.0 # Needed to run DB commands in virtualenvs PyMySQL>=0.7.6 # MIT License tempest>=14.0.0 # Apache-2.0 +bashate>=0.2 # Apache-2.0 diff --git a/tools/abandon_old_reviews.sh b/tools/abandon_old_reviews.sh index 3d129a99233..dfed108b169 100755 --- a/tools/abandon_old_reviews.sh +++ b/tools/abandon_old_reviews.sh @@ -43,10 +43,10 @@ function abandon_review { local msg=$@ # echo ssh review.openstack.org gerrit review $gitid --abandon --message \"$msg\" if [ $DRY_RUN -eq 1 ]; then - echo "Would abandon $gitid" + echo "Would abandon $gitid" else - echo "Abandoning $gitid" - ssh review.openstack.org gerrit review $gitid --abandon --message \"$msg\" + echo "Abandoning $gitid" + ssh review.openstack.org gerrit review $gitid --abandon --message \"$msg\" fi } diff --git a/tools/coding-checks.sh b/tools/coding-checks.sh index 287b205e96d..d852f98fe18 100755 --- a/tools/coding-checks.sh +++ b/tools/coding-checks.sh @@ -3,26 +3,26 @@ set -eu usage () { - echo "Usage: $0 [OPTION]..." - echo "Run Neutron's coding check(s)" - echo "" - echo " -Y, --pylint [] Run pylint check on the entire neutron module or just files changed in basecommit (e.g. HEAD~1)" - echo " -h, --help Print this usage message" - echo - exit 0 + echo "Usage: $0 [OPTION]..." + echo "Run Neutron's coding check(s)" + echo "" + echo " -Y, --pylint [] Run pylint check on the entire neutron module or just files changed in basecommit (e.g. HEAD~1)" + echo " -h, --help Print this usage message" + echo + exit 0 } process_options () { - i=1 - while [ $i -le $# ]; do - eval opt=\$$i - case $opt in - -h|--help) usage;; - -Y|--pylint) pylint=1;; - *) scriptargs="$scriptargs $opt" - esac - i=$((i+1)) - done + i=1 + while [ $i -le $# ]; do + eval opt=\$$i + case $opt in + -h|--help) usage;; + -Y|--pylint) pylint=1;; + *) scriptargs="$scriptargs $opt" + esac + i=$((i+1)) + done } run_pylint () { @@ -31,10 +31,10 @@ run_pylint () { if [ "$target" = "all" ]; then files="neutron" else - case "$target" in - *HEAD~[0-9]*) files=$(git diff --diff-filter=AM --name-only $target -- "*.py");; - *) echo "$target is an unrecognized basecommit"; exit 1;; - esac + case "$target" in + *HEAD~[0-9]*) files=$(git diff --diff-filter=AM --name-only $target -- "*.py");; + *) echo "$target is an unrecognized basecommit"; exit 1;; + esac fi echo "Running pylint..." diff --git a/tools/configure_for_func_testing.sh b/tools/configure_for_func_testing.sh index 4f76978a6dd..10d80d275ae 100755 --- a/tools/configure_for_func_testing.sh +++ b/tools/configure_for_func_testing.sh @@ -26,9 +26,9 @@ if [[ "$IS_GATE" != "True" ]] && [[ "$#" -lt 1 ]]; then >&2 echo "Usage: $0 /path/to/devstack [-i] Configure a host to run Neutron's functional test suite. --i Install Neutron's package dependencies. By default, it is assumed - that devstack has already been used to deploy neutron to the - target host and that package dependencies need not be installed. +-i Install Neutron's package dependencies. By default, it is assumed + that devstack has already been used to deploy neutron to the + target host and that package dependencies need not be installed. Warning: This script relies on devstack to perform extensive modification to the underlying host. It is recommended that it be @@ -60,8 +60,8 @@ INSTALL_BASE_DEPENDENCIES=${INSTALL_BASE_DEPENDENCIES:-$IS_GATE} if [ ! -f "$DEVSTACK_PATH/stack.sh" ]; then - >&2 echo "Unable to find devstack at '$DEVSTACK_PATH'. Please verify that the specified path points to a valid devstack repo." - exit 1 + >&2 echo "Unable to find devstack at '$DEVSTACK_PATH'. Please verify that the specified path points to a valid devstack repo." + exit 1 fi diff --git a/tools/deploy_rootwrap.sh b/tools/deploy_rootwrap.sh index 27d9a36f643..9edbf1493eb 100755 --- a/tools/deploy_rootwrap.sh +++ b/tools/deploy_rootwrap.sh @@ -15,7 +15,7 @@ set -eu if [ "$#" -ne 3 ]; then - >&2 echo "Usage: $0 /path/to/neutron /path/to/target/etc /path/to/target/bin + >&2 echo "Usage: $0 /path/to/neutron /path/to/target/etc /path/to/target/bin Deploy Neutron's rootwrap configuration. Warning: Any existing rootwrap files at the specified etc path will be @@ -23,7 +23,7 @@ removed by this script. Optional: set OS_SUDO_TESTING=1 to deploy the filters required by Neutron's functional testing suite." - exit 1 + exit 1 fi OS_SUDO_TESTING=${OS_SUDO_TESTING:-0} diff --git a/tools/misc-sanity-checks.sh b/tools/misc-sanity-checks.sh index 8f8ebc670f7..a62fd544e9c 100755 --- a/tools/misc-sanity-checks.sh +++ b/tools/misc-sanity-checks.sh @@ -38,8 +38,8 @@ check_pot_files_errors () { # obsolete entries duplicate normal entries. Prevent obsolete # entries to slip in find neutron -type f -regex '.*\.pot?' \ - -print0|xargs -0 -n 1 msgfmt --check-format \ - -o /dev/null + -print0|xargs -0 -n 1 msgfmt --check-format \ + -o /dev/null if [ "$?" -ne 0 ]; then echo "PO files syntax is not correct!" >>$FAILURES fi diff --git a/tools/ostestr_compat_shim.sh b/tools/ostestr_compat_shim.sh index a483ed1a1ef..195cbd248b3 100755 --- a/tools/ostestr_compat_shim.sh +++ b/tools/ostestr_compat_shim.sh @@ -2,7 +2,7 @@ # preserve old behavior of using an arg as a regex when '--' is not present case $@ in - (*--*) ostestr $@;; - ('') ostestr;; - (*) ostestr --regex "$@" + (*--*) ostestr $@;; + ('') ostestr;; + (*) ostestr --regex "$@" esac diff --git a/tools/split.sh b/tools/split.sh index 995d937a742..9ce28c7ab27 100755 --- a/tools/split.sh +++ b/tools/split.sh @@ -24,8 +24,8 @@ set -e if [ $# -lt 2 ]; then - echo "Usage $0 " - exit 1 + echo "Usage $0 " + exit 1 fi set -x diff --git a/tox.ini b/tox.ini index 5137a68360b..86978748df5 100644 --- a/tox.ini +++ b/tox.ini @@ -94,6 +94,7 @@ commands= neutron-db-manage --config-file neutron/tests/etc/neutron.conf check_migration python ./tools/list_moved_globals.py {[testenv:genconfig]commands} + {[testenv:bashate]commands} whitelist_externals = sh bash @@ -128,6 +129,17 @@ exclude = ./.*,build,dist import_exceptions = neutron._i18n local-check-factory = neutron.hacking.checks.factory +[testenv:bashate] +commands = bash -c "find {toxinidir} \ + -not \( -type d -name .tox\* -prune \) \ + -not \( -type d -name .venv\* -prune \) \ + -type f \ + -name \*.sh \ +# E005 file does not begin with #! or have a .sh prefix +# E006 check for lines longer than 79 columns +# E042 local declaration hides errors + -print0 | xargs -0 bashate -v -iE006 -eE005,E042" + [testenv:genconfig] commands = {toxinidir}/tools/generate_config_file_samples.sh