All of lore.kernel.org
 help / color / mirror / Atom feed
From: Stefan Beller <sbeller@google.com>
To: Samuel Lijin <sxlijin@gmail.com>
Cc: "git@vger.kernel.org" <git@vger.kernel.org>
Subject: Re: [PATCH 1/7] t7300: skip untracked dirs containing ignored files
Date: Wed, 3 May 2017 11:34:50 -0700	[thread overview]
Message-ID: <CAGZ79katJKWOY=FdoU2DO-bwyZSA9KL3kcUgw1uKLyytNCXd-Q@mail.gmail.com> (raw)
In-Reply-To: <CAJZjrdWLuXF7c=kt4SM1CcOUPpmmZXmOZPUCuzcO0z2nNsVMLQ@mail.gmail.com>

On Wed, May 3, 2017 at 11:26 AM, Samuel Lijin <sxlijin@gmail.com> wrote:
> 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

  reply	other threads:[~2017-05-03 18:34 UTC|newest]

Thread overview: 89+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-04-28 22:56 Bug Report: .gitignore behavior is not matching in git clean and git status Chris Johnson
2017-05-01  1:41 ` Junio C Hamano
2017-05-01  1:56   ` Chris Johnson
2017-05-01  3:15     ` Junio C Hamano
2017-05-01 13:51     ` Samuel Lijin
2017-05-01 15:34       ` Samuel Lijin
2017-05-02  0:26         ` Junio C Hamano
2017-05-03  3:29           ` [PATCH 0/7] Keep git clean -d from inadvertently removing ignored files Samuel Lijin
2017-05-03  3:29             ` [PATCH 1/7] t7300: skip untracked dirs containing " Samuel Lijin
2017-05-03 18:19               ` Stefan Beller
2017-05-03 18:26                 ` Samuel Lijin
2017-05-03 18:34                   ` Stefan Beller [this message]
2017-05-03  3:29             ` [PATCH 2/7] dir: recurse into untracked dirs for " Samuel Lijin
2017-05-03  3:29             ` [PATCH 3/7] dir: add method to check if a dir_entry lexically contains another Samuel Lijin
2017-05-03 18:09               ` Stefan Beller
2017-05-03  3:29             ` [PATCH 4/7] dir: hide untracked contents of untracked dirs Samuel Lijin
2017-05-03  3:29             ` [PATCH 5/7] dir: change linkage of cmp_name() and check_contains() Samuel Lijin
2017-05-03  3:29             ` [PATCH 6/7] builtin/clean: teach clean -d to skip dirs containing ignored files Samuel Lijin
2017-05-03  3:29             ` [PATCH 7/7] t7061: check for ignored file in untracked dir Samuel Lijin
2017-05-05 10:46             ` [PATCH v2 0/9] Keep git clean -d from inadvertently removing ignored files Samuel Lijin
2017-05-08  4:26               ` Junio C Hamano
2017-05-08 21:37                 ` Samuel Lijin
2017-05-09  0:31                   ` Junio C Hamano
2017-05-16  7:34               ` [PATCH v3 0/8] Fix clean -d and status --ignored Samuel Lijin
2017-05-18  8:21                 ` [PATCH v4 0/6] " Samuel Lijin
2017-05-23  9:18                   ` [PATCH v5 " Samuel Lijin
2017-05-23 10:09                     ` [PATCH v6 " Samuel Lijin
2017-05-23 10:09                     ` [PATCH v6 1/6] t7300: clean -d should skip dirs with ignored files Samuel Lijin
2017-05-23 10:09                     ` [PATCH v6 2/6] t7061: status --ignored should search untracked dirs Samuel Lijin
2017-05-23 10:09                     ` [PATCH v6 3/6] dir: recurse into untracked dirs for ignored files Samuel Lijin
2017-05-23 10:09                     ` [PATCH v6 4/6] dir: hide untracked contents of untracked dirs Samuel Lijin
2017-05-23 10:09                     ` [PATCH v6 5/6] dir: expose cmp_name() and check_contains() Samuel Lijin
2017-05-23 10:09                     ` [PATCH v6 6/6] clean: teach clean -d to preserve ignored paths Samuel Lijin
2017-05-23 22:35                       ` Junio C Hamano
2017-05-24  4:14                       ` Torsten Bögershausen
2017-05-25 15:36                         ` Samuel Lijin
2017-05-26  1:05                           ` Junio C Hamano
2017-05-23  9:18                   ` [PATCH v5 1/6] t7300: clean -d should skip dirs with ignored files Samuel Lijin
2017-05-23  9:18                   ` [PATCH v5 2/6] t7061: status --ignored should search untracked dirs Samuel Lijin
2017-05-23  9:18                   ` [PATCH v5 3/6] dir: recurse into untracked dirs for ignored files Samuel Lijin
2017-05-23  9:18                   ` [PATCH v5 4/6] dir: hide untracked contents of untracked dirs Samuel Lijin
2017-05-23  9:18                   ` [PATCH v5 5/6] dir: expose cmp_name() and check_contains() Samuel Lijin
2017-05-23  9:18                   ` [PATCH v5 6/6] clean: teach clean -d to preserve ignored paths Samuel Lijin
2017-05-23 12:52                     ` Junio C Hamano
2017-05-23 19:16                       ` Samuel Lijin
2017-05-18  8:21                 ` [PATCH v4 1/6] t7300: clean -d should skip dirs with ignored files Samuel Lijin
2017-05-18  8:21                 ` [PATCH v4 2/6] t7061: status --ignored should search untracked dirs Samuel Lijin
2017-05-18  8:21                 ` [PATCH v4 3/6] dir: recurse into untracked dirs for ignored files Samuel Lijin
2017-05-18  8:21                 ` [PATCH v4 4/6] dir: hide untracked contents of untracked dirs Samuel Lijin
2017-05-22  3:13                   ` Junio C Hamano
2017-05-18  8:21                 ` [PATCH v4 5/6] dir: expose cmp_name() and check_contains() Samuel Lijin
2017-05-22  0:38                   ` Junio C Hamano
2017-05-18  8:21                 ` [PATCH v4 6/6] clean: teach clean -d to skip dirs containing ignored files Samuel Lijin
2017-05-22  4:48                   ` Junio C Hamano
2017-05-22  5:58                     ` Samuel Lijin
2017-05-22  6:17                       ` Junio C Hamano
2017-05-23  2:43                         ` Samuel Lijin
2017-05-23  7:50                           ` Junio C Hamano
2017-05-16  7:34               ` [PATCH v3 1/8] t7300: clean -d should skip dirs with " Samuel Lijin
2017-05-16  7:34               ` [PATCH v3 2/8] t7061: status --ignored should search untracked dirs Samuel Lijin
2017-05-16  7:34               ` [PATCH v3 3/8] dir: recurse into untracked dirs for ignored files Samuel Lijin
2017-05-17  6:23                 ` Junio C Hamano
2017-05-17  7:02                   ` Samuel Lijin
2017-05-16  7:34               ` [PATCH v3 4/8] dir: hide untracked contents of untracked dirs Samuel Lijin
2017-05-17  6:47                 ` Junio C Hamano
2017-05-17  7:32                   ` Samuel Lijin
2017-05-17 10:14                     ` Junio C Hamano
2017-05-16  7:34               ` [PATCH v3 5/8] dir: expose cmp_name() and check_contains() Samuel Lijin
2017-05-16  7:34               ` [PATCH v3 6/8] clean: teach clean -d to skip dirs containing ignored files Samuel Lijin
2017-05-18  2:34                 ` Junio C Hamano
2017-05-18  6:32                   ` Samuel Lijin
2017-05-16  7:34               ` [PATCH v3 7/8] t7300: clean -d now skips untracked " Samuel Lijin
2017-05-18  2:38                 ` Junio C Hamano
2017-05-16  7:34               ` [PATCH v3 8/8] t7061: status --ignored now searches untracked dirs Samuel Lijin
2017-05-18  2:46                 ` Junio C Hamano
2017-05-05 10:46             ` [PATCH v2 1/9] t7300: skip untracked dirs containing ignored files Samuel Lijin
2017-05-07 19:12               ` Torsten Bögershausen
2017-05-08 21:24                 ` Samuel Lijin
2017-05-05 10:46             ` [PATCH v2 2/9] t7061: expect failure where expected behavior will change Samuel Lijin
2017-05-08  4:34               ` Junio C Hamano
2017-05-05 10:46             ` [PATCH v2 3/9] dir: recurse into untracked dirs for ignored files Samuel Lijin
2017-05-05 10:46             ` [PATCH v2 4/9] dir: add method to check if a dir_entry lexically contains another Samuel Lijin
2017-05-05 10:46             ` [PATCH v2 5/9] dir: hide untracked contents of untracked dirs Samuel Lijin
2017-05-05 10:46             ` [PATCH v2 6/9] dir: change linkage of cmp_name() and check_contains() Samuel Lijin
2017-05-08  4:37               ` Junio C Hamano
2017-05-05 10:46             ` [PATCH v2 7/9] builtin/clean: teach clean -d to skip dirs containing ignored files Samuel Lijin
2017-05-05 10:46             ` [PATCH v2 8/9] t7300: clean -d now skips untracked " Samuel Lijin
2017-05-05 10:46             ` [PATCH v2 9/9] t7061: expect ignored files in untracked dirs Samuel Lijin
2017-05-08  6:35               ` Junio C Hamano

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to='CAGZ79katJKWOY=FdoU2DO-bwyZSA9KL3kcUgw1uKLyytNCXd-Q@mail.gmail.com' \
    --to=sbeller@google.com \
    --cc=git@vger.kernel.org \
    --cc=sxlijin@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.