Re: [PATCH] SubmittingPatches: avoid add-failing-test-then-fix anti-pattern

["Kristoffer Haugsbakk" <kristofferhaugsbakk@fastmail.com>]

On Sun, Oct 11, 2026, at 17:32, Jon Simons wrote:
> Document the preference to avoid the 'add a failing test and fix it in a
> subsequent commit' anti-pattern.  Although such a sequence is fine for
> local development, reviewers instead prefer a single commit that
> introduces the test together with its code change.

I have skimmed this document and indeed there does not seem to be an
existing mention of this point. And this is a useful point to mention.

I think the commit message should link to a few examples of this being
stated. Like one of the ones you seemed to have found in your previous
message.

>
> The goal of this addition is to capture the preference in writing, with
> hopes of optimizing future review flows by reducing back-and-forths to
> the list.

Sure, that’s always the case for additions to this file. ;)

>
> Signed-off-by: Jon Simons <jon@jonsimons.org>
> ---
>  Documentation/SubmittingPatches | 10 ++++++++++
>  1 file changed, 10 insertions(+)
>
> diff --git a/Documentation/SubmittingPatches
> b/Documentation/SubmittingPatches
> index c60855f706..27b53d2b31 100644
> --- a/Documentation/SubmittingPatches
> +++ b/Documentation/SubmittingPatches
> @@ -271,6 +271,16 @@ fixed by accident to avoid regression.  Also, try
> merging your work to
>  that are still in flight may have unexpected interactions with what
>  you are trying to do in your topic.
>
> +When fixing a bug, you may find it useful to first write a test using
> +`test_expect_failure` to confirm that the test exposes the bug.
> +However, unless there is a strong reason not to, submit the test as
> +`test_expect_success` in the same commit as your code change that
> +fixes the bug.  A separate commit that only changes `test_expect_failure`
> +to `test_expect_success` can leave the behavior being tested out of the
> +diff context, making the fix harder to review.  Reviewers who want to
> +verify that your test reproduces the bug can temporarily undo the code
> +changes while keeping the test.

This is well-written.

Tis comes under `[[tests]]`, so it is appropriately placed. It also
comes after `[[separate-commits]]`, and it sort of functions as a
counterweight to making *too many* separate commits.

> +
>  Pushing to a fork of https://github.com/git/git will use their CI
>  integration to test your changes on Linux, Mac and Windows. See the
>  <<GHCI,GitHub CI>> section for details.
> --
> 2.55.0