Re: [PATCH 1/2] completion: refactor __gitcomp related tests

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Mon, Oct 22, 2012 at 03:39:00AM +0200, Felipe Contreras wrote:
> Lots of duplicated code!
> 
> No functional changes.

I'm not sure.
I'm all for removing duplicated application code, but I'm usually more
conservative when it comes to test code.  The more logic, the more
possibility for bugs in tests.  So tests should be dead simple, even
if that means some duplicated test code or the lack of convenience
functions.
While this might be considered just a matter of personal preference, ...

> Signed-off-by: Felipe Contreras <felipe.contreras@xxxxxxxxx>
> ---
>  t/t9902-completion.sh | 72 ++++++++++++++++-----------------------------------
>  1 file changed, 23 insertions(+), 49 deletions(-)
> 
> diff --git a/t/t9902-completion.sh b/t/t9902-completion.sh
> index cbd0fb6..1c6952a 100755
> --- a/t/t9902-completion.sh
> +++ b/t/t9902-completion.sh
> @@ -72,87 +72,61 @@ test_completion_long ()
>  
>  newline=$'\n'
>  
> -test_expect_success '__gitcomp - trailing space - options' '
> -	sed -e "s/Z$//" >expected <<-\EOF &&
> -	--reuse-message=Z
> -	--reedit-message=Z
> -	--reset-author Z
> -	EOF
> +test_gitcomp ()
> +{
> +	sed -e 's/Z$//' > expected &&
>  	(
>  		local -a COMPREPLY &&
> -		cur="--re" &&
> -		__gitcomp "--dry-run --reuse-message= --reedit-message=
> -				--reset-author" &&
> +		cur="$1" &&
> +		shift &&
> +		__gitcomp "$@" &&

... I was really puzzled by how __gitcomp() gets its arguments here,
and had to think for a while to figure out why it's not broken.

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]