Re: [PATCH 1/2] t3507: pin CHERRY_PICK_HEAD absence for a conflicting --no-commit

[Patrick Steinhardt <ps@pks.im>]

On Fri, Sep 04, 2026 at 12:45:53AM +0300, Aleksei Sviridkin wrote:
> Junio C Hamano <gitster@pobox.com> writes:
> > It is not apparent what problem, if any, the description
> > above claims the commit addresses.  Nor is it clear why
> > checking these combinations is relevant.
> > [...]
> > Can you help me understand the above two paragraphs a bit better?
> 
> The test pins the one combination t3507 did not cover. The file already
> checks CHERRY_PICK_HEAD after a conflicting pick, after a clean pick, and
> after a clean pick under --no-commit, but not after a conflicting pick
> under --no-commit. That is the case a user hits by accident: the pick
> stops on conflicts, they resolve and run "git commit", and the original
> author is not restored. --no-commit never wrote the ref, d7e5c0cbfb skips
> it on purpose. Your reading is right and Gemini's is backwards: under
> --no-commit we do not want CHERRY_PICK_HEAD, and the test asserts it is
> absent. Without it, teaching git to write the ref there would leave the
> whole file green.

The question is whether it really makes sense to have tests for every
single edge case. In a perfect world we of course would, but in the real
world there are a) gazillions of different combinations and b) every
test brings its own overhead as it increases both wall time and
maintenance costs.

That doesn't specifically mean that this one test you add here is not
useful. But we need to have a better argument than "we didn't have it
yet". For example we might've seen regressions, the logic is extremely
fragile or we risk bad consequences like data loss or an unrecoverable
situation if a property does not hold.

It's a thin line to walk at times, and I usually wouldn't care about
this too much. But over the last couple weeks we've seen more patch
series that add random tests to our test case without good reasoning
just for the sake of adding a test. And that's something that we need to
contain a bit.

Thanks!

Patrick