Re: [PATCH 1/7] t7300: skip untracked dirs containing ignored files

From: Stefan Beller <sbeller@google.com>
Date: May 3, 2017, 18:34 UTC
Message-ID: <CAGZ79katJKWOY=FdoU2DO-bwyZSA9KL3kcUgw1uKLyytNCXd-Q@mail.gmail.com>
In-Reply-To: <CAJZjrdWLuXF7c=kt4SM1CcOUPpmmZXmOZPUCuzcO0z2nNsVMLQ@mail.gmail.com>

On Wed, May 3, 2017 at 11:26 AM, Samuel Lijin <sxlijin@gmail.com> wrote:

Show 17 quoted lines

> On Wed, May 3, 2017 at 1:19 PM, Stefan Beller <sbeller@google.com> wrote:
>> On Tue, May 2, 2017 at 8:29 PM, Samuel Lijin <sxlijin@gmail.com> wrote:
>>> If git sees a directory which contains only untracked and ignored
>>> files, clean -d should not remove that directory.
>>
>> Yes that states a fact; it is not clear why we want to have this test here
>> and now. (Is it testing for a recently fixed regression?)
>
> It was recently discovered that this is not true of clean -d (and I'm
> not sure if it ever was, to be honest). See
> http://public-inbox.org/git/xmqqshkof6jd.fsf@gitster.mtv.corp.google.com/T/#mf541c06250724bb000461d210b4ed157e415a596
>
>> Are you just introducing the test to demonstrate it keeps working later on?
>> Do you plan on changing this behavior in a later patch?)
>
> The idea was to introduce the broken test and then have the rest of
> the patch series fix it;

This is valuable information for the commit message. Also to keep git-bisect happy we want to have all tests passing on all commits, which this does not. However to further signal your intent, you can mark it as test_expect_failure. This is marking a test as "if Git was not buggy, this is a test that would pass", so it actually marks success when it fails, for example when running t7300:

... ok 31 - should not clean submodules ok 32 - should avoid cleaning possible submodules ok 33 - nested (empty) git should be kept ok 34 - nested bare repositories should be cleaned not ok 35 - nested (empty) bare repositories should be cleaned even when in .git # TODO known breakage not ok 36 - nested (non-empty) bare repositories should be cleaned even when in .git # TODO known breakage ok 37 - giving path in nested git work tree will remove it ok 38 - giving path to nested .git will not remove it ok 39 - giving path to nested .git/ will remove contents ok 40 - force removal of nested git work tree ...

Similarly if you were to mark that test as expecting failure, we'd see:

no ok - 45 git clean -d skips untracked dirs containing ignored files # TODO known breakage

And then in a later patch, when you actually fix the bug, you would just flip it to test_expect_success.

Thanks, Stefan