Re: [PATCH v2 2/2] attr: more matching optimizations from .gitignore

From: Junio C Hamano <gitster@pobox.com>
Date: Oct 12, 2012, 19:09 UTC
Message-ID: <7vy5jb33sq.fsf@alter.siamese.dyndns.org>
In-Reply-To: <1349864466-28289-1-git-send-email-pclouds@gmail.com>

Nguyễn Thái Ngọc Duy  <pclouds@gmail.com> writes:

Show 23 quoted lines

> diff --git a/t/t0003-attributes.sh b/t/t0003-attributes.sh
> index 51f3045..4a1402f 100755
> --- a/t/t0003-attributes.sh
> +++ b/t/t0003-attributes.sh
> @@ -242,4 +242,18 @@ test_expect_success 'bare repository: test info/attributes' '
>  	attr_check subdir/a/i unspecified
>  '
>  
> +test_expect_success 'leave bare' '
> +	cd ..
> +'
> +
> +test_expect_success 'negative patterns' '
> +	echo "!f test=bar" >.gitattributes &&
> +	test_must_fail git check-attr test -- f
> +'
> +
> +test_expect_success 'patterns starting with exclamation' '
> +	echo "\!f test=foo" >.gitattributes &&
> +	attr_check "!f" foo
> +'
> +
>  test_done

This is not entirely your fault, but please don't do that "cd ..".

The original test had "cd bare", made an assumption that step will never fail (which is mostly correct), and ran everything afterward in that subdirectory.

Adding "Do a 'cd ..' to come back" is a horrible way to build on it. Imagine what happens when another person also did the same thing, and both changes need to be merged. You will end up going up two levels, which is not what you want.

I think the right fix is to make each of the test that wants to run in "bare" chdir in its own subshell, or not append these new tests that do not run in the "bare" to the end of this file, but before the execution goes down to "bare".