Re: [PATCH v3 3/3] sequencer: pass explicit --no-gpg-sign to merge

From: Junio C Hamano <gitster@pobox.com>
Date: Oct 17, 2020, 22:34 UTC
Message-ID: <xmqq5z788osw.fsf@gitster.c.googlers.com>
In-Reply-To: <20201017220224.o6fs5ukwqq7sqgrb@fastboi.localdomain>

Samuel Čavoj <samuel@cavoj.net> writes:

Show 7 quoted lines

>> Now that we know that the root cause of the bug you fixed was
>> because rebase rebase with the default merge strategy for two-head
>> merges use separate codepaths from and all other rebases, I wonder
>> if it is prudent to also test the same cases this series adds
>> without giving "-s resolve".  That would exercise the other codepath
>
> I will leave that for someone else to tackle eventually.

We know that other codepath has been working even before this fix, but tests are not about showing off what we fixed, but are about making sure similar breakage won't be introduced by mistake in the future. Leaving it "for someone", when we know what the problem is and how to solve it, is asking for the "evantually" not materialize forever.

Show 5 quoted lines

> As the number of very similar test is slowly growing, do you think it is
> worth copying (or making more generic) the test_rebase_gpg_sign for this
> situation as well? We currently have 4 almost identical tests (counting
> the new one you suggested for v4). Just a thought, as it is simpler to
> just add it at this point. Thanks for the feedback.

That is a tough question. Often, a generic test helper makes it too easy to do a full matrix of tests and encourages us to overdo it, which we probably would want to avoid. I think what I've suggested so far is a bare minimum combination for code coverage.