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

2 messages, 2 authors, 2016-06-15 · open the first message on its own page

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

From: Junio C Hamano <hidden>
Date: 2016-06-15 22:55:00

Nguyễn Thái Ngọc Duy  [off-list ref] writes:
quoted hunk
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".

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

From: Nguyen Thai Ngoc Duy <hidden>
Date: 2016-06-15 22:55:00

On Fri, Oct 12, 2012 at 12:09:57PM -0700, Junio C Hamano wrote:
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".
The reason I put these tests at the end was because I destroy
.gitattributes and it might affect the following tests. But obviously
the bare tests run in its own repository (and I have not committed
anything till the repo is cloned) so my .gitattributes changes can't
affect them.

Please squash this in

-- 8< --
diff --git a/t/t0003-attributes.sh b/t/t0003-attributes.sh
index 4a1402f..f6c21ea 100755
--- a/t/t0003-attributes.sh
+++ b/t/t0003-attributes.sh
@@ -206,6 +206,16 @@ test_expect_success 'root subdir attribute test' '
 	attr_check subdir/a/i unspecified
 '
 
+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_expect_success 'setup bare' '
 	git clone --bare . bare.git &&
 	cd bare.git
@@ -242,18 +252,4 @@ 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
-- 8< --
-- 
Duy
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help