Thread (1 message) 1 message, 1 author, 2022-03-17

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

From: Junio C Hamano <hidden>
Date: 2022-03-17 15:54:34

Ævar Arnfjörð Bjarmason  [off-list ref] 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.
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help