views:

391

answers:

2

I've recently started using git, and also begun unit testing (using Python's unittest module). I'd like to run my tests each time I commit, and only commit if they pass.

I'm guessing I need to use pre-commit in /hooks, and I've managed to make it run the tests, but I can't seem to find a way to stop the commit if they tests fail. I'm running the tests with make test, which in turn is running python3.1 foo.py --test. It seems like I don't get a different exit condition whether the tests pass or fail, but I may be looking in the wrong place.

Edit: Is this something uncommon that I want to do here? I would have thought it was a common requirement...

Edit2: Just in case people can't be bothered to read the comments, the problem was that unittest.TextTestRunner doesn't exit with non-zero status, whether the test suite is successful or not. To catch it, I did:

result = runner.run(allTests)
if not result.wasSuccessful():
    sys.exit(1)
+3  A: 

Could you parse the result of the python test session and make sure to exit your pre-commit hook with a non-zero status?

The hook should exit with non-zero status after issuing an appropriate message if it wants to stop the commit.

So if your python script does not return the appropriate status for any reason, you need to determine that status directly from the pre-commit hook script.
That would ensure the commit does not go forward if the tests failed.
(or you could call from the hook a python wrapper which would call the tests, and ensure a sys.exit(exit_status) according to the test results).

VonC
http://www.heikkitoivonen.net/blog/2009/01/28/pexpect-and-inconsistent-exit-status/ could be relevant too here
VonC
@VonC Interestingly, in the Git Book, Scott Chacon does a similar thing (parsing the test results), with Ruby: http://book.git-scm.com/5_git_hooks.html
Skilldrick
@VonC - Thanks for your help with this one. You were right that the problem was with the appropriate status not being returned. Part of the problem is my noobish shell skills - I haven't quite got the hang yet!
Skilldrick
+6  A: 

I would check to make sure that each step of the way, your script returns a non-zero exit code on failure. Check to see if your python3.1 foo.py --test returns a non-zero exit code if a test fails. Check to make sure your make test command returns a non-zero exit code. And finally, check that your pre-commit hook itself returns a non-zero exit code on failure.

You can check for a non-zero exit code by adding || echo $? to the end of a command; that will print out the exit code if the command failed.

The following example works for me (I'm redirecting stderr to /dev/null to avoid including too much extraneous output here):

$ python3.1 test.py 2>/dev/null || echo $?
1
$ make test 2>/dev/null || echo $?
python3.1 test.py
2
$ .git/hooks/pre-commit 2>/dev/null || echo $?
python3.1 test.py
1

test.py:

import unittest

class TestFailure(unittest.TestCase):
    def testFail(self):
        assert(False)

if __name__ == '__main__':
    unittest.main()

Makefile:

test:
    python3.1 test.py

.git/hooks/pre-commit:

#!/bin/sh
make test || exit 1

Note the || exit 1. This isn't necessary if make test is the last command in the hook, as the exit status of the last command will be the exit status of the script. But if you have later checks in your pre-commit hook, then you need to make sure you exit with an error; otherwise, a successful command at the end of the hook will cause your script to exit with a status of 0.

Brian Campbell
Very interesting. +1
VonC
@Brian - Thanks. The problem was that I was running my tests with `unittest.TextTestRunner` which doesn't exit with non-zero status. I had to do `if not result.wasSuccessful(): sys.exit(1)`.
Skilldrick
Another problem here is that the test is running on the working copy of the revision and not the staged. If you always commit all your changes this is not an issue, but if you do partial commits, then your intermediate commits may not pass the test (See: [http://stackoverflow.com/questions/2412450/git-pre-commit-hook-changed-added-files/3068990#3068990]
LarryH