Re: [PATCH 01/12] tests: add initial bash completion tests

From: Junio C Hamano <gitster@pobox.com>
Date: Apr 8, 2012, 04:25 UTC
Message-ID: <7vpqbiyhmu.fsf@alter.siamese.dyndns.org>
In-Reply-To: <1333854479-23260-2-git-send-email-felipe.contreras@gmail.com>

Felipe Contreras <felipe.contreras@gmail.com> writes:

Show 25 quoted lines

> Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
> ---
>  t/t9902-completion.sh |  201 +++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 201 insertions(+)
>  create mode 100755 t/t9902-completion.sh
>
> diff --git a/t/t9902-completion.sh b/t/t9902-completion.sh
> new file mode 100755
> index 0000000..7eb80dd
> --- /dev/null
> +++ b/t/t9902-completion.sh
> @@ -0,0 +1,201 @@
> +#!/bin/sh
> +#
> +# Copyright (c) 2012 Felipe Contreras
> +#
> +
> +if type bash >/dev/null 2>&1
> +then
> +	# execute inside bash
> +	test -z "$BASH" && exec bash "$0" "$@"
> +else
> +	echo '1..0 #SKIP skipping bash completion tests; bash not available'
> +	exit 0
> +fi

What shell do you use on your system as /bin/sh (or if you use SHELL_PATH in the Makefile to override it, what do you use)?

Bash has a special case code to behave in a "posixly correct" mode, where a lot of extensions are disabled, when invoked as "sh". Unfortunately, it still defines BASH_* variables, including BASH, when operating in this mode.

This unfortunately breaks the above code on a system where /bin/sh is symlinked to "bash". "make" runs test scripts using with SHELL_PATH, defaulting to /bin/sh, and you say "Ah, bash is available, oh, we have BASH defined so let's keep going" without doing an "exec", so later,

> +. "$GIT_BUILD_DIR/contrib/completion/git-completion.bash"

may trigger errors like this:

  sh: .../git-completion.bash: line 128: syntax error near unexpected token `<'
  sh: .../git-completion.bash: line 128: `    done < <(git config -z --get...

depending on what bash-ism is used in the then-current version of that file.

So we may want to be a bit more careful like this:

if test "z${BASH%/bash}" != "z${BASH}"
then
	: ends with bash so we are already running bash as bash
elif type bash >/dev/null 2>&1
then
	exec bash "$0" "$@"
else
	echo '1..0 #SKIP skipping bash completion tests; bash not available'
	exit 0
fi

This incidentally avoids running type twice when we are already running as bash.

Show 39 quoted lines

> +
> +test_description='test bash completion'
> +
> +. ./test-lib.sh
> +
> +complete ()
> +{
> +	# do nothing
> +	return 0
> +}
> +
> +. "$GIT_BUILD_DIR/contrib/completion/git-completion.bash"
> +
> +print_comp ()
> +{
> +	local IFS=$'\n'
> +	echo "${COMPREPLY[*]}" > out
> +}
> +
> +_get_comp_words_by_ref ()
> +{
> +	while [ $# -gt 0 ]; do
> +		case "$1" in
> +		cur)
> +			cur=${_words[_cword]}
> +			;;
> +		prev)
> +			prev=${_words[_cword-1]}
> +			;;
> +		words)
> +			words=("${_words[@]}")
> +			;;
> +		cword)
> +			cword=$_cword
> +			;;
> +		esac
> +		shift
> +	done
> +}

Could you add an explanation before this function to describe what is going on? The completion script being tested relies on the shell function of the same name defined there to behave one way, and this overrides it with a different implementation. Often such an override is an effective way to intercept and expose the internal state to test framework, but the above does not seem to be such an override, and it is unclear what is going on here.