Re: [PATCH 09/12] t4150: am with post-applypatch hook

From: Paul Tan <pyokagan@gmail.com>
Date: Jul 7, 2015, 06:47 UTC
Message-ID: <CACRoPnQ_bUtLuavFCdJGFAW08EY_LGV5Q=RR9Ey3jsoPk6-C3w@mail.gmail.com>
In-Reply-To: <17f64cb49c820eb41d0fb58435c91c40@www.dscho.org>

On Sun, Jul 5, 2015 at 11:58 PM, Johannes Schindelin <johannes.schindelin@gmx.de> wrote:

Show 53 quoted lines

> On 2015-07-02 20:16, Paul Tan wrote:
>
>> diff --git a/t/t4150-am.sh b/t/t4150-am.sh
>> index dd6fe81..62b678c 100755
>> --- a/t/t4150-am.sh
>> +++ b/t/t4150-am.sh
>> @@ -275,6 +275,48 @@ test_expect_success 'am with failing pre-applypatch hook' '
>>       test_cmp_rev first HEAD
>>  '
>>
>> +test_expect_success 'am with post-applypatch hook' '
>> +     test_when_finished "rm -f .git/hooks/post-applypatch" &&
>> +     rm -fr .git/rebase-apply &&
>> +     git reset --hard &&
>> +     git checkout first &&
>> +     mkdir -p .git/hooks &&
>> +     cat >.git/hooks/post-applypatch <<-\EOF &&
>> +     #!/bin/sh
>> +     git rev-parse HEAD >head.actual
>> +     git diff second >diff.actual
>> +     exit 0
>> +     EOF
>> +     chmod +x .git/hooks/post-applypatch &&
>> +     git am patch1 &&
>> +     test_path_is_missing .git/rebase-apply &&
>> +     test_cmp_rev second HEAD &&
>> +     git rev-parse second >head.expected &&
>> +     test_cmp head.expected head.actual &&
>> +     git diff second >diff.expected &&
>> +     test_cmp diff.expected diff.actual
>> +'
>> +
>> +test_expect_success 'am with failing post-applypatch hook' '
>> +     test_when_finished "rm -f .git/hooks/post-applypatch" &&
>> +     rm -fr .git/rebase-apply &&
>> +     git reset --hard &&
>> +     git checkout first &&
>> +     mkdir -p .git/hooks &&
>> +     cat >.git/hooks/post-applypatch <<-\EOF &&
>> +     #!/bin/sh
>> +     git rev-parse HEAD >head.actual
>> +     exit 1
>> +     EOF
>> +     chmod +x .git/hooks/post-applypatch &&
>> +     git am patch1 &&
>> +     test_path_is_missing .git/rebase-apply &&
>> +     git diff --exit-code second &&
>> +     test_cmp_rev second HEAD &&
>> +     git rev-parse second >head.expected &&
>> +     test_cmp head.expected head.actual
>> +'
>
> These 2 tests as well as the previous patches look to me as if they could be refactored (the paradigm is the same: add a certain hook after resetting and then apply the patch, verify that the hook ran/failed)... do you think there is a chance for that?

I had a look, but I think that while it is true that the overall sequence of each test is the same, the details differ enough that there's no obvious way to refactor the tests sensibly. For example, the contents of the hook scripts are not the same, as we need to check that the hooks are run at the correct stage of git-am execution. And as such, the verification tests are also different as well.

Junio did correctly point out though that we can shave off 2 lines if the write_script helper is used (the shebang and the chmod).

Thanks, Paul