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

On Fri, Sep 04, 2026 at 12:45:53AM +0300, Aleksei Sviridkin wrote:

Junio C Hamano [off-list ref] writes:

quoted

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