Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Git Pre-commit Hooks Setup Instructions #9

Open
hjiang36 opened this issue Jul 22, 2014 · 8 comments
Open

Git Pre-commit Hooks Setup Instructions #9

hjiang36 opened this issue Jul 22, 2014 · 8 comments

Comments

@hjiang36
Copy link
Contributor

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

  • Only tested on mac: This instruction is only tested on mac. It should also work on Linux machine. However, Windows machines might need some more hacks

  • Bypass the pre-commit hook: To avoid the auto unit test in some situation, you could use

    $ git commit --no-verify

  • GUI vs Command Window: Pre-commit hook will be triggered by both Github GUI or terminal commands. The difference is that in terminal, you could see the test process and error message if encountered.

@DavidBrainard
Copy link
Contributor

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
Copy link
Contributor

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
Copy link
Contributor

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 v_runAllUnitTests 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
Copy link
Contributor

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
Copy link
Contributor Author

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
Copy link
Contributor Author

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
Copy link
Contributor Author

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
Copy link
Contributor

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants