Re: [PATCH v3 2/2] merge: add --quit

From: Junio C Hamano <gitster@pobox.com>
Date: May 15, 2019, 02:58 UTC
Message-ID: <xmqqlfz877ba.fsf@gitster-ct.c.googlers.com>
In-Reply-To: <nycvar.QRO.7.76.6.1905141540300.44@tvgsbejvaqbjf.bet>

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

Show 6 quoted lines

>> +test_expect_success 'merge --quit' '
>> +	git reset --hard c2 &&
>> +	test_must_fail git -c rerere.enabled=true merge master &&
>
> This makes me really worried. It is the same `master` (i.e. *not* a tag)
> that broke this test case in the previous round.

I'll let you two figure this out, but I tend to agree.

Show 10 quoted lines

>> +	test_path_is_file .git/MERGE_HEAD &&
>> +	test_path_is_file .git/MERGE_MODE &&
>> +	test_path_is_file .git/MERGE_MSG &&
>> +	test_path_is_file .git/MERGE_RR &&
>
> Isn't this a clear implementation details of `git rerere` that you just
> taught `git merge`'s regression test?
> ...
> It would probably make a ton more sense to look at the output of `git
> rerere status` instead.

While I understand your concern, it is not the business of this test to detect a bug in "git rerere status", either. The safest thing to do would be to test both ;-)

t4151 that tests "am --abort" already looks at MERGE_RR for the same "we want to make sure that the rerere state is cleared" purpose, so I'd not be worried too much about this particular test.

Thanks.