Re: [PATCH 1/2] t4255: test am submodule with diff.submodule

From: Doug Kelly <dougk.ff7@gmail.com>
Date: Jan 7, 2015, 19:34 UTC
Message-ID: <CAEtYS8SiP8bU=82H+XxXZqa47hQ7hOAsZChCr94DwgPNft9L=g@mail.gmail.com>
In-Reply-To: <xmqqiogu1n06.fsf@gitster.dls.corp.google.com>

On Mon, Dec 29, 2014 at 9:42 AM, Junio C Hamano <gitster@pobox.com> wrote:

Show 7 quoted lines

> Eric Sunshine <sunshine@sunshineco.com> writes:
>
>>> +       (git am --abort || true) &&
>
> Why (x || y)?  Is 'x' so unreliable that we do not know how should exit?
> Should this be "test_must_fail git am --abort"?
>

Updated to test_might_fail -- we don't know if a merge is in progress or not. We still need to clean up, but disregard failure if a merge isn't in progress.

>>> +       (cd submodule && git rev-parse HEAD >../actual) &&
>
> "git -C submodule rev-parse HEAD >actual" perhaps?
>

Seems sane to me.

Show 5 quoted lines

>>> +test_expect_success 'diff.submodule unset' '
>>> +       (git config --unset diff.submodule || true) &&
>
> I think test_config and test_unconfig were invented for things like
> this (same for all the other use of "git config").

Yep, much nicer. :) Also updated to test_commit as suggested by Eric.

Thanks!

--Doug