From: Jon Simons <jon@jonsimons.org>
To: git@vger.kernel.org,
Kristoffer Haugsbakk <kristofferhaugsbakk@fastmail.com>,
Junio C Hamano <gitster@pobox.com>,
Patrick Steinhardt <ps@pks.im>
Cc: Jon Simons <jon@jonsimons.org>
Subject: [PATCH v2] SubmittingPatches: avoid add-failing-test-then-fix anti-pattern
Date: Sun, 11 Oct 2026 23:00:29 -0400 [thread overview]
Message-ID: <20261012030029.12200-1-jon@jonsimons.org> (raw)
In-Reply-To: <20261011153206.72386-1-jon@jonsimons.org>
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 [1].
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.
[1] A couple examples of feedback, with rationale for avoiding
the pattern, from August 2025 here:
- https://lore.kernel.org/git/xmqqqzxpzo8b.fsf@gitster.g/
- https://lore.kernel.org/git/xmqqfrdk3aqy.fsf@gitster.g/
Signed-off-by: Jon Simons <jon@jonsimons.org>
---
Documentation/SubmittingPatches | 9 +++++++++
1 file changed, 9 insertions(+)
diff --git a/Documentation/SubmittingPatches b/Documentation/SubmittingPatches
index c60855f706..3238a33932 100644
--- a/Documentation/SubmittingPatches
+++ b/Documentation/SubmittingPatches
@@ -271,6 +271,15 @@ 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.
+A bugfix patch should include tests in test_expect_success() blocks that
+demonstrate the breakage when the code change that implements the fix is
+omitted. Submit tests 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.
+
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
next prev parent reply other threads:[~2026-10-12 4:17 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-10-11 15:32 [PATCH] SubmittingPatches: avoid add-failing-test-then-fix anti-pattern Jon Simons
2026-10-11 16:21 ` Kristoffer Haugsbakk
2026-10-11 23:44 ` Junio C Hamano
2026-10-12 1:46 ` Jon Simons
2026-10-12 3:00 ` Jon Simons [this message]
2026-10-12 14:34 ` [PATCH v2] " Junio C Hamano
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20261012030029.12200-1-jon@jonsimons.org \
--to=jon@jonsimons.org \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.com \
--cc=kristofferhaugsbakk@fastmail.com \
--cc=ps@pks.im \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox