[RFC PATCH] tests: fix PATH for GIT_TEST_INSTALLED tests

[RFC PATCH] tests: fix PATH for GIT_TEST_INSTALLED tests

[Guillaume Maudoux]

From: Guillaume Maudoux <layus.on@gmail.com>

When running tests on an existing git installation with
GIT_TEST_INSTALLED (as described in t/README), the test helpers are
missing in the PATH.

This fixes the test suite in a way that allows all the tests to pass.

Signed-off-by: Guillaume Maudoux <layus.on@gmail.com>
---

This is more a bug report than a real patch. The issue is described
above and this patch does solve it. I however think that someone with
more knowledge should refactor all that chunck of code that was last
changed in 2010.

In particular, it seems that the GIT_TEST_INSTALLED path does not use
bin-wrappers at all. This may imply that --with-dashes also breaks
tests.

 t/test-lib.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git t/test-lib.sh t/test-lib.sh
index 7740d511d..0d51261f7 100644
--- t/test-lib.sh
+++ t/test-lib.sh
@@ -923,7 +923,7 @@ elif test -n "$GIT_TEST_INSTALLED"
 then
 	GIT_EXEC_PATH=$($GIT_TEST_INSTALLED/git --exec-path)  ||
 	error "Cannot run git from $GIT_TEST_INSTALLED."
-	PATH=$GIT_TEST_INSTALLED:$GIT_BUILD_DIR:$PATH
+	PATH=$GIT_TEST_INSTALLED:$GIT_BUILD_DIR/t/helper:$GIT_BUILD_DIR:$PATH
 	GIT_EXEC_PATH=${GIT_TEST_EXEC_PATH:-$GIT_EXEC_PATH}
 else # normal case, use ../bin-wrappers only unless $with_dashes:
 	git_bin_dir="$GIT_BUILD_DIR/bin-wrappers"
-- 
2.17.0

Re: [RFC PATCH] tests: fix PATH for GIT_TEST_INSTALLED tests

[Johannes Schindelin]

Hi Guillaume,

On Sat, 14 Apr 2018, Guillaume Maudoux wrote:

> From: Guillaume Maudoux <layus.on@gmail.com>
> 
> When running tests on an existing git installation with
> GIT_TEST_INSTALLED (as described in t/README), the test helpers are
> missing in the PATH.
> 
> This fixes the test suite in a way that allows all the tests to pass.
> 
> Signed-off-by: Guillaume Maudoux <layus.on@gmail.com>
> ---
> 
> This is more a bug report than a real patch. The issue is described
> above and this patch does solve it. I however think that someone with
> more knowledge should refactor all that chunck of code that was last
> changed in 2010.
> 
> In particular, it seems that the GIT_TEST_INSTALLED path does not use
> bin-wrappers at all. This may imply that --with-dashes also breaks
> tests.
> 
>  t/test-lib.sh | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git t/test-lib.sh t/test-lib.sh
> index 7740d511d..0d51261f7 100644
> --- t/test-lib.sh
> +++ t/test-lib.sh
> @@ -923,7 +923,7 @@ elif test -n "$GIT_TEST_INSTALLED"
>  then
>  	GIT_EXEC_PATH=$($GIT_TEST_INSTALLED/git --exec-path)  ||
>  	error "Cannot run git from $GIT_TEST_INSTALLED."
> -	PATH=$GIT_TEST_INSTALLED:$GIT_BUILD_DIR:$PATH
> +	PATH=$GIT_TEST_INSTALLED:$GIT_BUILD_DIR/t/helper:$GIT_BUILD_DIR:$PATH
>  	GIT_EXEC_PATH=${GIT_TEST_EXEC_PATH:-$GIT_EXEC_PATH}
>  else # normal case, use ../bin-wrappers only unless $with_dashes:
>  	git_bin_dir="$GIT_BUILD_DIR/bin-wrappers"

This is essentially identical to what we have in

http://github.com/git-for-windows/git/commit/e408b7517d

So: ACK.

You might also want to go a bit further and let the test suite run with
GIT_TEST_INSTALLED when Git has not actually be built, but only the test
helpers. I started something along those lines here:

http://github.com/git-for-windows/git/commit/a80f047abc5

I always meant to come back to polish those patches and submit them to the
Git mailing list, so: thank you for getting the ball rolling.

