On 2014-01-08, at 20:06Z, Matthieu Moy <Matthieu.Moy@xxxxxxxxxxxxxxx> wrote:
> Ryan Biesemeyer <ryan@xxxxxxxxxx> writes:
>
>> index 3573751..89cdfe8 100755
>> --- a/t/t7505-prepare-commit-msg-hook.sh
>> +++ b/t/t7505-prepare-commit-msg-hook.sh
>> @@ -181,5 +181,27 @@ test_expect_success 'with failing hook (merge)' '
>> test_must_fail git merge other
>>
>> '
>> +git merge --abort # cleanup, since the merge failed.
>
> Please, avoid having code outside a test_expect_* (see t/README, " - Put
> all code inside test_expect_success and other assertions.").
In this case it was not immediately clear to me how to add cleanup to an existing
test that dirtied the state of the test repository by leaving behind an in-progress
merge. I see `test_cleanup` defined in the test lib & related functions, but see no
examples of its use in the test suites. Could you advise?
>
>> +test_expect_success 'should have MERGE_HEAD (merge)' '
>> +
>> + git checkout -B other HEAD@{1} &&
>> + echo "more" >> file &&
>> + git add file &&
>> + rm -f "$HOOK" &&
>> + git commit -m other &&
>> + git checkout - &&
>> + write_script "$HOOK" <<-EOF
>> + if [ -s "$(git rev-parse --git-dir)/MERGE_HEAD" ]; then
>> + exit 0
>> + else
>> + exit 1
>> + fi
>> + EOF
>
> I think you lack one && for the write_script line.
Good catch.
> There's another instance in the same file (probably where you got it
> from), you should add this to your patch series:
I'm new to the mailing-list patch submission process; how would I go about adding it?
Submit the cover-letter & patches again? Squash your commit into the relevant one of
mine?
> From c3d754a2a16d98b31d43a7e45973821ae8adc043 Mon Sep 17 00:00:00 2001
> From: Matthieu Moy <Matthieu.Moy@xxxxxxx>
> Date: Wed, 8 Jan 2014 21:03:27 +0100
> Subject: [PATCH] t7505: add missing &&
>
> ---
> t/t7505-prepare-commit-msg-hook.sh | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/t/t7505-prepare-commit-msg-hook.sh b/t/t7505-prepare-commit-msg-hook.sh
> index 3573751..1c95652 100755
> --- a/t/t7505-prepare-commit-msg-hook.sh
> +++ b/t/t7505-prepare-commit-msg-hook.sh
> @@ -174,7 +174,7 @@ test_expect_success 'with failing hook (merge)' '
> git add file &&
> rm -f "$HOOK" &&
> git commit -m other &&
> - write_script "$HOOK" <<-EOF
> + write_script "$HOOK" <<-EOF &&
> exit 1
> EOF
> git checkout - &&
> --
> 1.8.5.rc3.4.g8bd3721
>
> (a quick "git grep write_script" seems to show a lot of other instances,
> but no time to dig this now sorry)
>
> --
> Matthieu Moy
> http://www-verimag.imag.fr/~moy/
Thanks for the review.
--
Ryan Biesemeyer (@yaauie)
Attachment:
signature.asc
Description: Message signed with OpenPGP using GPGMail