Re: [PATCH] t5801: properly test the test shell

From: Junio C Hamano <gitster@pobox.com>
Date: Apr 25, 2013, 16:20 UTC
Message-ID: <7vppxi60pj.fsf@alter.siamese.dyndns.org>
In-Reply-To: <ff7ccf22012f069ceca054d90aa0f72666cc11c2.1366884583.git.git@drmicha.warpmail.net>

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.

Show 11 quoted lines

> 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?

Show 23 quoted lines

> Make t5801 actually test whether the test shell is bash.
>
> An even better alternative would be to make the test POSIX compliant, of
> course.
>
> Signed-off-by: Michael J Gruber <git@drmicha.warpmail.net>
> ---
>  t/t5801-remote-helpers.sh | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/t/t5801-remote-helpers.sh b/t/t5801-remote-helpers.sh
> index ed962c4..c979863 100755
> --- a/t/t5801-remote-helpers.sh
> +++ b/t/t5801-remote-helpers.sh
> @@ -8,7 +8,7 @@ test_description='Test remote-helper import and export commands'
>  . ./test-lib.sh
>  . "$TEST_DIRECTORY"/lib-gpg.sh
>  
> -if ! type "${BASH-bash}" >/dev/null 2>&1; then
> +if test $(basename "${SHELL_PATH}") != "bash"; then
>  	skip_all='skipping remote-testgit tests, bash not available'
>  	test_done
>  fi