FWIW my use case is that I want to test a "MinGit" package, i.e. a subset
of Git for Windows intended to cater to third-party applications requiring
Git functionality (but not requiring any interactive parts of it).

What is your use case?

Ciao,
Johannes

Re: [RFC PATCH] tests: fix PATH for GIT_TEST_INSTALLED tests

[Guillaume Maudoux (Layus)]

Hi Joannnes,

Le 14 avril 2018 14:30:38 UTC+02:00, Johannes Schindelin <Johannes.Schindelin@gmx.de> a écrit :
>Hi Guillaume,
>
>On Sat, 14 Apr 2018, Guillaume Maudoux wrote:
>
>> From: Guillaume Maudoux <layus.on@gmail.com>
>> 
>> When running tests on an existing git installation with
>> GIT_TEST_INSTALLED (as described in t/README), the test helpers are
>> missing in the PATH.
>> 
>> This fixes the test suite in a way that allows all the tests to pass.
>> 
>> Signed-off-by: Guillaume Maudoux <layus.on@gmail.com>
>> ---
>> 
>> This is more a bug report than a real patch. The issue is described
>> above and this patch does solve it. I however think that someone with
>> more knowledge should refactor all that chunck of code that was last
>> changed in 2010.
>> 
>> In particular, it seems that the GIT_TEST_INSTALLED path does not use
>> bin-wrappers at all. This may imply that --with-dashes also breaks
>> tests.
>> 
>>  t/test-lib.sh | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>> 
>> diff --git t/test-lib.sh t/test-lib.sh
>> index 7740d511d..0d51261f7 100644
>> --- t/test-lib.sh
>> +++ t/test-lib.sh
>> @@ -923,7 +923,7 @@ elif test -n "$GIT_TEST_INSTALLED"
>>  then
>>  	GIT_EXEC_PATH=$($GIT_TEST_INSTALLED/git --exec-path)  ||
>>  	error "Cannot run git from $GIT_TEST_INSTALLED."
>> -	PATH=$GIT_TEST_INSTALLED:$GIT_BUILD_DIR:$PATH
>>
>+	PATH=$GIT_TEST_INSTALLED:$GIT_BUILD_DIR/t/helper:$GIT_BUILD_DIR:$PATH
>>  	GIT_EXEC_PATH=${GIT_TEST_EXEC_PATH:-$GIT_EXEC_PATH}
>>  else # normal case, use ../bin-wrappers only unless $with_dashes:
>>  	git_bin_dir="$GIT_BUILD_DIR/bin-wrappers"
>
>This is essentially identical to what we have in
>
>http://github.com/git-for-windows/git/commit/e408b7517d
>
>So: ACK.
>
>You might also want to go a bit further and let the test suite run with
>GIT_TEST_INSTALLED when Git has not actually be built, but only the
>test
>helpers. I started something along those lines here:
>
>http://github.com/git-for-windows/git/commit/a80f047abc5
>
>I always meant to come back to polish those patches and submit them to
>the
>Git mailing list, so: thank you for getting the ball rolling.
>
>FWIW my use case is that I want to test a "MinGit" package, i.e. a
>subset
>of Git for Windows intended to cater to third-party applications
>requiring
>Git functionality (but not requiring any interactive parts of it).
>
>What is your use case?
>
>Ciao,
>Johannes

My use case is packaging git in Nix. We were bitten last week by the 2.17.0 update that modified default perl paths, and we broke the package without warning. See https://github.com/NixOS/nixpkgs/pull/38924 for the install tests and https://github.com/NixOS/nixpkgs/pull/38636#comment-380712557 for an idea of the mess we created...

I first tried to run the tests, but it turns out to be pretty uninformative because everything is setup properly in the build environment. Plus you do not expect meaningful test failures on a released version. What I really want is testing the final, installed version with all the weird wrapping, hacking and tuning that Nix requires.

As a packager, it would be nice to have dedicated install tests (integration tests?) that check the setup of perl path or the ability for got to find libexec executables without testing internal features. But we cannot require that from every project.

Running `make clean` before running the install test would be a good idea, because I have the feeling that the current implementation of GIT_TEST_INSTALLED relies a bit too much on build products in the source tree. Or at least that tests are not clearly separated from the build.

I will look at your patches and would be happy to review anything that you deem ready to upstream.

Cheers,

Guillaume, aka layus.