Merge lp:~rackspace-titan/nova/run-tests-pep8-option into lp:~hudson-openstack/nova/trunk

[Naveed Massjouni]

Added an option to run_tests.sh so you can run just pep8. So now you can:
    ./run_tests.sh --just-pep8
or
    ./run_tests.sh -p

[Brian Waldon (bcwaldon)]

Looks good. Could we call the option "--pep8", or "--pep8-only"? That would line up better with "-p".

review: Approve

[Naveed Massjouni (ironcamel)]

--pep8-only is fine with me. I would mostly run it with -p anyway :)

[Devin Carlen (devcamcar)]

--pep8-only seems unnecessarily verbose. suggest --pep8

review: Needs Fixing

[lp:~rackspace-titan/nova/run-tests-pep8-option]

951. By Naveed Massjouni on 2011-04-13

Changed pep8 command line option from --just-pep8 to --pep8.

[Naveed Massjouni (ironcamel)]

> --pep8-only seems unnecessarily verbose. suggest --pep8

Done.

[Devin Carlen (devcamcar)]

Cool, lgtm :)

review: Approve

[Vish Ishaya (vishvananda)]

doesn't this need a few || exit 1 after the run_tests and pep8 line to make sure the exit code gets passed the same way?

as in:

pep8 --repeat --show-pep8 --show-source --exclude=vcsversion.py ${srcfiles} || exit 1

remove line 41

run_tests || exit 1

I think that will preserve the old exit codes.

review: Needs Fixing

[lp:~rackspace-titan/nova/run-tests-pep8-option]

952. By Naveed Massjouni on 2011-04-20

Exit early if tests fail, before pep8 is run.

[Naveed Massjouni (ironcamel)]

Vish, thanks for taking a look at this.

> doesn't this need a few || exit 1 after the run_tests and pep8 line to make
> sure the exit code gets passed the same way?
>
> as in:
>
> pep8 --repeat --show-pep8 --show-source --exclude=vcsversion.py ${srcfiles} ||
> exit 1
>

Adding a || exit 1 would be redundant. Consider this code:

    if [ $just_pep8 -eq 1 ]; then
        run_pep8
        exit
    fi

This will correctly exit with an error status if pep8 failed. I manually tested this just to confirm.

> remove line 41

Line 41 cannot be removed. The point of this is to run just pep8 without running the whole test suite. Originally, the option I added was named --just-pep8, but others felt that was too verbose.

>
> run_tests || exit 1

Good point. Done.

>
> I think that will preserve the old exit codes.

Thanks,
Naveed

[Vish Ishaya (vishvananda)]

ok then lgtm!

review: Approve

=== modified file 'run_tests.sh'
--- run_tests.sh	2011-03-08 20:39:58 +0000
+++ run_tests.sh	2011-04-20 20:29:26 +0000
@@ -7,6 +7,7 @@
   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 "  -f, --force              Force a clean re-build of the virtual environment. Useful when dependencies have been added."
+  echo "  -p, --pep8               Just run pep8"
   echo "  -h, --help               Print this usage message"
   echo ""
   echo "Note: with no options specified, the script will try to run the tests in a virtual environment,"
@@ -21,6 +22,7 @@
     -V|--virtual-env) let always_venv=1; let never_venv=0;;
     -N|--no-virtual-env) let always_venv=0; let never_venv=1;;
     -f|--force) let force=1;;
+    -p|--pep8) let just_pep8=1;;
     *) noseargs="$noseargs $1"
   esac
 }
@@ -32,6 +34,7 @@
 force=0
 noseargs=
 wrapper=""
+just_pep8=0
 
 for arg in "$@"; do
   process_option $arg
@@ -53,6 +56,18 @@
   return $RESULT
 }
 
+function run_pep8 {
+  echo "Running pep8 ..."
+  srcfiles=`find bin -type f ! -name "nova.conf*"`
+  srcfiles+=" nova setup.py plugins/xenserver/xenapi/etc/xapi.d/plugins/glance"
+  pep8 --repeat --show-pep8 --show-source --exclude=vcsversion.py ${srcfiles}
+}
+
+if [ $just_pep8 -eq 1 ]; then
+    run_pep8
+    exit
+fi
+
 NOSETESTS="python run_tests.py $noseargs"
 
 if [ $never_venv -eq 0 ]
@@ -81,11 +96,9 @@
   fi
 fi
 
-if [ -z "$noseargs" ];
-then
-  srcfiles=`find bin -type f ! -name "nova.conf*"`
-  srcfiles+=" nova setup.py plugins/xenserver/xenapi/etc/xapi.d/plugins/glance"
-  run_tests && pep8 --repeat --show-pep8 --show-source --exclude=vcsversion.py ${srcfiles} || exit 1
-else
-  run_tests
+run_tests || exit
+
+# Also run pep8 if no options were provided.
+if [ -z "$noseargs" ]; then
+  run_pep8
 fi