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

[Eric Sunshine <sunshine@sunshineco.com>]

On Tue, May 3, 2016 at 5:18 AM, Pranit Bauva <pranit.bauva@gmail.com> wrote:
> On Tue, May 3, 2016 at 12:19 PM, Eric Sunshine <sunshine@sunshineco.com> wrote:
>>>> 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 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.
>>>
>>> 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.
>
> I forgot to mention about this earlier. When I was rebasing, this stroked me.
> I guess making any changes in ordering the commits will make one of
> the test as absurd. One of the test uses a configuration variable
> 'commit.verbose' will won't be effective before the patch 6/7. So I
> guess I will have to only change the commit message to reflect as
> "improving test coverage".

I also had intended to talk about this but forgot. What would be quite
logical is to introduce only the "git-status without --verbose" test
in this new "improve coverage" patch before 4/7. The other test, which
ensures that git-status doesn't regress with commit.verbose, would
then very naturally be included in the patch which adds the
commit.verbose functionality (currently patch 6/7).