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

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