Re: [PATCH v2 2/9] t7061: expect failure where expected behavior will change

From: Junio C Hamano <gitster@pobox.com>
To: Samuel Lijin <sxlijin@gmail.com>
Cc: git@vger.kernel.org
Subject: Re: [PATCH v2 2/9] t7061: expect failure where expected behavior will change
Date: Mon, 08 May 2017 13:34:04 +0900	[thread overview]
Message-ID: <xmqqd1bk9dc3.fsf@gitster.mtv.corp.google.com> (raw)
In-Reply-To: <20170505104611.17845-3-sxlijin@gmail.com> (Samuel Lijin's message of "Fri, 5 May 2017 05:46:04 -0500")

Samuel Lijin <sxlijin@gmail.com> writes:

> This changes tests for `status --ignored` from test_expect_success to
> test_expect_failure in preparation for a change in its expected behavior
> (namely, that ignored files in untracked dirs will be reported).
>
> Signed-off-by: Samuel Lijin <sxlijin@gmail.com>
> ---

This is an odd way to do this.  If we stop applying your patches at
this step, these tests will still see output from "status --ignored"
that is expected by them, so there is no expect_failure here.

If we decide that the current output from "status --ignored" is
WRONG, and your series to fix "clean -d" FIXES "status --ignored" as
a side effect, then having a step to describe a desired behaviour in
the new world order in the test like this patch does makes sense,
but if that is what is going on, then not just flipping "success" to
"failure", the patch would be changing the expected output as well,
i.e. by adding the ignored files in untracked directories in the
expected output.  Obviously the code at this point after applying
only patches 1 & 2 will not produce such an output, so marking the
test that expects output based on the new world order as "expect
failure" would make sense.  Then your future commit that FIXES
"status --ignored" output would flip _failure to _success.

It is unclear to me if the new behaviour of "status --ignored" is a
bugfix, or a new bug, though.

>  t/t7061-wtstatus-ignore.sh | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/t/t7061-wtstatus-ignore.sh b/t/t7061-wtstatus-ignore.sh
> index cdc0747bf..dc3be92a2 100755
> --- a/t/t7061-wtstatus-ignore.sh
> +++ b/t/t7061-wtstatus-ignore.sh
> @@ -11,7 +11,7 @@ cat >expected <<\EOF
>  ?? untracked/
>  EOF
>  
> -test_expect_success 'status untracked directory with --ignored' '
> +test_expect_failure 'status untracked directory with --ignored' '
>  	echo "ignored" >.gitignore &&
>  	mkdir untracked &&
>  	: >untracked/ignored &&
> @@ -20,7 +20,7 @@ test_expect_success 'status untracked directory with --ignored' '
>  	test_cmp expected actual
>  '
>  
> -test_expect_success 'same with gitignore starting with BOM' '
> +test_expect_failure 'same with gitignore starting with BOM' '
>  	printf "\357\273\277ignored\n" >.gitignore &&
>  	mkdir -p untracked &&
>  	: >untracked/ignored &&