isetbio / isetbio_v0.1

Tools for modeling image systems engineering in the human visual system front end
MIT License
7 stars 0 forks source link

Git Pre-commit Hooks Setup Instructions #9

Open hjiang36 opened 10 years ago

hjiang36 commented 10 years ago

Introduction

Git pre-commit hooks could enable an auto unit test for every git commit command. All the unit test information is printed if 'git commit' is called in a command window. If commit is issued by Github software GUI, the auto unit test will also be run, but no output information is visible.

Setup Step-by-Step

In this section, we will introduce how to setup the pre-commit hooks for ISETBIO. This could take you around 10 minutes. Now let's begin.

Initialize Git Repository

At this stage, we assume that you have already got git (version 1.8.2 or later) running on your machine. If not, you could download it from here. In a command window (terminal), change current working directory to ISETBIO root folder and initialize (or re-initialize) git there. For example:

# Init git folder
$ cd ~/isetbio
$ git init

Create Pre-commit Hooks

In command window (terminal), change current working directory to hooks in .git folder of ISETBIO (e.g. ~/isetbio/.git/hooks). In hooks folder, you could see a bunch of example hooks. Create a new file called pre-commit (with no extension) in this folder and copy the following lines to it.

# pre-commit.sh
echo Start pre-commit testing...
git stash -q --keep-index
./.git/hooks/run_tests.sh
RESULT=$?
git stash pop -q
[ $RESULT -ne 0 ] && exit 1
exit 0

Set Path for Shell Commands

In isetbio/.git/hooks, create a file called run_tests.sh (this is called by pre-commit). Copy and paste the following lines to the file

# Setup alias
matlab="/Applications/MATLAB_R2014a.app/bin/matlab"

# Run unit test in matlab
cmd="cd ../../utility/unit\ test/; unitTest;"
"$matlab" -nodesktop -nosplash -nodisplay -r "$cmd"
if [ "$?" == "0" ];
then
    echo Unit test passed!
    exit 0
else
    echo Unit Test Failed
    exit 1
fi

You should change the first line to the path to your local matlab executable. Also, remember to make this file executable. You could achieve this by

$chmod u+x run_tests.sh

Adding more unit test modules

To add more testing functions, you could create a Matlab function with no output argument. Then add the function name into the try-catch structure in unitTest.m

Notes

DavidBrainard commented 10 years ago

I think the information posted here as an issue is really documentation that should go onto the gitHub wiki page that goes with this repository. That is, we want to be able to find these instructions in the future, and it won't be obvious (to us or to users) to hunt it down in an issue. So Nicolas and I made a wiki page and put all this information onto it.

DavidBrainard commented 10 years ago

One thing I am worried about is what will happen if we have these in place and I run a commit from my client. If it fails, will it tell me or will I think I've committed something? Or will it crash the client completely? Or not run the validations at all? I will see if I can try this in the next few days, but am holding off on this pull request for now.

DavidBrainard commented 10 years ago

So just to make sure I'm clear, the idea is that as we add more tests, we simply add them to unitTest.m? I think I'd call that vrunAllUnitTests or something like it, following the "v" convention (v for validation). Also note that the header comment in unitTest.m is incorrect, as it indicates that it returns a result when it does not.

DavidBrainard commented 10 years ago

Is there some scheme we can come up with to make it so that it isn't necessary to edit the shell script to point to Matlab on each machine? That seems like a bit of a pain. If nothing else, an environment variable might do it. But even better if there were an automated way.

hjiang36 commented 10 years ago

I think we could create a Matlab script to install hooks and get related stuff done. One command installation could be better and easier to use. I'll make that later today.

hjiang36 commented 10 years ago

I made a new function called installHooks which could help setup all the stuff you need. Please try if interested.

There is one potential issue here: if there's an interactive startup.m for matlab, the auto-unit test might get stuck. I will try to find a way out there...

hjiang36 commented 10 years ago

Here's one possible way to minimize the influence from startup.m In startup.m, check jvm and desktop options. If there's nojvm and nodesktop, we assume that this session is started to run unit-test. Otherwise, it's a usual matlab session and do whatever you like.

To achieve this, you might need to add one line at the beginning of startup.m as below:

if ~usejava('desktop'), return; end
DavidBrainard commented 10 years ago

OK, the install script almost worked. The only problem I found is that it doesn't make the pre-commit routine executable. Then I was able to see the unit test run when I did a commit from the command line. Yay!

There remain some issues. I think the hook runs when I commit from within my client (because it takes noticeably longer), but I haven't figured out how to look at what happened. I have submitted a support request with the vendor to see if there is a way to do this. Or maybe, Nicolas, you know how to examine the output of the commit command from within Tower?

Also, the instructions for manual installation on the wiki are broken. They give the wrong place to put the run_tests.sh (as far as I can tell, based on what the install script did and the fact that it didn't run until I ran the install script), and the run_tests.sh as given there doesn't run (several little problems: cd command fails as written because the "\ " isn't interpreted right, and the "../.." isn't needed on my system. I think it would be fine to modify the instructions on the wiki just to refer to how to install using the install script, once it is fixed.

I think we should also think about whether we want to use the hook mechanism. In addition to the client issue, it does slow things down quite a bit per commit. It is possible we just want to have the unit test script set up and to run it manually before committing, or at least before issuing a pull request. One more thing to talk about tomorrow.