Re: [PATCH v15 7/7] t/t7507: tests for broken behavior of status

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

 



On Tue, May 3, 2016 at 2:42 AM, Pranit Bauva <pranit.bauva@xxxxxxxxx> wrote:
> On Tue, May 3, 2016 at 10:42 AM, Eric Sunshine <sunshine@xxxxxxxxxxxxxx> wrote:
>> On Mon, May 2, 2016 at 11:39 PM, Pranit Bauva <pranit.bauva@xxxxxxxxx> wrote:
>>> I agree that using test_expect_failure would be a better way of going
>>> with this thing. Thanks. Will send an updated patch for this.
>>
>> Please don't. test_expect_failure() is not warranted.
>
> I got confused between test_must_fail and test_expect_failure. I
> thought Junio mentioned to use test_must_fail and remove the " ! "
> sign.
>
>> Step back a moment and recall why these tests were added. Earlier
>> rounds of this series were buggy and caused regressions in git-status.
>> As a consequence, reviewers suggested[1,2] that you improve test
>> coverage to ensure that such breakage is caught early.
>>
>> The problems which caused the regressions were addressed in later
>> versions of the series, thus using test_expect_success() is indeed
>> correct, whereas test_expect_failure(), which illustrates broken
>> behavior, would be the wrong choice.
>>
>> The point of these new tests is to prevent regressions caused by
>> *subsequent* changes, which is why it was suggested that these tests
>> be added early (as a "preparatory patch"[3]), not at the very end of
>> the series as done here in v15.
>>
>> This patch's commit message is perhaps a bit too detailed about what
>> could have gone wrong in earlier patches in this series; indeed, it
>> misled Junio into thinking that patches in this series did break
>> behavior, when in fact, it was instead previous rounds of this series
>> which were buggy. If you instead make this a preparatory patch[3],
>> then you can sell it more simply by explaining that git-commit and
>> git-status share implementation (without necessarily going into detail
>> about exactly what is shared), and that you're improving test coverage
>> to ensure that changes specific to git-commit don't accidentally
>> impact git-status, as well.
>
> Sure! I just wanted the commit message to be detailed as per the
> guidelines given by SubmittingPatches. I will swap the patch 6/7 and
> patch 7/7 changing the commit message. Also I will make the commit
> message less detailed.

This patch should be inserted before 4/7 since it needs to protect
against breakage which might occur when 4/7 changes the behavior of
OPTION_COUNTUP.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[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]