Re: [PATCH 2/2] Support Out-Of-Tree Valgrind Tests

dag@cray.com (David A. Greene) writes:

Thomas Rast [off-list ref] writes:

quoted

greened@obbligato.org writes:

quoted

+# GIT_VALGRIND_TOOLS is the location of tools like valgrind.sh.
+if test -z "$GIT_VALGRIND_TOOLS"
+then
+	# We allow tests to override this, in case they want to run tests
+	# outside of t/.
+ 
+        # For in-tree test scripts, this is in TEST_DIRECTORY/valgrind
+        # (t/valgrind), but a test script that lives outside t/ can
+        # set this variable to point at the right place so that it can
+        # find t/valgrind directory that house test helpers like
+        # valgrind.sh.
+       GIT_VALGRIND_TOOLS="$TEST_DIRECTORY"/valgrind
+fi

I'm a bit curious: why isn't it enough to spell that path
$GIT_BUILD_DIR/t/valgrind instead of making it fully configurable?

For the same reason that TEST_DIRECTORY is different and unrelated from
GIT_BUILD_DIR.  It's my understanding that GIT_BUILD_DIR could end up
being somewhere compeltely unrelated to where TOP_SRC/t/valgrind is.
At least that's why I introduced a new parameter.

I'm just worried that for such a fringe use-case, the maintainer of the
out-of-tree tests will never notice that he missed to customize *this*
particular parameter.  So I'd rather have it spelled in terms of the
existing two (?).

Don't we, right now, get stuff as follows:

  item                   path
  --------------------------------------------
  test-lib.sh            $TEST_DIRECTORY
  git                    $GIT_BUILD_DIR/bin-wrappers
  valgrind.sh            $TEST_DIRECTORY/valgrind
  git (with --valgrind)  $TEST_DIRECTORY/valgrind/bin

You are saying this must change to an entirely new path

  valgrind.sh            $GIT_VALGRIND_TOOLS
  git (with --valgrind)  $GIT_VALGRIND_TOOLS/bin

but what's wrong with simply

  valgrind.sh            $GIT_BUILD_DIR/t/valgrind
  git (with --valgrind)  $TEST_DIRECTORY/valgrind/bin

In the common case of t/, these just map to what we had before.  In the
out-of-tree case, we'd create valgrind/bin in the test directory for the
*temporary* stuff, and still look for the wrapping valgrind.sh in the
git tree.

-- 
Thomas Rast
trast@{inf,student}.ethz.ch