Skip to contentSkip to conversation

git-checkout: Add a test case for relative paths use.

PATCH In review David Symonds 3 participants 4 messages Jun 15, 2016

lore
Share & tools
lore

David Symonds

author
#0 lore raw

PATCH 1/1 git-checkout: Add a test case for relative paths use.

Signed-off-by David Symonds <[email protected]>
---
 t/t2008-checkout-subdir.sh |   47 ++++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 47 insertions(+), 0 deletions(-)
 create mode 100755 t/t2008-checkout-subdir.sh
t/t2008-checkout-subdir.sh new file +47 -0
new file mode 100755
index 0000000..45b9e13
@@ -0,0 +1,47 @@
+#!/bin/sh
+#
+# Copyright (c) 2007 David Symonds
+
+test_description='git checkout from subdirectories'
+
+. ./test-lib.sh
+
+test_expect_success setup '
+
+	echo base > file0 &&
+	git add file0 &&
+	mkdir dir1 &&
+	echo hello > dir1/file1 &&
+	git add dir1/file1 &&
+	test_tick &&
+	mkdir dir2 &&
+	echo bonjour > dir2/file2 &&
+	git add dir2/file2 &&
+	git commit -m "populate tree"
+
+'
+
+test_expect_success 'remove and restore with relative path' '
+
+	cd dir1 &&
+	rm ../file0 &&
+	git checkout HEAD -- ../file0 && test -f ../file0 &&
+	rm ../dir2/file2 &&
+	git checkout HEAD -- ../dir2/file2 && test -f ../dir2/file2 &&
+	rm ../file0 ./file1 &&
+	git checkout HEAD -- .. && test -f ../file0 && test -f ./file1 &&
+	rm file1 &&
+	git checkout HEAD -- ../dir1/../dir1/file1 && test -f ./file1
+
+'
+
+test_expect_failure 'checkout with relative path outside tree should fail (1)' \
+	'git checkout HEAD -- ../file0'
+
+test_expect_failure 'checkout with relative path outside tree should fail (2)' \
+	'cd dir1 && git checkout HEAD -- ./file0'
+
+test_expect_failure 'checkout with relative path outside tree should fail (2)' \
+	'cd dir1 && git checkout HEAD -- ../../file0'
+
+test_done
Signature · 1 line
1.5.3.1
#1 lore raw

Hi,

just a few nitpicks:

t/t2008-checkout-subdir.sh line 16

Quoted 8 lines · David Symonds
t/t2008-checkout-subdir.sh new file +8 -0
+test_expect_success setup '
+
+	echo base > file0 &&
+	git add file0 &&
+	mkdir dir1 &&
+	echo hello > dir1/file1 &&
+	git add dir1/file1 &&
+	test_tick &&

please move the test_tick directly in front of the commit. Readers might assume that it has an effect on mkdir otherwise.

t/t2008-checkout-subdir.sh line 22

Quoted 6 lines
t/t2008-checkout-subdir.sh new file +6 -0
+	mkdir dir2 &&
+	echo bonjour > dir2/file2 &&
+	git add dir2/file2 &&
+	git commit -m "populate tree"
+
+'

Please lose the empty line before the closing quote. (This applies to all tests.)

Quoted 6 lines
t/t2008-checkout-subdir.sh new file +6 -0
+test_expect_success 'remove and restore with relative path' '
+
+	cd dir1 &&
+	rm ../file0 &&
+	git checkout HEAD -- ../file0 && test -f ../file0 &&
+	rm ../dir2/file2 &&

t/t2008-checkout-subdir.sh line 39

Quoted 10 lines
t/t2008-checkout-subdir.sh new file +10 -0
+	git checkout HEAD -- ../dir2/file2 && test -f ../dir2/file2 &&
+	rm ../file0 ./file1 &&
+	git checkout HEAD -- .. && test -f ../file0 && test -f ./file1 &&
+	rm file1 &&
+	git checkout HEAD -- ../dir1/../dir1/file1 && test -f ./file1
+
+'
+
+test_expect_failure 'checkout with relative path outside tree should fail (1)' \
+	'git checkout HEAD -- ../file0'

Maybe do that with an existing file? Since the test script lives in t/, and the test is run in t/trash/, we can test for "../Makefile".

Also, I would shorten the message to "relative path outside tree should fail".

t/t2008-checkout-subdir.sh line 42

Quoted 2 lines
t/t2008-checkout-subdir.sh new file +2 -0
+test_expect_failure 'checkout with relative path outside tree should fail (2)' \
+	'cd dir1 && git checkout HEAD -- ./file0'

I am not convinced that this should fail.

Review

Quoted 2 lines
(quoted diff) +2 -0
+test_expect_failure 'checkout with relative path outside tree should fail (2)' \
+	'cd dir1 && git checkout HEAD -- ../../file0'

Please add some other test like

test_expect_success 'checkout with empty prefix' '

	rm file0 &&
	git checkout HEAD -- file0 &&
	test base = "$(cat file0)"

'

Thanks,
Dscho

#2 lore raw
Quoted 9 lines · Johannes Schindelin

t/t2008-checkout-subdir.sh line 22

Quoted 6 lines
t/t2008-checkout-subdir.sh new file +6 -0
+	mkdir dir2 &&
+	echo bonjour > dir2/file2 &&
+	git add dir2/file2 &&
+	git commit -m "populate tree"
+
+'

Please lose the empty line before the closing quote. (This applies to all tests.)

I personaly find the extra blank lines before and after the indented test body easier to read. That is..

	test_expect_sucess 'test description comes here' '

		test command 1 &&
		test command 2 &&
                ...
                test command N

	'

I agree with all other suggestions from your message.

#3 lore raw

Hi,

Quoted 12 lines · Junio C Hamano
Quoted 9 lines · Johannes Schindelin

t/t2008-checkout-subdir.sh line 22

Quoted 6 lines
t/t2008-checkout-subdir.sh new file +6 -0
+	mkdir dir2 &&
+	echo bonjour > dir2/file2 &&
+	git add dir2/file2 &&
+	git commit -m "populate tree"
+
+'

Please lose the empty line before the closing quote. (This applies to all tests.)

Quoted 2 lines

I personaly find the extra blank lines before and after the indented test body easier to read.

Personally, I don't. But you are the maintainer.

Ciao,
Dscho