Re: [PATCH v2 9/9] t7061: expect ignored files in untracked dirs

[Junio C Hamano <gitster@pobox.com>]

Samuel Lijin <sxlijin@gmail.com> writes:

> We now expect `status --ignored` to list ignored files even if they are
> in an untracked directory.
>
> Signed-off-by: Samuel Lijin <sxlijin@gmail.com>
> ---
>  t/t7061-wtstatus-ignore.sh | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/t/t7061-wtstatus-ignore.sh b/t/t7061-wtstatus-ignore.sh
> index dc3be92a2..fc6013ba3 100755
> --- a/t/t7061-wtstatus-ignore.sh
> +++ b/t/t7061-wtstatus-ignore.sh
> @@ -9,9 +9,10 @@ cat >expected <<\EOF
>  ?? actual
>  ?? expected
>  ?? untracked/
> +!! untracked/ignored
>  EOF

In my comment on an earlier step (2/9, I think), I said it is
unclear if the change in behaviour to "status --ignored" is a new
bug you are introducing, or is a fix that happens as a side-effect.

I may be misunderstanding what the test that uses this expected
output is trying to see, but to me, it seems that:

        --ignored::
                Show ignored files as well.

is clear enough that "status --ignored", regardless of the
"--unracked" settings, should show the ignored file even when the
directory it happens to be in does not have any file that is
registerd in the index, and the expected output updated by this
patch looks to me the one that we _should_ be expecting.  IOW, the
change in behaviour looks like a bugfix to me.  And if that is
indeed the case, the above change should be in the earlier patch
that flips "expect_success" to "expect_failure".  The "expected"
file is prepared to expect the "correct" output before the code is
updated to produce one (i.e. the test update declares that the
current behaviour is broken), and then with changes in a later step
(i.e. somewhere before 7/9) the code starts to produce the "correct"
output at which point in that same patch you flip expect_failure into
expect_success.

The log message of eb8c5b87 ("git-status: Test --ignored behavior",
2012-12-30) says otherwise, though.  I am undecided, if I agree with
the design decision described by the first two bullet points:

    commit eb8c5b872ef144add4ac89f85bcddc974ac7114d
    Author: Antoine Pelisse <apelisse@gmail.com>
    Date:   Sun Dec 30 15:39:01 2012 +0100

        git-status: Test --ignored behavior

        Test all possible use-cases of git-status "--ignored" with the
        "--untracked-files" option with values "normal" and "all":

         - An untracked directory is listed as untracked if it has a mix of
           untracked and ignored files in it.  With -uall, ignored/untracked
           files are listed as ignored/untracked.

         - An untracked directory with only ignored files is listed as
           ignored.  With -uall, all files in the directory are listed.

         - An ignored directory is listed as ignored. With -uall, all files
           in the directory are listed as ignored.

         - An ignored and committed directory is listed as ignored if it has
           untracked files.  With -uall, all untracked files in the
           directory are listed as ignored.

        Signed-off-by: Antoine Pelisse <apelisse@gmail.com>
        Signed-off-by: Junio C Hamano <gitster@pobox.com>

     t/t7061-wtstatus-ignore.sh | 146 +++++++++++++++++++++++++++++++++++++++++++++
     1 file changed, 146 insertions(+)

What do others think?

By the way, I wonder what the performance impact of this change
would be.  Do we end up needing to scan more parts of the
filesystem, when we already know that a directory does not have any
tracked paths?

> -test_expect_failure 'status untracked directory with --ignored' '
> +test_expect_success 'status untracked directory with --ignored' '
>  	echo "ignored" >.gitignore &&
>  	mkdir untracked &&
>  	: >untracked/ignored &&
> @@ -20,7 +21,7 @@ test_expect_failure 'status untracked directory with --ignored' '
>  	test_cmp expected actual
>  '
>  
> -test_expect_failure 'same with gitignore starting with BOM' '
> +test_expect_success 'same with gitignore starting with BOM' '
>  	printf "\357\273\277ignored\n" >.gitignore &&
>  	mkdir -p untracked &&
>  	: >untracked/ignored &&