From: Doug Kelly <dougk.ff7@gmail.com>
Date: Wed, 07 Jan 2015 19:34:23 GMT
Subject: Re: [PATCH 1/2] t4255: test am submodule with diff.submodule
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:
> 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.

>>> +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