Re: [PATCH v2 1/2] tests: demonstrate "show --word-diff --color-moved" regression

From: Junio C Hamano <gitster@pobox.com>
Date: Mar 17, 2022, 15:54 UTC
Message-ID: <xmqqtubwh8wf.fsf@gitster.g>
In-Reply-To: <patch-v2-1.2-7f6a6450259-20220317T144838Z-avarab@gmail.com>

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

Show 5 quoted lines

> +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.