Re: [PATCHv3] checkout: do not mention detach advice for explicit --detach option

From: Stefan Beller <sbeller@google.com>
Date: Aug 12, 2016, 15:51 UTC
Message-ID: <CAGZ79kaOHdLrPownWdnZQSEzG6P6-iZqx2xu3MEG51DZU1U+Cg@mail.gmail.com>
In-Reply-To: <20160812154331.y2z6pv2w6cwsdsqw@sigill.intra.peff.net>

On Fri, Aug 12, 2016 at 8:43 AM, Jeff King <peff@peff.net> wrote:

Show 12 quoted lines

> On Fri, Aug 12, 2016 at 08:37:44AM -0700, Stefan Beller wrote:
>
>> > Is there a reason for not unsetting `advice.detachedHead` at the
>> > end of the test?
>>
>> done
>>
>> I did not consider to clean up after myself... what a selfish world!
>
> The right way to do it is:
>
>   test_config advice.detachedHead false &&

okay.

Show 23 quoted lines

>   ...
>
> so that the cleanup runs whether or not you may it to the end of the
> script.
>
>> +test_expect_success 'no advice given for explicit detached head state' '
>> +     git config advice.detachedHead false &&
>> +     git checkout child &&
>> +     git checkout --detach HEAD >expect &&
>> +     git config advice.detachedHead true &&
>> +     git checkout child &&
>> +     git checkout --detach HEAD >actual &&
>> +     test_cmp expect actual &&
>> +     git checkout child &&
>> +     git checkout HEAD >actual &&
>> +     ! test_cmp expect actual &&
>> +     git config --unset advice.detachedHead
>> +'
>
> Hmm. Using "!test_cmp" seems weird to me, just because it would falsely
> claim success if something else unexpected changes. Our usual method for
> making sure some particular output does not appear is "test_i18ngrep"
> with a liberal pattern.

and I advanced the liberal a bit more. ;) So maybe we'd be looking that no 'detached HEAD' occurs?

    test_i18ngrep ! "'detached HEAD'" actual

I think testing that we do not give out advice (i.e. behave the same as if not giving out advice) is best tested to just compare the output to the output of "git -c advice.detachedHead=false ...", which is what I do here. This seems to be future proof to me no matter how we reword the advice or the actual message on checkout?

Thanks, Stefan

>
> -Peff