git@vger.kernel.org mailing list mirror (one of many)
 help / Atom feed
* [RFC PATCH] tests: fix PATH for GIT_TEST_INSTALLED tests
@ 2018-04-14  0:33 Guillaume Maudoux
  2018-04-14 12:30 ` Johannes Schindelin
  0 siblings, 1 reply; 3+ messages in thread
From: Guillaume Maudoux @ 2018-04-14  0:33 UTC (permalink / raw)
  To: git; +Cc: Jeff King, 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


^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [RFC PATCH] tests: fix PATH for GIT_TEST_INSTALLED tests
  2018-04-14  0:33 [RFC PATCH] tests: fix PATH for GIT_TEST_INSTALLED tests Guillaume Maudoux
@ 2018-04-14 12:30 ` Johannes Schindelin
  2018-04-14 19:36   ` Guillaume Maudoux (Layus)
  0 siblings, 1 reply; 3+ messages in thread
From: Johannes Schindelin @ 2018-04-14 12:30 UTC (permalink / raw)
  To: Guillaume Maudoux; +Cc: git, Jeff King

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

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [RFC PATCH] tests: fix PATH for GIT_TEST_INSTALLED tests
  2018-04-14 12:30 ` Johannes Schindelin
@ 2018-04-14 19:36   ` Guillaume Maudoux (Layus)
  0 siblings, 0 replies; 3+ messages in thread
From: Guillaume Maudoux (Layus) @ 2018-04-14 19:36 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: git, Jeff King

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.

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, back to index

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-04-14  0:33 [RFC PATCH] tests: fix PATH for GIT_TEST_INSTALLED tests Guillaume Maudoux
2018-04-14 12:30 ` Johannes Schindelin
2018-04-14 19:36   ` Guillaume Maudoux (Layus)

git@vger.kernel.org mailing list mirror (one of many)

Archives are clonable:
	git clone --mirror https://public-inbox.org/git
	git clone --mirror http://ou63pmih66umazou.onion/git
	git clone --mirror http://czquwvybam4bgbro.onion/git
	git clone --mirror http://hjrcffqmbrq6wope.onion/git

Newsgroups are available over NNTP:
	nntp://news.public-inbox.org/inbox.comp.version-control.git
	nntp://ou63pmih66umazou.onion/inbox.comp.version-control.git
	nntp://czquwvybam4bgbro.onion/inbox.comp.version-control.git
	nntp://hjrcffqmbrq6wope.onion/inbox.comp.version-control.git
	nntp://news.gmane.org/gmane.comp.version-control.git

 note: .onion URLs require Tor: https://www.torproject.org/
       or Tor2web: https://www.tor2web.org/

AGPL code for this site: git clone https://public-inbox.org/ public-inbox