Re: [PATCH(amend)] Really run "git-clean -n" in test

From: Junio C Hamano <junkio@cox.net>
Date: May 6, 2007, 19:18 UTC
Message-ID: <7vlkg1bw17.fsf@assigned-by-dhcp.cox.net>
In-Reply-To: <463E27BC.5060604@uwaterloo.ca>

Michael Spang <mspang@uwaterloo.ca> writes:

Show 7 quoted lines

> Signed-off-by: Michael Spang <mspang@uwaterloo.ca>
> ---
>
> Whoops.
>
>  t/t7300-clean.sh |    9 +++++----
>  1 files changed, 5 insertions(+), 4 deletions(-)

Oops indeed.

* This is not an "amend"; you are following up an earlier patch of
  your own.  Marking such a patch as "amend" is only confusing.
  Please do not do it.

Show 7 quoted lines

>  test_expect_failure \
>      'clean.requireForce' \
> -    "mkdir -p build docs &&
> -    touch a.out src/part3.c docs/manual.txt obj.o build/lib.so &&
> -    git-config clean.requireForce true &&
> +    "git-config clean.requireForce true &&
>      git-clean"

* Please do not do this.  test_expect_failure, unless used for a
  very simple single test, is almost always a bug.  You would
  not know which step of && chain failed, even though you may be
  expecting the failure from the last one.

* I do not think a "this reformats every existing tests" patch
  is needed nor wanted, but at least I'd like to see new scripts
  and updates to the existing ones to be consistently formatted
  like this:

	test_expect_success 'name of the test' '

		test body goes here &&
                like this &&
		and this