[RFC] Support TEST_GIT_PATH variable for the path for the git to test

[RFC] Support TEST_GIT_PATH variable for the path for the git to test

[Daniel Barkalow]

This is useful if you want to see how some other version of git
handles the tests for some reason. (For example, see how a working
version manages to work while a broken one doesn't)

This version only sort of works, but it's enough to have been helpful in 
debugging builtin-clone, and most of what's left I don't really understand 
at all (the perl code, for example).

Signed-off-by: Daniel Barkalow <barkalow@iabervon.org>
---
 t/test-lib.sh |    7 ++++---
 1 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/t/test-lib.sh b/t/test-lib.sh
index 83889c4..5300549 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -281,7 +281,8 @@ test_create_repo () {
 	cd "$repo" || error "Cannot setup test environment"
 	"$GIT_EXEC_PATH/git" init --template=$GIT_EXEC_PATH/templates/blt/ >/dev/null 2>&1 ||
 	error "cannot run git init -- have you built things yet?"
-	mv .git/hooks .git/hooks-disabled
+	[ ! -e .git/hooks ] || mv .git/hooks .git/hooks-disabled
+	[ -e .git/info ] || mkdir .git/info
 	cd "$owd"
 }
 
@@ -321,8 +322,8 @@ test_done () {
 
 # Test the binaries we have just built.  The tests are kept in
 # t/ subdirectory and are run in trash subdirectory.
-PATH=$(pwd)/..:$PATH
-GIT_EXEC_PATH=$(pwd)/..
+GIT_EXEC_PATH=${TEST_GIT_PATH:-$(pwd)/..}
+PATH=$GIT_EXEC_PATH:$(pwd)/..:$PATH
 GIT_TEMPLATE_DIR=$(pwd)/../templates/blt
 unset GIT_CONFIG
 unset GIT_CONFIG_LOCAL
-- 
1.5.4.2.261.g851a5.dirty

Re: [RFC] Support TEST_GIT_PATH variable for the path for the git to test

[Junio C Hamano]

Daniel Barkalow <barkalow@iabervon.org> writes:

> @@ -281,7 +281,8 @@ test_create_repo () {
>  	cd "$repo" || error "Cannot setup test environment"
>  	"$GIT_EXEC_PATH/git" init --template=$GIT_EXEC_PATH/templates/blt/ >/dev/null 2>&1 ||
>  	error "cannot run git init -- have you built things yet?"
> -	mv .git/hooks .git/hooks-disabled
> +	[ ! -e .git/hooks ] || mv .git/hooks .git/hooks-disabled
> +	[ -e .git/info ] || mkdir .git/info

This is just a style issue, but if you try to be old fashioned,
please say "test frotz || xyzzy".  If you prefer to be modern,
"if ! test frotz; then xyzzy; fi".  I'd prefer the former, but
in either case I really do not want to see [ ... ] that do not
make anything more readable.

Also we tend to avoid "test -e" unless absolutely needed.

> @@ -321,8 +322,8 @@ test_done () {
>  
>  # Test the binaries we have just built.  The tests are kept in
>  # t/ subdirectory and are run in trash subdirectory.
> -PATH=$(pwd)/..:$PATH
> -GIT_EXEC_PATH=$(pwd)/..
> +GIT_EXEC_PATH=${TEST_GIT_PATH:-$(pwd)/..}
> +PATH=$GIT_EXEC_PATH:$(pwd)/..:$PATH

Hmmmm.

I have bunch of gits installed under $HOME/git-vX.Y.Z/bin and
when I need to test one from a different vintage, I just say:

	PATH=$HOME/git-vX.Y.Z/bin:/usr/bin:/bin
	... do git stuff which all use version X.Y.Z

and have $HOME/git-vX.Y.Z/bin/git find its corresponding friends
on the GIT_EXEC_PATH embedded in it.  Because you are interested
in testing installed versions, I suspect something like:

	if test -z "$TEST_GIT_ON_PATH"
	then
                GIT_EXEC_PATH=$(pwd)/..
        	PATH=$GIT_EXEC_PATH:$PATH
	else
        	: We do not do any customization
	fi

would be easier to read and more to the point.  Perhaps the
tester even has his own GIT_EXEC_PATH that is unrelated to
TEST_GIT_PATH.

Re: [RFC] Support TEST_GIT_PATH variable for the path for the git to test

[Daniel Barkalow]

On Mon, 25 Feb 2008, Junio C Hamano wrote:

> Daniel Barkalow <barkalow@iabervon.org> writes:
> 
> > @@ -281,7 +281,8 @@ test_create_repo () {
> >  	cd "$repo" || error "Cannot setup test environment"
> >  	"$GIT_EXEC_PATH/git" init --template=$GIT_EXEC_PATH/templates/blt/ >/dev/null 2>&1 ||
> >  	error "cannot run git init -- have you built things yet?"
> > -	mv .git/hooks .git/hooks-disabled
> > +	[ ! -e .git/hooks ] || mv .git/hooks .git/hooks-disabled
> > +	[ -e .git/info ] || mkdir .git/info
> 
> This is just a style issue, but if you try to be old fashioned,
> please say "test frotz || xyzzy".  If you prefer to be modern,
> "if ! test frotz; then xyzzy; fi".  I'd prefer the former, but
> in either case I really do not want to see [ ... ] that do not
> make anything more readable.
> 
> Also we tend to avoid "test -e" unless absolutely needed.

This is supposed to be: don't complain if .git/hooks already doesn't 
exist; also, create .git/info if it doesn't exist. What is the right style 
for that? I've been only reading git shell code and writing C, other than 
this little bit.

> > @@ -321,8 +322,8 @@ test_done () {
> >  
> >  # Test the binaries we have just built.  The tests are kept in
> >  # t/ subdirectory and are run in trash subdirectory.
> > -PATH=$(pwd)/..:$PATH
> > -GIT_EXEC_PATH=$(pwd)/..
> > +GIT_EXEC_PATH=${TEST_GIT_PATH:-$(pwd)/..}
> > +PATH=$GIT_EXEC_PATH:$(pwd)/..:$PATH
> 
> Hmmmm.
> 
> I have bunch of gits installed under $HOME/git-vX.Y.Z/bin and
> when I need to test one from a different vintage, I just say:
> 
> 	PATH=$HOME/git-vX.Y.Z/bin:/usr/bin:/bin
> 	... do git stuff which all use version X.Y.Z
> 
> and have $HOME/git-vX.Y.Z/bin/git find its corresponding friends
> on the GIT_EXEC_PATH embedded in it.  Because you are interested
> in testing installed versions, I suspect something like:
> 
> 	if test -z "$TEST_GIT_ON_PATH"
> 	then
>                 GIT_EXEC_PATH=$(pwd)/..
>         	PATH=$GIT_EXEC_PATH:$PATH
> 	else
>         	: We do not do any customization
> 	fi
> 
> would be easier to read and more to the point.  Perhaps the
> tester even has his own GIT_EXEC_PATH that is unrelated to
> TEST_GIT_PATH.

That doesn't make test_create_repo() work right. Is there some reason it's 
currently using it with an explicit path, or is that just a legacy of when 
it wasn't running the git that gets tested in general? With that 
simplified, it seems to work this way, I think, and that's almost 
certainly the right thing to do.

There's additionally the problem that things which are built for testing 
in the git directory won't be installed anywhere. One possibility would be 
to put $(pwd)/.. at the end of the PATH (instead of the start), but I 
think it would be cleanest to put the built test tools somewhere else and 
include that directory in the PATH no matter what. Does this sound like 
the right track?

	-Daniel
*This .sig left intentionally blank*

Re: [RFC] Support TEST_GIT_PATH variable for the path for the git to test

[Junio C Hamano]

Daniel Barkalow <barkalow@iabervon.org> writes:

> On Mon, 25 Feb 2008, Junio C Hamano wrote:
> ...
>> Also we tend to avoid "test -e" unless absolutely needed.
>
> This is supposed to be: don't complain if .git/hooks already doesn't 
> exist; also, create .git/info if it doesn't exist. What is the right style 
> for that? I've been only reading git shell code and writing C, other than 
> this little bit.

What I meant to say was we have tried to stick to "test -d" or
"test -f" and avoided a newer "test -e" when able.  It's not a
style but more about courtesy to porters.  Majority of platforms
do not care.

>> I have bunch of gits installed under $HOME/git-vX.Y.Z/bin and
>> when I need to test one from a different vintage, I just say:
>> 
>> 	PATH=$HOME/git-vX.Y.Z/bin:/usr/bin:/bin
>> 	... do git stuff which all use version X.Y.Z
>> 
>> and have $HOME/git-vX.Y.Z/bin/git find its corresponding friends
>> on the GIT_EXEC_PATH embedded in it.  Because you are interested
>> in testing installed versions, I suspect something like:
>> ...
>> would be easier to read and more to the point....
>
> That doesn't make test_create_repo() work right.

I did not mean to imply I was presenting the whole solution; I
was trying to hint at a different direction which may or may not
work.  I did not look at what test_create_repo() actually did
when I wrote the message, but you are right.  It too needs to
be made conditional, and when trying an installed version, it
should not do the "template" dance but let the installed "git init"
figure it out.

> There's additionally the problem that things which are built for testing 
> in the git directory won't be installed anywhere.

To test without installing, in order to make them call out their
siblings, I have this piece that I source to a new shell:

        GIT_EXEC_PATH=`pwd`
        PATH=`pwd`:/usr/bin:/bin
        GITPERLLIB=`pwd`/perl/blib/lib
        export GIT_EXEC_PATH PATH GITPERLLIB

(and this is not enough if what is being tested is a change to
the templates, but that is rare so I haven't bothered).  We
could do something similar and tell the test scripts where that
directory is via GIT_TEST_SOURCE_DIR perhaps, if we were to
support this third variant?

I originally thought that the problem you are trying to solve
was more like "My WIP does not work well with the tests yet, but
how does the one from my distro sitting in /usr/bin/git (and
calling /usr/bin/git-clone when told to 'git clone') work on
them?" (i.e. as opposed to "testing the one we just built here",
which our test scripts are about originally, "testing an
installed version" --- let's call that the second variant).

And for that, not setting nor exporting is the right way (you
run tests as if they are just a random set of user scripts that
happen to call "git" as intended).  

If you want to test git that you just have built, you arrange
that your use of "git-foo" and "git foo" both refer to what's in
$(pwd) and that is something t/test-lib.sh already knows how to
take care of.  The third variant of testing git that you built
elsewhere else should be easier to implement by borrowing the
current setup, just replacing $(pwd) with $elsewhere, I would
imagine.  But that depends on the target git version --- it may
have different rules and expect different settings, so we cannot
have a perfectly futureproof support for this kind of testing.

We would ideally want both (installed vs potentially different
vintage built elsewhere), but they have different requirements.
The latter wants more than just $PATH tweaked, while the former
do not want anything to be tweaked but use the environment the
user has as-is.  Maybe the user is always running with a strange
$GIT_EXEC_PATH and he wants to test _that_ configuration.

So how about making these two separate changes?

 (1) a patch that lets you say "We are going to test a built but
     not installed git, but that build directory is not in this
     tree" (i.e. the third variant) with TEST_GIT_SOURCE_TREE.
     When unset, we will test a built but not installed git here
     in this tree.  I would imagine it would mostly be the
     matter of replacing use of $(pwd) to specify where the
     built stuff is, with $TEST_GIT_SOURCE_TREE (use of
     GIT_EXEC_PATH to find template in test_create_repo needs to
     be fixed);

 (2) a patch that lets you say "Test what is installed on my
     $PATH using my $GIT_EXEC_PATH and friends if they exist.
     Do not muck with my environment", with TEST_GIT_INSTALLED
     or something.

Re: [RFC] Support TEST_GIT_PATH variable for the path for the git to test

[Daniel Barkalow]

On Tue, 26 Feb 2008, Junio C Hamano wrote:

> Daniel Barkalow <barkalow@iabervon.org> writes:
> 
> > On Mon, 25 Feb 2008, Junio C Hamano wrote:
> > ...
> >> Also we tend to avoid "test -e" unless absolutely needed.
> >
> > This is supposed to be: don't complain if .git/hooks already doesn't 
> > exist; also, create .git/info if it doesn't exist. What is the right style 
> > for that? I've been only reading git shell code and writing C, other than 
> > this little bit.
> 
> What I meant to say was we have tried to stick to "test -d" or
> "test -f" and avoided a newer "test -e" when able.  It's not a
> style but more about courtesy to porters.  Majority of platforms
> do not care.

Ah, okay. I didn't know what tests were the more available ones.

> >> I have bunch of gits installed under $HOME/git-vX.Y.Z/bin and
> >> when I need to test one from a different vintage, I just say:
> >> 
> >> 	PATH=$HOME/git-vX.Y.Z/bin:/usr/bin:/bin
> >> 	... do git stuff which all use version X.Y.Z
> >> 
> >> and have $HOME/git-vX.Y.Z/bin/git find its corresponding friends
> >> on the GIT_EXEC_PATH embedded in it.  Because you are interested
> >> in testing installed versions, I suspect something like:
> >> ...
> >> would be easier to read and more to the point....
> >
> > That doesn't make test_create_repo() work right.
> 
> I did not mean to imply I was presenting the whole solution; I
> was trying to hint at a different direction which may or may not
> work.  I did not look at what test_create_repo() actually did
> when I wrote the message, but you are right.  It too needs to
> be made conditional, and when trying an installed version, it
> should not do the "template" dance but let the installed "git init"
> figure it out.

Does it need to be conditional, or is simply "git init" right (where we've 
already set environment variables for the local stuff if applicable)?

> > There's additionally the problem that things which are built for testing 
> > in the git directory won't be installed anywhere.
> 
> To test without installing, in order to make them call out their
> siblings, I have this piece that I source to a new shell:
> 
>         GIT_EXEC_PATH=`pwd`
>         PATH=`pwd`:/usr/bin:/bin
>         GITPERLLIB=`pwd`/perl/blib/lib
>         export GIT_EXEC_PATH PATH GITPERLLIB

Might be nice to have a "sgitpath" (on the model of sg or su) in t/, since 
this is handy in general.

> (and this is not enough if what is being tested is a change to
> the templates, but that is rare so I haven't bothered).  We
> could do something similar and tell the test scripts where that
> directory is via GIT_TEST_SOURCE_DIR perhaps, if we were to
> support this third variant?
> 
> I originally thought that the problem you are trying to solve
> was more like "My WIP does not work well with the tests yet, but
> how does the one from my distro sitting in /usr/bin/git (and
> calling /usr/bin/git-clone when told to 'git clone') work on
> them?" (i.e. as opposed to "testing the one we just built here",
> which our test scripts are about originally, "testing an
> installed version" --- let's call that the second variant).

You understood correctly the first time; I was unclear the second time. 
The problem here is that, in order to run tests, we call 
"test-absolute-path", "test-genrandom", etc., and we can't use these from 
the user's $PATH because, being only for testing, they don't get installed 
there. We need to get git-remote (for example) from $PATH, but 
test-genrandom from $(pwd)/.. in order to make the tests run.

> And for that, not setting nor exporting is the right way (you
> run tests as if they are just a random set of user scripts that
> happen to call "git" as intended).  

Right, except that we use a couple of helpers which we've built in 
$(pwd)/.. that are effectively part of the tests, but are inconveniently 
in the same directory with programs we want to not use. That's the issue 
with not adding anything PATH-related.

> If you want to test git that you just have built, you arrange
> that your use of "git-foo" and "git foo" both refer to what's in
> $(pwd) and that is something t/test-lib.sh already knows how to
> take care of.  The third variant of testing git that you built
> elsewhere else should be easier to implement by borrowing the
> current setup, just replacing $(pwd) with $elsewhere, I would
> imagine.  But that depends on the target git version --- it may
> have different rules and expect different settings, so we cannot
> have a perfectly futureproof support for this kind of testing.

This would, in fact, be useful for the case where you've got one tree with 
some new tests from somebody else and another tree with some changes 
you've made yourself, and you want to know what effect your changes have 
on those tests. (For example, does builtin-clone support bundles?)

> We would ideally want both (installed vs potentially different
> vintage built elsewhere), but they have different requirements.
> The latter wants more than just $PATH tweaked, while the former
> do not want anything to be tweaked but use the environment the
> user has as-is.  Maybe the user is always running with a strange
> $GIT_EXEC_PATH and he wants to test _that_ configuration.
> 
> So how about making these two separate changes?
> 
>  (1) a patch that lets you say "We are going to test a built but
>      not installed git, but that build directory is not in this
>      tree" (i.e. the third variant) with TEST_GIT_SOURCE_TREE.
>      When unset, we will test a built but not installed git here
>      in this tree.  I would imagine it would mostly be the
>      matter of replacing use of $(pwd) to specify where the
>      built stuff is, with $TEST_GIT_SOURCE_TREE (use of
>      GIT_EXEC_PATH to find template in test_create_repo needs to
>      be fixed);
> 
>  (2) a patch that lets you say "Test what is installed on my
>      $PATH using my $GIT_EXEC_PATH and friends if they exist.
>      Do not muck with my environment", with TEST_GIT_INSTALLED
>      or something.

That sounds good (although only (2) is on my current path), but I also 
need a prerequisite patch for both, I believe, for simply making test 
helpers available independant of actual build products.

	-Daniel
*This .sig left intentionally blank*

Re: [RFC] Support TEST_GIT_PATH variable for the path for the git to test

[Junio C Hamano]

Daniel Barkalow <barkalow@iabervon.org> writes:

>> I did not mean to imply I was presenting the whole solution; I
>> was trying to hint at a different direction which may or may not
>> work.  I did not look at what test_create_repo() actually did
>> when I wrote the message, but you are right.  It too needs to
>> be made conditional, and when trying an installed version, it
>> should not do the "template" dance but let the installed "git init"
>> figure it out.
>
> Does it need to be conditional, or is simply "git init" right (where we've 
> already set environment variables for the local stuff if applicable)?

I suspect "uninstalled" cases (both "here" and "elsewhere")
needs to do the template magic, while "installed" case should be
just "git init".

>> > There's additionally the problem that things which are built for testing 
>> > in the git directory won't be installed anywhere.
>> 
>> To test without installing, in order to make them call out their
>> siblings, I have this piece that I source to a new shell:
>> 
>>         GIT_EXEC_PATH=`pwd`
>>         PATH=`pwd`:/usr/bin:/bin
>>         GITPERLLIB=`pwd`/perl/blib/lib
>>         export GIT_EXEC_PATH PATH GITPERLLIB
>
> Might be nice to have a "sgitpath" (on the model of sg or su) in t/, since 
> this is handy in general.

I have them in ./+denv and after building if I want to see it
work in real repo elsewhere I do:

	: gitster; sh
        $ . ./+denv
        $ cd ../elsewhere.git
        $ do the real workload trial
        $ ^D

> You understood correctly the first time; I was unclear the second time. 
> The problem here is that, in order to run tests, we call 
> "test-absolute-path", "test-genrandom", etc., and we can't use these from 
> the user's $PATH because, being only for testing, they don't get installed 
> there. We need to get git-remote (for example) from $PATH, but 
> test-genrandom from $(pwd)/.. in order to make the tests run.

Ok, so even "installed" case need to rely on "test-blah" we build.

> That sounds good (although only (2) is on my current path), but I also 
> need a prerequisite patch for both, I believe, for simply making test 
> helpers available independant of actual build products.

Yeah, sounds like it.

Re: [RFC] Support TEST_GIT_PATH variable for the path for the git to test

[Daniel Barkalow]

On Tue, 26 Feb 2008, Junio C Hamano wrote:

> Daniel Barkalow <barkalow@iabervon.org> writes:
> 
> >> I did not mean to imply I was presenting the whole solution; I
> >> was trying to hint at a different direction which may or may not
> >> work.  I did not look at what test_create_repo() actually did
> >> when I wrote the message, but you are right.  It too needs to
> >> be made conditional, and when trying an installed version, it
> >> should not do the "template" dance but let the installed "git init"
> >> figure it out.
> >
> > Does it need to be conditional, or is simply "git init" right (where we've 
> > already set environment variables for the local stuff if applicable)?
> 
> I suspect "uninstalled" cases (both "here" and "elsewhere")
> needs to do the template magic, while "installed" case should be
> just "git init".

Ah, but that doesn't have to be here, since we can (and, AFAICT do) use 
the GIT_TEMPLATE_DIR environment variable, and can set it as appropriate 
in the setup code.

> >> > There's additionally the problem that things which are built for testing 
> >> > in the git directory won't be installed anywhere.
> >> 
> >> To test without installing, in order to make them call out their
> >> siblings, I have this piece that I source to a new shell:
> >> 
> >>         GIT_EXEC_PATH=`pwd`
> >>         PATH=`pwd`:/usr/bin:/bin
> >>         GITPERLLIB=`pwd`/perl/blib/lib
> >>         export GIT_EXEC_PATH PATH GITPERLLIB
> >
> > Might be nice to have a "sgitpath" (on the model of sg or su) in t/, since 
> > this is handy in general.
> 
> I have them in ./+denv and after building if I want to see it
> work in real repo elsewhere I do:
> 
> 	: gitster; sh
>         $ . ./+denv
>         $ cd ../elsewhere.git
>         $ do the real workload trial
>         $ ^D

I was thinking it would be handy to have around t/ in case you want to go 
step-by-step through a failing test, using the git the test would use. For 
that purpose, it would also be handy to pick up the environment variables 
I'm introducing in this thread, I guess.

> > You understood correctly the first time; I was unclear the second time. 
> > The problem here is that, in order to run tests, we call 
> > "test-absolute-path", "test-genrandom", etc., and we can't use these from 
> > the user's $PATH because, being only for testing, they don't get installed 
> > there. We need to get git-remote (for example) from $PATH, but 
> > test-genrandom from $(pwd)/.. in order to make the tests run.
> 
> Ok, so even "installed" case need to rely on "test-blah" we build.

Right. Would it be okay to build those into t/helpers/* or something, 
instead of into the project root, so that they're just naturally separate 
from the actual program?

	-Daniel
*This .sig left intentionally blank*

Re: [RFC] Support TEST_GIT_PATH variable for the path for the git to test

[Junio C Hamano]

Daniel Barkalow <barkalow@iabervon.org> writes:

>> Ok, so even "installed" case need to rely on "test-blah" we build.
>
> Right. Would it be okay to build those into t/helpers/* or something, 
> instead of into the project root, so that they're just naturally separate 
> from the actual program?

Yeah, it sounds like a plan.