Re: [PATCH v2 1/9] completion: add file completion tests

[Junio C Hamano <gitster@pobox.com>]

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

> The commit fea16b4 (git-completion.bash: add support for path
> completion) introduced quite a few changes, without the usual tests.
>
> Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
> ---
>  t/t9902-completion.sh | 68 +++++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 68 insertions(+)
>
> diff --git a/t/t9902-completion.sh b/t/t9902-completion.sh
> index 6d9d141..385e1e4 100755
> --- a/t/t9902-completion.sh
> +++ b/t/t9902-completion.sh
> @@ -347,4 +347,72 @@ test_expect_success 'send-email' '
>  	test_completion "git send-email ma" "master "
>  '
>  
> +test_expect_success 'complete files' '
> +	git init tmp && cd tmp &&
> +	test_when_finished "cd .. && rm -rf tmp" &&

Hmm, this construct is not very nice.

Later part of the code can be modified carelessly and chdir around
even more, and "cd .." may end up at an unexpected place.  Can't
this be done in a way to make later reviews of patches that come
after this change lands doable without looking at the whole 70
lines?  For example, if the test ran the whole "these are done in a
subdirectry" part, starting from the "cd tmp", inside a subshell,
and "rm -rf tmp" is done in the original shell that ran "git init
tmp" to create it, it would make it much harder to make mistakes for
anybody who add new code to the body of the test to affect the
clean-up part.

> +
> +	echo "expected" > .gitignore &&
> +	echo "out" >> .gitignore &&

Style: we tend to omit SP between redireciton operator and targets.

> +	git add .gitignore &&
> +	test_completion "git commit " ".gitignore" &&
> +
> +	git commit -m ignore &&
> +
> +	touch new &&
> +	test_completion "git add " "new" &&
> +
> +	git add new &&
> +	git commit -a -m new &&
> +	test_completion "git add " "" &&
> +
> +	git mv new modified &&
> +	echo modify > modified &&
> +	test_completion "git add " "modified" &&
> +
> +	touch untracked &&
> +
> +	: TODO .gitignore should not be here &&
> +	test_completion "git rm " <<-\EOF &&
> +	.gitignore
> +	modified
> +	EOF
> +
> +	test_completion "git clean " "untracked" &&
> +
> +	: TODO .gitignore should not be here &&
> +	test_completion "git mv " <<-\EOF &&
> +	.gitignore
> +	modified
> +	EOF
> +
> +	mkdir dir &&
> +	touch dir/file-in-dir &&
> +	git add dir/file-in-dir &&
> +	git commit -m dir &&
> +
> +	mkdir untracked-dir &&
> +
> +	: TODO .gitignore should not be here &&
> +	test_completion "git mv modified " <<-\EOF &&
> +	.gitignore
> +	dir
> +	modified
> +	untracked
> +	untracked-dir
> +	EOF
> +
> +	test_completion "git commit " "modified" &&
> +
> +	: TODO .gitignore should not be here &&
> +	test_completion "git ls-files " <<-\EOF
> +	.gitignore
> +	dir
> +	modified
> +	EOF
> +
> +	touch momified &&
> +	test_completion "git add mom" "momified"
> +'
> +
>  test_done