[chirp_devel] [PATCH 0 of 2] Style checks and consistent tests
This set adds two things:
1. A strict(er) set of style checks for a given patch. This will enforce <80 column lines for all files touched in a patch. This will avoid failing on everything right now, but any patch that touches a file will need to fix all the lines. This is a nice ideal, but I'm not sure how other folks feel about it. This check also verifies the bug number in the commit message, etc. I'd like this to be a "does this meet style requirements" hook for people to run before submitting a patch.
2. A top-level test function that runs everything we have, to maybe help avoid what happened recently, where the unit tests passed, but the driver tests did not.
If folks like this, I'll make jenkins run all of this on each build.
# HG changeset patch # User Dan Smith dsmith@danplanet.com # Date 1386019635 28800 # Mon Dec 02 13:27:15 2013 -0800 # Node ID e6c6ffd8cae979cbeafa14c5fdfbb80d22333ecd # Parent c1271344c63cbba1919fa290b857a33e1bb1bae5 Add tools/checkpatch.sh for style compliance checking
This adds tools/checkpatch.sh which checks a given HG revision for style compliance. We should call this as part of our tests, have Jenkins call it, and keep it full of things we'd like to enforce. Developers should run this on their patches (when possible) before submitting.
Had this idea after the patch for #1277
diff -r c1271344c63c -r e6c6ffd8cae9 tools/checkpatch.sh --- /dev/null Thu Jan 01 00:00:00 1970 +0000 +++ b/tools/checkpatch.sh Mon Dec 02 13:27:15 2013 -0800 @@ -0,0 +1,69 @@ +#!/bin/bash +# +# CHIRP coding standards compliance script +# +# To add a test to this file, create a new check_foo() function +# and then add it to the list of TESTS= below +# + +TESTS="check_long_lines check_bug_number check_commit_message_line_length" + +function check_long_lines() { + local rev="$1" + local files="$2" + + if [ -z "$files" ]; then + return + fi + pep8 --select=E501 $files || \ + error "Please use <80 columns in source files" +} + +function check_bug_number() { + local rev="$1" + hg log -vr $rev | grep -qE '#[0-9]+' || \ + error "A bug number is required like #123" + +} + +function _less_than_80() { + while true; do + read line + if [ -z "$line" ]; then + break + elif [ $(echo $line | wc -c) -ge 80 ]; then + return 1 + fi + done +} + +function check_commit_message_line_length() { + local rev="$1" + hg log -vr $rev | (_less_than_80) || \ + error "Please keep commit message lines to <80 columns" +} + +# --- END OF TEST FUNCTIONS --- + +function error() { + echo FAIL: $* + ERROR=1 +} + +function get_touched_files() { + local rev="$1" + hg status -n --change $rev | grep '.py$' +} + +files=$(get_touched_files tip) +rev=${1:-tip} + +for testname in $TESTS; do + eval "$testname $rev "$files"" +done + +if [ -z "$ERROR" ]; then + echo "Patch '${rev}' is OK" +else + exit 1 +fi
# HG changeset patch # User Dan Smith dsmith@danplanet.com # Date 1386020810 28800 # Mon Dec 02 13:46:50 2013 -0800 # Node ID a9d8238eea0d9306c210eceeb292e5e180cfc434 # Parent e6c6ffd8cae979cbeafa14c5fdfbb80d22333ecd Add top-level test script
This runs our unit tests, driver tests, and style checks.
Had this idea after the patch for #1277
diff -r e6c6ffd8cae9 -r a9d8238eea0d run_all_tests.sh --- /dev/null Thu Jan 01 00:00:00 1970 +0000 +++ b/run_all_tests.sh Mon Dec 02 13:46:50 2013 -0800 @@ -0,0 +1,38 @@ +#!/bin/bash + +function record_failure() { + FAILED="$1 $FAILED" +} + +function unit_tests() { + NOSE=$(which nosetests) + if [ -x "$NOSE" ]; then + $NOSE -v tests/unit + else + echo "NOTE: nosetests required for unit tests!" + record_failure unit_tests + fi +} + +function driver_tests() { + (cd tests && ./run_tests) +} + +function style_tests() { + ./tools/checkpatch.sh +} + +TESTS="unit_tests driver_tests style_tests" +for testname in $TESTS; do + eval "$testname" || record_failure "$testname" +done + +echo "================================================" + +if [ -z "$FAILED" ]; then + echo Tests OK +else + failed=$(echo $FAILED | sed 's/ /, /g' | sed 's/_/ /g') + echo Tests FAILED: $failed + exit 1 +fi
On Mon, Dec 2, 2013 at 1:47 PM, Dan Smith dsmith@danplanet.com wrote:
This set adds two things:
- A strict(er) set of style checks for a given patch. This will enforce
<80 column lines for all files touched in a patch. This will avoid failing on everything right now, but any patch that touches a file will need to fix all the lines. This is a nice ideal, but I'm not sure how other folks feel about it. This check also verifies the bug number in the commit message, etc. I'd like this to be a "does this meet style requirements" hook for people to run before submitting a patch.
- A top-level test function that runs everything we have, to maybe help
avoid what happened recently, where the unit tests passed, but the driver tests did not.
If folks like this, I'll make jenkins run all of this on each build.
This makes sense to me, I don't see a problem with using it.
mike
+1
73 de IZ3GME Marco
On 03/12/2013 15:05, Michael Pittaro wrote:
This makes sense to me, I don't see a problem with using it.
mike _______________________________________________ chirp_devel mailing list chirp_devel@intrepid.danplanet.com http://intrepid.danplanet.com/mailman/listinfo/chirp_devel Developer docs: http://chirp.danplanet.com/projects/chirp/wiki/Developers
participants (3)
-
Dan Smith -
IZ3GME Marco -
Michael Pittaro