From: Erwan Mathoniere <erwan.mathoniere@grenoble-inp.org>
Date: Tue, 07 Jun 2016 12:54:20 GMT
Subject: Re: [RFC/PATCH v2] pull: add --set-upstream
Message-ID: <22613f02-06b8-9f5f-fd92-15df3ca3a34f@grenoble-inp.org>
In-Reply-To: <vpqeg89bhqt.fsf@anie.imag.fr>



On 07/06/2016 09:06, Matthieu Moy wrote:
> Junio C Hamano <gitster@pobox.com> writes:
>
>> Matthieu Moy <Matthieu.Moy@grenoble-inp.fr> writes:
>>
>>>> +test_config_unchanged () {
>>>> +	git config --list --local >original
>>>> +	"$@"
>>>> +	git config --list --local >modified
>>>> +	test_cmp original modified
>>>> +}
>>>
>>> The test passes if "$@" fails. You should &&-chain the lines here to
>>> catch things like crashes or unexpected "exit 1" in git.
>>
>> That is true, but allowing "$@" failure may be deliberate.
>
> I don't think so:
>
> +test_expect_success 'pull -u should not work when merging unrelated histories' '
> +	git checkout master &&
> +	test_config_unchanged test_must_fail git pull -u step_parent master
> +'
>
> ;-)
>

When writing `test_config_unchanged` procedure, I wanted to return the 
comparison even if "$@" failed. But it's indeed not consistent with the 
way I wrote the tests.

Adding "&&" between instructions and using `test_must_fail` when calling 
the procedure is clearer and more logical.