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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Ævar Arnfjörð Bjarmason  <avarab@xxxxxxxxx> 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.





[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux