Re: [PATCH] Interactive editor tests for commit-msg hook

From: Junio C Hamano <gitster@pobox.com>
Date: Dec 10, 2007, 03:42 UTC
Message-ID: <7vwsrnmczu.fsf@gitster.siamese.dyndns.org>
In-Reply-To: <1197204731-19553-1-git-send-email-win@wincent.com>

Wincent Colaiuta <win@wincent.com> writes:

> I didn't add similar tests for the pre-commit hook because I don't
> think that's an interesting code path; we don't care about the commit
> message in that case, only whether the commit is allowed to proceed or
> not.

Sensible.

Show 9 quoted lines

> +# set up fake editor for interactive editing
> +cat > fake-editor <<'EOF'
> +#!/bin/sh
> +cp FAKE_MSG "$1"
> +exit 0
> +EOF
> +chmod +x fake-editor
> +FAKE_EDITOR="$(pwd)/fake-editor"
> +export FAKE_EDITOR

Hmm.  Why "export"?

Show 6 quoted lines

> +test_expect_success "with no hook (editor)" \
> +	"echo 'more foo' >> file &&
> +	 git add file &&
> +	 echo 'more foo' > FAKE_MSG &&
> +	 GIT_EDITOR="$FAKE_EDITOR" git commit"
> +

I initially was going to say "this is just a style thing", but I really want our tests to read uniformly like this:

        test_expect_success 'without hook (editor)' '

                echo more foo >>file &&
                git add file &&
                echo more foo >FAKE_MSG &&
                GIT_EDITOR="$FAKE_EDITOR" git commit

 * Use single quotes around test unless there is a compelling reason not
   to;

 * The opening single quote of the test body on the same line as
   expect-success; the end quote at column 1 on a line on its own.

Notice a bug in your version? Dq around $FAKE_EDITOR is stripped away and it is harder to spot it because the script does not follow that style.