git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: "Ævar Arnfjörð Bjarmason" <avarab@gmail.com>
Cc: git@vger.kernel.org, Michael J Gruber <git@grubix.eu>
Subject: Re: [PATCH v2 1/2] tests: demonstrate "show --word-diff --color-moved" regression
Date: Thu, 17 Mar 2022 08:54:24 -0700	[thread overview]
Message-ID: <xmqqtubwh8wf.fsf@gitster.g> (raw)
In-Reply-To: <patch-v2-1.2-7f6a6450259-20220317T144838Z-avarab@gmail.com> (=?utf-8?B?IsOGdmFyIEFybmZqw7Zyw7A=?= Bjarmason"'s message of "Thu, 17 Mar 2022 15:55:34 +0100")

Ævar Arnfjörð Bjarmason  <avarab@gmail.com> writes:

> +test_expect_failure 'no effect on show from --color-moved with --word-diff' '
> +	git show --color-moved --word-diff >actual &&
> +	git show --word-diff >expect &&
> +	test_cmp expect actual
> +'

OK.  Just for future reference ...

In this case it is OK to start with a "document current failure"
that gets turned into expecting success in a separate patch for two
reasons, (1) it is by somebody other than the author of the patch
that fixes the breakage, and more importantly (2) the body of the
test is short enough.

But in all normal cases, please add a test that expects success in
the commit that implements a fix.  A one-line change that turns
expects_failure to expect_success in the commit that implements a
fix, when presented in a patch form with the standard 3-line
context, does not often have enough post-context to show the
behaviour the test tries to exercise and makes reviewing harder.  It
also makes it more cumbersome to cherry-pick the fix to a different
context as the two patches must be kept together.

Thanks.


  reply	other threads:[~2022-03-17 15:54 UTC|newest]

Thread overview: 22+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-03-04  4:31 What's cooking in git.git (Mar 2022, #01; Thu, 3) Junio C Hamano
2022-03-04 13:25 ` ab/plug-random-leaks (was Re: What's cooking in git.git (Mar 2022, #01; Thu, 3)) Derrick Stolee
2022-03-04 18:33   ` Ævar Arnfjörð Bjarmason
2022-03-17 12:46     ` [PATCH] tests: test show --word-diff --color-moved Michael J Gruber
2022-03-17 14:55       ` [PATCH v2 0/2] diff.c: fix a recent memory leak regression Ævar Arnfjörð Bjarmason
2022-03-17 14:55         ` [PATCH v2 1/2] tests: demonstrate "show --word-diff --color-moved" regression Ævar Arnfjörð Bjarmason
2022-03-17 15:54           ` Junio C Hamano [this message]
2022-03-17 14:55         ` [PATCH v2 2/2] diff.c: fix a double-free regression in a18d66cefb Ævar Arnfjörð Bjarmason
2022-03-04 15:35 ` tb/cruft-packs (was Re: What's cooking in git.git (Mar 2022, #01; Thu, 3)) Derrick Stolee
2022-03-07 18:06   ` Jonathan Nieder
2022-03-07 18:18     ` Taylor Blau
2022-03-07 18:32       ` Derrick Stolee
2022-03-07 20:18         ` Jonathan Nieder
2022-03-07 20:51           ` Derrick Stolee
2022-03-07 21:34             ` Junio C Hamano
2022-03-08  0:52               ` Taylor Blau
2022-03-08  0:25       ` Junio C Hamano
2022-03-08  0:49         ` Taylor Blau
2022-03-05 14:25 ` jc/stash-drop (was: " Ævar Arnfjörð Bjarmason
2022-03-07 18:22   ` jc/stash-drop Junio C Hamano
2022-03-07 13:49 ` ds/commit-graph-gen-v2-fixes (was Re: What's cooking in git.git (Mar 2022, #01; Thu, 3)) Derrick Stolee
2022-03-07 17:18   ` 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=xmqqtubwh8wf.fsf@gitster.g \
    --to=gitster@pobox.com \
    --cc=avarab@gmail.com \
    --cc=git@grubix.eu \
    --cc=git@vger.kernel.org \
    /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;
as well as URLs for NNTP newsgroup(s).