Re: [PATCH] test-lib: do not remove trash_directory if called with --debug

From: Jeff King <peff@peff.net>
Date: Aug 21, 2008, 12:55 UTC
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:

Show 6 quoted lines

> 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