From: Jeff King <peff@peff.net>
Date: Thu, 21 Aug 2008 12:55:17 GMT
Subject: Re: [PATCH] test-lib: do not remove trash_directory if called with --debug
Message-ID: <20080821125516.GA22453@coredump.intra.peff.net>
In-Reply-To: <7vy72qiwc0.fsf@gitster.siamese.dyndns.org>

On Thu, Aug 21, 2008 at 01:52:31AM -0700, Junio C Hamano wrote:

> Yeah, I think hooking this with --debug is makes actual sense.  Much saner
> than changing the behaviour magically with "make -j".
> 
> Even though I personally do not particularly like having an _easy_ way to
> leave the cruft around, I'll apply it if Jeff and other people like this
> behaviour.

I think the workflow you described is probably a better one for adding a
test, and I'll try to adjust to that. However, I do think Dscho's patch
makes sense for any time you simply want to look at the output of a
successful test (e.g., even if you aren't editing it).

My only concern with the patch is that it conditionally sets
$remove_trash; if debug is set, that variable is not set at all. Yet
later we look at it and "rm -rf" it. Obviously the chances of somebody
accidentally passing in such a variable are quite low, but style-wise
(and given that we are calling rm -rf!), I think I would prefer:

  if ! test -z "$debug"; then
    remove_trash="$TEST_DIRECTORY/$test"
  else
    remove_trash=
  fi

And a final comment on leaving cruft around: this is something that
developers have to deal with _anyway_, when tests fail or when we break
out of the test scripts with ^C. It eventually gets cleaned when the
test runs successfully (or one runs "git clean -xd").

-Peff