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

From: Junio C Hamano <gitster@pobox.com>
Date: Jan 7, 2015, 20:20 UTC
Message-ID: <xmqq387mjqbe.fsf@gitster.dls.corp.google.com>
In-Reply-To: <CAEtYS8SiP8bU=82H+XxXZqa47hQ7hOAsZChCr94DwgPNft9L=g@mail.gmail.com>

Doug Kelly <dougk.ff7@gmail.com> writes:

Show 10 quoted lines

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

Ah, OK. But even with "test_might_fail", it may not be clear why it might fail, so it would be easier to maintain if we can read "we don't know if a merge is in progress" next to the "test_might_fail".

For now we can add a comment, but in the longer term it might not be a bad idea to change test_might_fail to require two args, one is a command to run and the other is a text that explains why the outcome is unknown.

Thanks for clarifying.