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

From: Johannes Schindelin <johannes.schindelin@gmx.de>
Date: May 15, 2019, 15:00 UTC
Message-ID: <nycvar.QRO.7.76.6.1905151658090.46@tvgsbejvaqbjf.bet>
In-Reply-To: <xmqqlfz877ba.fsf@gitster-ct.c.googlers.com>

Hi Junio,

On Wed, 15 May 2019, Junio C Hamano wrote:

Show 20 quoted lines

> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
>
> >> +	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.

I spend *way* too much time chasing regression test failures that turn out not to show any bugs in the code they want to safeguard, but instead the bugs are found in the *assumptions* of the regression tests. So much time, in fact, that I have to disagree with you here. t4151 is just as wrong.

Ciao, Dscho