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

> On Tue, May 3, 2016 at 2:42 AM, Pranit Bauva <pranit.bauva@gmail.com> wrote:
>> On Tue, May 3, 2016 at 10:42 AM, Eric Sunshine <sunshine@sunshineco.com> wrote:
>>> On Mon, May 2, 2016 at 11:39 PM, Pranit Bauva <pranit.bauva@gmail.com> 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.