Re: [PATCH 4/5] tests: do not require Git to be built when testing an installed Git

From: Johannes Schindelin <johannes.schindelin@gmx.de>
Date: Nov 14, 2018, 13:20 UTC
Message-ID: <nycvar.QRO.7.76.6.1811141417160.39@tvgsbejvaqbjf.bet>
In-Reply-To: <xmqqa7mcp80u.fsf@gitster-ct.c.googlers.com>

Hi Junio,

On Wed, 14 Nov 2018, Junio C Hamano wrote:

Show 11 quoted lines

> "Johannes Schindelin via GitGitGadget" <gitgitgadget@gmail.com>
> writes:
> 
> > From: Johannes Schindelin <johannes.schindelin@gmx.de>
> >
> > We really only need the test helpers in that case, but that is not what
> > we test for. So let's skip the test for now when we know that we want to
> > test an installed Git.
> 
> True, but...  hopefully we are making sure t/helpers/ has been built
> in some other ways, though, right?

We do it implicitly, in the test cases that use the helpers.

However, t/test-lib.sh does not particularly verify that the test helpers have been built.

And I think that is a good thing: we do have several test scripts, I would think, that do not require any test helper to begin with. These scripts can work pretty well without having to build anything (read: on a machine where there is not even a toolchain available to build things).

Besides, it is pretty much only t/helper/test-tool these days, and it is unlikely that anybody has a `test-tool` in their `PATH`. If they do, they'll find out soon enough iff they test with `GIT_TEST_INSTALLED` and iff they do not have the test helper(s) in t/helper/ ;-)

Ciao, Dscho

Show 25 quoted lines

> I do not offhand see how tests would work in a pristine checkout with
> "cd t && sh t0000-*.sh" as t/test-lib.sh is not running $(MAKE) itself
> (and the design of the test-lib.sh, as can be seen in the check this
> part of it makes, is to just abort if we cannot test) with this patch
> (and PATCH 1/5) under the test-installed mode, though.
> 
> > Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> > ---
> >  t/test-lib.sh | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/t/test-lib.sh b/t/test-lib.sh
> > index 832ede5099..1ea20dc2dc 100644
> > --- a/t/test-lib.sh
> > +++ b/t/test-lib.sh
> > @@ -51,7 +51,7 @@ export LSAN_OPTIONS
> >  
> >  ################################################################
> >  # It appears that people try to run tests without building...
> > -"$GIT_BUILD_DIR/git" >/dev/null
> > +test -n "$GIT_TEST_INSTALLED" || "$GIT_BUILD_DIR/git" >/dev/null ||
> >  if test $? != 1
> >  then
> >  	echo >&2 'error: you do not seem to have built git yet.'
>