All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jonathan Nieder <jrnieder@gmail.com>
To: Jeff King <peff@peff.net>
Cc: git@vger.kernel.org, John Keeping <john@keeping.me.uk>,
	Thomas Rast <tr@thomasrast.ch>
Subject: Re: [PATCH 1/3] t0000: set TEST_OUTPUT_DIRECTORY for sub-tests
Date: Sat, 28 Dec 2013 14:13:13 -0800	[thread overview]
Message-ID: <20131228221313.GB5544@google.com> (raw)
In-Reply-To: <20131228092915.GA21109@sigill.intra.peff.net>

Hi,

Jeff King wrote:

> Once upon a time, the test-lib library would create trash
> directories in the current working directory, unless we were
> explicitly told to put it elsewhere via --root. As a result,
> t0000 created the sub-test trash directories inside its own
> trash directory.
>
> However, we noticed that this did not cover all cases, since
> we would need to respect $TEST_OUTPUT_DIRECTORY even if
> --root is not given (or is relative). Commit 38b074d fixed
> this to consistently use the full path.

So the idea if I am reading correctly is "Instead of relying on the
implicit output directory chosen with chdir, which doesn't even work
any more, set TEST_OUTPUT_DIRECTORY to decide where output for the
sub-tests used by t0000's sanity checks for the test harness go".

I'm not sure I completely understand the regression caused by 38b074d.
Is the idea that before that commit, TEST_OUTPUT_DIRECTORY was only
used for the test-results/ directory so the only harm done was some
mixing of test results?

What is the symptom this patch alleviates?

> As a result, t0000's sub-tests are now created in git's
> original test output directory rather than in our trash
> directory.

This might be the source of my confusion.  Is "sub-tests" an
abbreviation for "sub-test trash directories" here?

>            Furthermore, since some of the sub-tests simulate
> failures, the trash directories do not get cleaned up, and
> the cruft is left in the t/ directory.
>
> We could fix this by passing a new "--root=$TRASH_DIRECTORY"
> option to the sub-test. However, we do not want the sub-tests
> to write anything at all to git's directory (e.g., they
> should not be writing to t/test-results, either, although
> this is already handled by separate code).

Ah, HARNESS_ACTIVE prevents output of test-results.

Does the git test harness write something else to
TEST_OUTPUT_DIRECTORY?  Is the idea that using --root would be
functionally equivalent but (1) more confusing and (2) less
futureproof?

>                                            So the best
> solution is to simply reset $TEST_OUTPUT_DIRECTORY entirely
> in the sub-test, which covers this case, as well as any
> future ones.

So, to sum up: if I understand correctly

 - git used to only use TEST_OUTPUT_DIRECTORY to decide where test
   results go.  You'd have to use --root to set a custom location for
   trash directories.

 - in that old setup, t0000 leaves around extra trash directories with
   --root, since the sub-tests inherit the parent test's $root and put
   trash directories there.

 - after 38b074d, that old problem still exists and furthermore
   t0000 leaves around extra trash directories even when --root is not
   in use, since the sub-tests inherit the value of
   TEST_OUTPUT_DIRECTORY from the parent test.

 - this patch fixes the TEST_OUTPUT_DIRECTORY problem (but not the $root
   problem) by setting TEST_OUTPUT_DIRECTORY explicitly

Does that sound right?  If so, should sub-tests unset $root, too?

Thanks and hope that helps,
Jonathan

  reply	other threads:[~2013-12-28 22:13 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-12-28  9:27 [PATCH 0/3] t0000 cleanups Jeff King
2013-12-28  9:29 ` [PATCH 1/3] t0000: set TEST_OUTPUT_DIRECTORY for sub-tests Jeff King
2013-12-28 22:13   ` Jonathan Nieder [this message]
2013-12-28 22:20     ` Jonathan Nieder
2013-12-29  7:17     ` Jeff King
2013-12-28  9:31 ` [PATCH 2/3] t0000: simplify HARNESS_ACTIVE hack Jeff King
2013-12-28 22:14   ` Jonathan Nieder
2013-12-28  9:33 ` [PATCH 3/3] t0000: drop "known breakage" test Jeff King
2013-12-28 20:51   ` Jonathan Nieder
2013-12-29  7:22     ` Jeff King
2013-12-28 22:21 ` [PATCH 0/3] t0000 cleanups Jonathan Nieder
2013-12-30 18:30   ` Junio C Hamano
2013-12-30 18:51     ` Jonathan Nieder
2013-12-30 19:24       ` Junio C Hamano
2013-12-31 10:33       ` Jeff King
2014-01-02 22:28         ` Jonathan Nieder
2014-01-02 22:41           ` Junio C Hamano
2014-01-03  1:04           ` Jeff King

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20131228221313.GB5544@google.com \
    --to=jrnieder@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=john@keeping.me.uk \
    --cc=peff@peff.net \
    --cc=tr@thomasrast.ch \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.