Re: [PATCH 4/3] t/Makefile: introduce TEST_SHELL_PATH

From: Jeff King <peff@peff.net>
Date: Oct 25, 2017, 21:50 UTC
Message-ID: <20171025215015.irnyvk7kvmxqkniy@sigill.intra.peff.net>
In-Reply-To: <alpine.DEB.2.21.1.1710252334000.6482@virtualbox>

On Wed, Oct 25, 2017 at 11:35:44PM +0200, Johannes Schindelin wrote:

Show 18 quoted lines

> > > Or alternatively we could prefix the assignment by
> > > 
> > > 	test -n "$TEST_SHELL_PATH" ||
> > > 
> > > or use the pattern
> > > 
> > > 	TEST_SHELL_PATH="${TEST_SHELL_PATH:-[...]}"
> > 
> > I'm not quite sure what this is fixing.  Is there a case where we
> > wouldn't have TEST_SHELL_PATH set when running the tests? I think there
> > are already other bits that assume that "make" has been run (including
> > the existing reference to $SHELL_PATH, I think).
> 
> The way I read your patch, setting the environment variable differnently
> at test time than at build time would be ignored: GIT-BUILD-OPTIONS is
> sourced and would override whatever you told the test suite to use.
> 
> I guess it does not really matter all that much in practice.

Right. I find that behavior mildly irritating at times, but it's consistent with other items like NO_PERL, etc. E.g., you cannot do:

  make NO_PERL=
  cd t
  NO_PERL=Nope ./t3701-*

and disable perl. It's testing what got built.

-Peff