public inbox for git@vger.kernel.org 
 help / color / mirror / Atom feed

From: Michael J Gruber <git@drmicha•warpmail.net>
To: Junio C Hamano <gitster@pobox•com>
Cc: git@vger•kernel.org, Felipe Contreras <felipe.contreras@gmail•com>
Subject: Re: [PATCH] t5801: properly test the test shell
Date: Fri, 26 Apr 2013 11:55:17 +0200	[thread overview]
Message-ID: <517A4F05.5030609@drmicha.warpmail.net> (raw)
In-Reply-To: <7vfvye4jrc.fsf@alter.siamese.dyndns.org>

Junio C Hamano venit, vidit, dixit 25.04.2013 19:12:
> Junio C Hamano <gitster@pobox•com> writes:
> 
>> Michael J Gruber <git@drmicha•warpmail.net> writes:
>>
>>> fc407f9 (Add new simplified git-remote-testgit, 2012-11-28) introduced a
>>> test which was meant to skip the test unless the test shell is bash.
>>> Unfortunately, it tests for the availability of bash only.
>>
>> True.
>>
>>> But users can
>>> opt to use a different shell (using SHELL_PATH) for the tests even though
>>> bash is available.
>>>
>>> At least for dash,
>>> 21610d8 (transport-helper: clarify pushing without refspecs, 2013-04-17)
>>> is the commit which actually introduces a test (pushing without refspec)
>>> which fails to fail even though it is supposed to. It uses the
>>> construct:
>>>
>>> VAR=value function arguments
>>
>> The right fix for that is to fix that line, so that the test itself
>> can run under any sane POSIX shell, isn't it?  The test in turn may
>> need to run git-remote-testgit, which, without J6t's updates, only
>> is usable under bash, but to make sure the test will choke on
>> absence of bash, the existing check should be sufficient, no?
> 
> Curiously enough, there were a few instances of the correct "set and
> export environment explicitly during the life of subshell" construct
> already in the script.  I found only this one as problematic.
> 
> Does it fix your issue without your change?
> 
> It is a separate issue to port git-remote-testgit to POSIX (J6t
> already has a two part draft), move it to git-remote-testgit.sh, and
> get its shebang line preprocessed like all other shell scripts.  I
> think it is worth doing.
> 
> Takers?
> 
>  t/t5801-remote-helpers.sh | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/t/t5801-remote-helpers.sh b/t/t5801-remote-helpers.sh
> index 4dcf744..c956abd 100755
> --- a/t/t5801-remote-helpers.sh
> +++ b/t/t5801-remote-helpers.sh
> @@ -118,7 +118,9 @@ test_expect_success 'pushing without refspecs' '
>  	(cd local2 &&
>  	echo content >>file &&
>  	git commit -a -m ten &&
> -	GIT_REMOTE_TESTGIT_REFSPEC="" test_must_fail git push 2>../error) &&
> +	GIT_REMOTE_TESTGIT_REFSPEC="" &&
> +	export GIT_REMOTE_TESTGIT_REFSPEC &&
> +	test_must_fail git push 2>../error) &&
>  	grep "remote-helper doesn.t support push; refspec needed" error
>  '
>  
> 

Perfect, I just failed to notice that the subshell would make the export
local to that test.

Thanks!

Michael

next prev parent reply	other threads:[~2013-04-26  9:55 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-04-25 10:09 [PATCH] t5801: properly test the test shell Michael J Gruber
2013-04-25 10:59 ` Johannes Sixt
2013-04-25 11:21   ` Michael J Gruber
2013-04-25 11:35     ` Johannes Sixt
2013-04-25 16:22   ` Junio C Hamano
2013-04-25 13:06 ` Torsten Bögershausen
2013-04-25 16:25   ` Junio C Hamano
2013-04-25 16:20 ` Junio C Hamano
2013-04-25 17:12   ` Junio C Hamano
2013-04-26  9:55     ` Michael J Gruber [this message]
2013-04-26 18:33       ` Junio C Hamano

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=517A4F05.5030609@drmicha.warpmail.net \
    --to=git@drmicha$(echo .)warpmail.net \
    --cc=felipe.contreras@gmail$(echo .)com \
    --cc=git@vger$(echo .)kernel.org \
    --cc=gitster@pobox$(echo .)com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox