All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/3] Fixes for tests run without perl
@ 2013-08-24  4:01 Kacper Kornet
  2013-08-24  4:01 ` [PATCH 1/3] Make test "using invalid commit with -C" more strict Kacper Kornet
                   ` (3 more replies)
  0 siblings, 4 replies; 13+ messages in thread
From: Kacper Kornet @ 2013-08-24  4:01 UTC (permalink / raw)
  To: git

This is a set of fixes for problems found while running
test suite without perl installed.

Kacper

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH 1/3] Make test "using invalid commit with -C" more strict
  2013-08-24  4:01 [PATCH 0/3] Fixes for tests run without perl Kacper Kornet
@ 2013-08-24  4:01 ` Kacper Kornet
  2013-08-24 20:01   ` Jonathan Nieder
  2013-08-24  4:01 ` [PATCH 2/3] t/t3701-add-interactive.sh: Add PERL prerequisite Kacper Kornet
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 13+ messages in thread
From: Kacper Kornet @ 2013-08-24  4:01 UTC (permalink / raw)
  To: git

In the test 'using invalid commit with -C' git-commit would have failed
even if the -C option  had been given the correct commit, as there was
nothing to commit. Fix it by making sure there is always something to
commit and git-commit fails because of the invalid commit provided to
it.

Signed-off-by: Kacper Kornet <draenog@pld-linux.org>
---
 t/t7501-commit.sh | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/t/t7501-commit.sh b/t/t7501-commit.sh
index 99ce36f..699a603 100755
--- a/t/t7501-commit.sh
+++ b/t/t7501-commit.sh
@@ -53,7 +53,10 @@ test_expect_success PERL 'can use paths with --interactive' '
 '
 
 test_expect_success 'using invalid commit with -C' '
-	test_must_fail git commit -C bogus
+	echo bong >file &&
+	git add file &&
+	test_must_fail git commit -C bogus &&
+	git reset
 '
 
 test_expect_success 'nothing to commit' '
-- 
1.8.3.4

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH 2/3] t/t3701-add-interactive.sh: Add PERL prerequisite
  2013-08-24  4:01 [PATCH 0/3] Fixes for tests run without perl Kacper Kornet
  2013-08-24  4:01 ` [PATCH 1/3] Make test "using invalid commit with -C" more strict Kacper Kornet
@ 2013-08-24  4:01 ` Kacper Kornet
  2013-08-24 20:13   ` Jonathan Nieder
  2013-08-24  4:01 ` [PATCH 3/3] t/t7106-reset-unborn-branch.sh: " Kacper Kornet
  2013-08-24 19:58 ` [PATCH 0/3] Fixes for tests run without perl Jonathan Nieder
  3 siblings, 1 reply; 13+ messages in thread
From: Kacper Kornet @ 2013-08-24  4:01 UTC (permalink / raw)
  To: git

The test 'patch mode ignores unmerged entries' uses git-add -p, so it
depends on the perl code.

Signed-off-by: Kacper Kornet <draenog@pld-linux.org>
---
 t/t3701-add-interactive.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/t/t3701-add-interactive.sh b/t/t3701-add-interactive.sh
index 9fab25c..8514220 100755
--- a/t/t3701-add-interactive.sh
+++ b/t/t3701-add-interactive.sh
@@ -330,7 +330,7 @@ test_expect_success PERL 'split hunk "add -p (edit)"' '
 	! grep "^+15" actual
 '
 
-test_expect_success 'patch mode ignores unmerged entries' '
+test_expect_success PERL 'patch mode ignores unmerged entries' '
 	git reset --hard &&
 	test_commit conflict &&
 	test_commit non-conflict &&
-- 
1.8.3.4

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH 3/3] t/t7106-reset-unborn-branch.sh: Add PERL prerequisite
  2013-08-24  4:01 [PATCH 0/3] Fixes for tests run without perl Kacper Kornet
  2013-08-24  4:01 ` [PATCH 1/3] Make test "using invalid commit with -C" more strict Kacper Kornet
  2013-08-24  4:01 ` [PATCH 2/3] t/t3701-add-interactive.sh: Add PERL prerequisite Kacper Kornet
@ 2013-08-24  4:01 ` Kacper Kornet
  2013-08-24 20:34   ` Jonathan Nieder
  2013-08-24 19:58 ` [PATCH 0/3] Fixes for tests run without perl Jonathan Nieder
  3 siblings, 1 reply; 13+ messages in thread
From: Kacper Kornet @ 2013-08-24  4:01 UTC (permalink / raw)
  To: git

The test 'reset -p' uses git-reset -p, so it depends on the perl code.

Signed-off-by: Kacper Kornet <draenog@pld-linux.org>
---
 t/t7106-reset-unborn-branch.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/t/t7106-reset-unborn-branch.sh b/t/t7106-reset-unborn-branch.sh
index 8062cf5..499cd88 100755
--- a/t/t7106-reset-unborn-branch.sh
+++ b/t/t7106-reset-unborn-branch.sh
@@ -27,7 +27,7 @@ test_expect_success 'reset $file' '
 	test "$(git ls-files)" = "b"
 '
 
-test_expect_success 'reset -p' '
+test_expect_success PERL 'reset -p' '
 	rm .git/index &&
 	git add a &&
 	echo y | git reset -p &&
-- 
1.8.3.4

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* Re: [PATCH 0/3] Fixes for tests run without perl
  2013-08-24  4:01 [PATCH 0/3] Fixes for tests run without perl Kacper Kornet
                   ` (2 preceding siblings ...)
  2013-08-24  4:01 ` [PATCH 3/3] t/t7106-reset-unborn-branch.sh: " Kacper Kornet
@ 2013-08-24 19:58 ` Jonathan Nieder
  3 siblings, 0 replies; 13+ messages in thread
From: Jonathan Nieder @ 2013-08-24 19:58 UTC (permalink / raw)
  To: Kacper Kornet; +Cc: git

Hi,

Kacper Kornet wrote:

> This is a set of fixes for problems found while running
> test suite without perl installed.

I don't think git ever supported that.  The PERL prerequisite
was to check for systems that did not have (a suitable) perl
at runtime, but perl is still pretty heavily used in tests.

I assume you do have perl installed and that these fixes are
from testing the NO_PERL=YesPlease case, which is valuable.
Thanks for working on this.

Now on to the patches.

Jonathan

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 1/3] Make test "using invalid commit with -C" more strict
  2013-08-24  4:01 ` [PATCH 1/3] Make test "using invalid commit with -C" more strict Kacper Kornet
@ 2013-08-24 20:01   ` Jonathan Nieder
  2013-08-25  6:38     ` Junio C Hamano
  0 siblings, 1 reply; 13+ messages in thread
From: Jonathan Nieder @ 2013-08-24 20:01 UTC (permalink / raw)
  To: Kacper Kornet; +Cc: git

Kacper Kornet wrote:

> In the test 'using invalid commit with -C' git-commit would have failed
> even if the -C option  had been given the correct commit, as there was
> nothing to commit.

Good catch.

[...]
> --- a/t/t7501-commit.sh
> +++ b/t/t7501-commit.sh
> @@ -53,7 +53,10 @@ test_expect_success PERL 'can use paths with --interactive' '
>  '
>  
>  test_expect_success 'using invalid commit with -C' '
> -	test_must_fail git commit -C bogus
> +	echo bong >file &&
> +	git add file &&
> +	test_must_fail git commit -C bogus &&
> +	git reset

I guess to be pedantic this should say

	echo bong >file &&
	git add file &&
	test_when_finished "git reset --hard" &&
	test_must_fail git commit -C bogus

to avoid interfering with later tests even when this one fails and
the && prevents the 'git reset' from being executed.

With or without that change,
Reviewed-by: Jonathan Nieder <jrnieder@gmail.com>

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 2/3] t/t3701-add-interactive.sh: Add PERL prerequisite
  2013-08-24  4:01 ` [PATCH 2/3] t/t3701-add-interactive.sh: Add PERL prerequisite Kacper Kornet
@ 2013-08-24 20:13   ` Jonathan Nieder
  0 siblings, 0 replies; 13+ messages in thread
From: Jonathan Nieder @ 2013-08-24 20:13 UTC (permalink / raw)
  To: Kacper Kornet; +Cc: git

Kacper Kornet wrote:

> --- a/t/t3701-add-interactive.sh
> +++ b/t/t3701-add-interactive.sh
> @@ -330,7 +330,7 @@ test_expect_success PERL 'split hunk "add -p (edit)"' '
>  	! grep "^+15" actual
>  '
>  
> -test_expect_success 'patch mode ignores unmerged entries' '
> +test_expect_success PERL 'patch mode ignores unmerged entries' '

Mph.  This is a symptom of f0459319 (change from skip_all=* to prereq
skip, 2010-08-13), which hurt maintainability without much upside to
balance it.

I wonder if it would be easier to do something like the following
instead.

-- >8 --
Subject: add -i test: use skip_all instead of repeated PERL prerequisite

It is too easy to forget to add the PERL prerequisite for new
"add -i" tests, especially given that many people do not test with
NO_PERL so the missing prereq is not always noticed quickly.

The test had used the skip_all mechanism since 1b19ccd2 (2009-04-03)
but switched to explicit PERL prereqs in f0459319 (2010-10-13) in hope
of helping people see how many tests were skipped, perhaps to motivate
them to tweak their platform or tests to improve test coverage.  That
didn't pan out much in practice, so let's move back to the simpler
skip_all method.

Reported-by: Kacper Kornet <draenog@pld-linux.org>
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
 t/t3701-add-interactive.sh | 76 +++++++++++++++++++++++++---------------------
 1 file changed, 41 insertions(+), 35 deletions(-)

diff --git a/t/t3701-add-interactive.sh b/t/t3701-add-interactive.sh
index 9fab25cc..9dc91d09 100755
--- a/t/t3701-add-interactive.sh
+++ b/t/t3701-add-interactive.sh
@@ -4,18 +4,24 @@ test_description='add -i basic tests'
 . ./test-lib.sh
 . "$TEST_DIRECTORY"/lib-prereq-FILEMODE.sh
 
-test_expect_success PERL 'setup (initial)' '
+if ! test_have_prereq PERL
+then
+	skip_all='skipping add -i tests, perl not available'
+	test_done
+fi
+
+test_expect_success 'setup (initial)' '
 	echo content >file &&
 	git add file &&
 	echo more >>file &&
 	echo lines >>file
 '
-test_expect_success PERL 'status works (initial)' '
+test_expect_success 'status works (initial)' '
 	git add -i </dev/null >output &&
 	grep "+1/-0 *+2/-0 file" output
 '
 
-test_expect_success PERL 'setup expected' '
+test_expect_success 'setup expected' '
 cat >expected <<EOF
 new file mode 100644
 index 0000000..d95f3ad
@@ -26,19 +32,19 @@ index 0000000..d95f3ad
 EOF
 '
 
-test_expect_success PERL 'diff works (initial)' '
+test_expect_success 'diff works (initial)' '
 	(echo d; echo 1) | git add -i >output &&
 	sed -ne "/new file/,/content/p" <output >diff &&
 	test_cmp expected diff
 '
-test_expect_success PERL 'revert works (initial)' '
+test_expect_success 'revert works (initial)' '
 	git add file &&
 	(echo r; echo 1) | git add -i &&
 	git ls-files >output &&
 	! grep . output
 '
 
-test_expect_success PERL 'setup (commit)' '
+test_expect_success 'setup (commit)' '
 	echo baseline >file &&
 	git add file &&
 	git commit -m commit &&
@@ -47,12 +53,12 @@ test_expect_success PERL 'setup (commit)' '
 	echo more >>file &&
 	echo lines >>file
 '
-test_expect_success PERL 'status works (commit)' '
+test_expect_success 'status works (commit)' '
 	git add -i </dev/null >output &&
 	grep "+1/-0 *+2/-0 file" output
 '
 
-test_expect_success PERL 'setup expected' '
+test_expect_success 'setup expected' '
 cat >expected <<EOF
 index 180b47c..b6f2c08 100644
 --- a/file
@@ -63,12 +69,12 @@ index 180b47c..b6f2c08 100644
 EOF
 '
 
-test_expect_success PERL 'diff works (commit)' '
+test_expect_success 'diff works (commit)' '
 	(echo d; echo 1) | git add -i >output &&
 	sed -ne "/^index/,/content/p" <output >diff &&
 	test_cmp expected diff
 '
-test_expect_success PERL 'revert works (commit)' '
+test_expect_success 'revert works (commit)' '
 	git add file &&
 	(echo r; echo 1) | git add -i &&
 	git add -i </dev/null >output &&
@@ -76,24 +82,24 @@ test_expect_success PERL 'revert works (commit)' '
 '
 
 
-test_expect_success PERL 'setup expected' '
+test_expect_success 'setup expected' '
 cat >expected <<EOF
 EOF
 '
 
-test_expect_success PERL 'setup fake editor' '
+test_expect_success 'setup fake editor' '
 	>fake_editor.sh &&
 	chmod a+x fake_editor.sh &&
 	test_set_editor "$(pwd)/fake_editor.sh"
 '
 
-test_expect_success PERL 'dummy edit works' '
+test_expect_success 'dummy edit works' '
 	(echo e; echo a) | git add -p &&
 	git diff > diff &&
 	test_cmp expected diff
 '
 
-test_expect_success PERL 'setup patch' '
+test_expect_success 'setup patch' '
 cat >patch <<EOF
 @@ -1,1 +1,4 @@
  this
@@ -103,7 +109,7 @@ cat >patch <<EOF
 EOF
 '
 
-test_expect_success PERL 'setup fake editor' '
+test_expect_success 'setup fake editor' '
 	echo "#!$SHELL_PATH" >fake_editor.sh &&
 	cat >>fake_editor.sh <<\EOF &&
 mv -f "$1" oldpatch &&
@@ -113,26 +119,26 @@ EOF
 	test_set_editor "$(pwd)/fake_editor.sh"
 '
 
-test_expect_success PERL 'bad edit rejected' '
+test_expect_success 'bad edit rejected' '
 	git reset &&
 	(echo e; echo n; echo d) | git add -p >output &&
 	grep "hunk does not apply" output
 '
 
-test_expect_success PERL 'setup patch' '
+test_expect_success 'setup patch' '
 cat >patch <<EOF
 this patch
 is garbage
 EOF
 '
 
-test_expect_success PERL 'garbage edit rejected' '
+test_expect_success 'garbage edit rejected' '
 	git reset &&
 	(echo e; echo n; echo d) | git add -p >output &&
 	grep "hunk does not apply" output
 '
 
-test_expect_success PERL 'setup patch' '
+test_expect_success 'setup patch' '
 cat >patch <<EOF
 @@ -1,0 +1,0 @@
  baseline
@@ -142,7 +148,7 @@ cat >patch <<EOF
 EOF
 '
 
-test_expect_success PERL 'setup expected' '
+test_expect_success 'setup expected' '
 cat >expected <<EOF
 diff --git a/file b/file
 index b5dd6c9..f910ae9 100644
@@ -157,13 +163,13 @@ index b5dd6c9..f910ae9 100644
 EOF
 '
 
-test_expect_success PERL 'real edit works' '
+test_expect_success 'real edit works' '
 	(echo e; echo n; echo d) | git add -p &&
 	git diff >output &&
 	test_cmp expected output
 '
 
-test_expect_success PERL 'skip files similarly as commit -a' '
+test_expect_success 'skip files similarly as commit -a' '
 	git reset &&
 	echo file >.gitignore &&
 	echo changed >file &&
@@ -177,7 +183,7 @@ test_expect_success PERL 'skip files similarly as commit -a' '
 '
 rm -f .gitignore
 
-test_expect_success PERL,FILEMODE 'patch does not affect mode' '
+test_expect_success FILEMODE 'patch does not affect mode' '
 	git reset --hard &&
 	echo content >>file &&
 	chmod +x file &&
@@ -186,7 +192,7 @@ test_expect_success PERL,FILEMODE 'patch does not affect mode' '
 	git diff file | grep "new mode"
 '
 
-test_expect_success PERL,FILEMODE 'stage mode but not hunk' '
+test_expect_success FILEMODE 'stage mode but not hunk' '
 	git reset --hard &&
 	echo content >>file &&
 	chmod +x file &&
@@ -196,7 +202,7 @@ test_expect_success PERL,FILEMODE 'stage mode but not hunk' '
 '
 
 
-test_expect_success PERL,FILEMODE 'stage mode and hunk' '
+test_expect_success FILEMODE 'stage mode and hunk' '
 	git reset --hard &&
 	echo content >>file &&
 	chmod +x file &&
@@ -208,14 +214,14 @@ test_expect_success PERL,FILEMODE 'stage mode and hunk' '
 
 # end of tests disabled when filemode is not usable
 
-test_expect_success PERL 'setup again' '
+test_expect_success 'setup again' '
 	git reset --hard &&
 	test_chmod +x file &&
 	echo content >>file
 '
 
 # Write the patch file with a new line at the top and bottom
-test_expect_success PERL 'setup patch' '
+test_expect_success 'setup patch' '
 cat >patch <<EOF
 index 180b47c..b6f2c08 100644
 --- a/file
@@ -229,7 +235,7 @@ EOF
 '
 
 # Expected output, similar to the patch but w/ diff at the top
-test_expect_success PERL 'setup expected' '
+test_expect_success 'setup expected' '
 cat >expected <<EOF
 diff --git a/file b/file
 index b6f2c08..61b9053 100755
@@ -244,7 +250,7 @@ EOF
 '
 
 # Test splitting the first patch, then adding both
-test_expect_success PERL 'add first line works' '
+test_expect_success 'add first line works' '
 	git commit -am "clear local changes" &&
 	git apply patch &&
 	(echo s; echo y; echo y) | git add -p file &&
@@ -252,7 +258,7 @@ test_expect_success PERL 'add first line works' '
 	test_cmp expected diff
 '
 
-test_expect_success PERL 'setup expected' '
+test_expect_success 'setup expected' '
 cat >expected <<EOF
 diff --git a/non-empty b/non-empty
 deleted file mode 100644
@@ -264,7 +270,7 @@ index d95f3ad..0000000
 EOF
 '
 
-test_expect_success PERL 'deleting a non-empty file' '
+test_expect_success 'deleting a non-empty file' '
 	git reset --hard &&
 	echo content >non-empty &&
 	git add non-empty &&
@@ -275,7 +281,7 @@ test_expect_success PERL 'deleting a non-empty file' '
 	test_cmp expected diff
 '
 
-test_expect_success PERL 'setup expected' '
+test_expect_success 'setup expected' '
 cat >expected <<EOF
 diff --git a/empty b/empty
 deleted file mode 100644
@@ -283,7 +289,7 @@ index e69de29..0000000
 EOF
 '
 
-test_expect_success PERL 'deleting an empty file' '
+test_expect_success 'deleting an empty file' '
 	git reset --hard &&
 	> empty &&
 	git add empty &&
@@ -294,7 +300,7 @@ test_expect_success PERL 'deleting an empty file' '
 	test_cmp expected diff
 '
 
-test_expect_success PERL 'split hunk setup' '
+test_expect_success 'split hunk setup' '
 	git reset --hard &&
 	for i in 10 20 30 40 50 60
 	do
@@ -310,7 +316,7 @@ test_expect_success PERL 'split hunk setup' '
 	done >test
 '
 
-test_expect_success PERL 'split hunk "add -p (edit)"' '
+test_expect_success 'split hunk "add -p (edit)"' '
 	# Split, say Edit and do nothing.  Then:
 	#
 	# 1. Broken version results in a patch that does not apply and
-- 
1.8.4.rc4

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* Re: [PATCH 3/3] t/t7106-reset-unborn-branch.sh: Add PERL prerequisite
  2013-08-24  4:01 ` [PATCH 3/3] t/t7106-reset-unborn-branch.sh: " Kacper Kornet
@ 2013-08-24 20:34   ` Jonathan Nieder
  2013-08-25  6:50     ` Junio C Hamano
  0 siblings, 1 reply; 13+ messages in thread
From: Jonathan Nieder @ 2013-08-24 20:34 UTC (permalink / raw)
  To: Kacper Kornet; +Cc: git, Martin von Zweigbergk

Kacper Kornet wrote:

> Signed-off-by: Kacper Kornet <draenog@pld-linux.org>

Thanks.

Reviewed-by: Jonathan Nieder <jrnieder@gmail.com>

Here's a style cleanup on top.

-- >8 --
Subject: reset test: modernize style

Avoid command substitution and pipes to ensure that the exit status
from each git command is tested (and in particular that any segfaults
are caught).

Maintain the test setup (no commits, one file named "a", another named
"b") even after the last test, to make it easier to rearrange tests or
add new tests after the last in the future.

Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
 t/t7106-reset-unborn-branch.sh | 31 ++++++++++++++++++++++++-------
 1 file changed, 24 insertions(+), 7 deletions(-)

diff --git a/t/t7106-reset-unborn-branch.sh b/t/t7106-reset-unborn-branch.sh
index 499cd88c..af00ab4d 100755
--- a/t/t7106-reset-unborn-branch.sh
+++ b/t/t7106-reset-unborn-branch.sh
@@ -11,7 +11,10 @@ test_expect_success 'setup' '
 test_expect_success 'reset' '
 	git add a b &&
 	git reset &&
-	test "$(git ls-files)" = ""
+
+	>expect &&
+	git ls-files >actual &&
+	test_cmp expect actual
 '
 
 test_expect_success 'reset HEAD' '
@@ -24,28 +27,42 @@ test_expect_success 'reset $file' '
 	rm .git/index &&
 	git add a b &&
 	git reset a &&
-	test "$(git ls-files)" = "b"
+
+	echo b >expect &&
+	git ls-files >actual &&
+	test_cmp expect actual
 '
 
 test_expect_success PERL 'reset -p' '
 	rm .git/index &&
 	git add a &&
-	echo y | git reset -p &&
-	test "$(git ls-files)" = ""
+	echo y >yes &&
+	git reset -p <yes &&
+
+	>expect &&
+	git ls-files >actual &&
+	test_cmp expect actual
 '
 
 test_expect_success 'reset --soft is a no-op' '
 	rm .git/index &&
 	git add a &&
-	git reset --soft
-	test "$(git ls-files)" = "a"
+	git reset --soft &&
+
+	echo a >expect &&
+	git ls-files >actual &&
+	test_cmp expect actual
 '
 
 test_expect_success 'reset --hard' '
 	rm .git/index &&
 	git add a &&
+	test_when_finished "echo a >a" &&
 	git reset --hard &&
-	test "$(git ls-files)" = "" &&
+
+	>expect &&
+	git ls-files >actual &&
+	test_cmp expect actual &&
 	test_path_is_missing a
 '
 
-- 
1.8.4.rc4

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* Re: [PATCH 1/3] Make test "using invalid commit with -C" more strict
  2013-08-24 20:01   ` Jonathan Nieder
@ 2013-08-25  6:38     ` Junio C Hamano
  2013-08-25  6:40       ` Jonathan Nieder
  0 siblings, 1 reply; 13+ messages in thread
From: Junio C Hamano @ 2013-08-25  6:38 UTC (permalink / raw)
  To: Jonathan Nieder; +Cc: Kacper Kornet, git

Jonathan Nieder <jrnieder@gmail.com> writes:

> Kacper Kornet wrote:
>
>> In the test 'using invalid commit with -C' git-commit would have failed
>> even if the -C option  had been given the correct commit, as there was
>> nothing to commit.
>
> Good catch.
>
> [...]
>> --- a/t/t7501-commit.sh
>> +++ b/t/t7501-commit.sh
>> @@ -53,7 +53,10 @@ test_expect_success PERL 'can use paths with --interactive' '
>>  '
>>  
>>  test_expect_success 'using invalid commit with -C' '
>> -	test_must_fail git commit -C bogus
>> +	echo bong >file &&
>> +	git add file &&
>> +	test_must_fail git commit -C bogus &&
>> +	git reset
>
> I guess to be pedantic this should say
>
> 	echo bong >file &&
> 	git add file &&
> 	test_when_finished "git reset --hard" &&
> 	test_must_fail git commit -C bogus
>
> to avoid interfering with later tests even when this one fails and
> the && prevents the 'git reset' from being executed.

Makes sense.

Also it would be much simpler to say "git commit --allow-empty".

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 1/3] Make test "using invalid commit with -C" more strict
  2013-08-25  6:38     ` Junio C Hamano
@ 2013-08-25  6:40       ` Jonathan Nieder
  0 siblings, 0 replies; 13+ messages in thread
From: Jonathan Nieder @ 2013-08-25  6:40 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Kacper Kornet, git

Junio C Hamano wrote:
>> Kacper Kornet wrote:

>>> In the test 'using invalid commit with -C' git-commit would have failed
>>> even if the -C option  had been given the correct commit, as there was
>>> nothing to commit.
[...]
> Also it would be much simpler to say "git commit --allow-empty".

Sounds good. ;-)

Thanks,
Jonathan

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 3/3] t/t7106-reset-unborn-branch.sh: Add PERL prerequisite
  2013-08-24 20:34   ` Jonathan Nieder
@ 2013-08-25  6:50     ` Junio C Hamano
  2013-08-25  7:09       ` Jonathan Nieder
  0 siblings, 1 reply; 13+ messages in thread
From: Junio C Hamano @ 2013-08-25  6:50 UTC (permalink / raw)
  To: Jonathan Nieder; +Cc: Kacper Kornet, git, Martin von Zweigbergk

Jonathan Nieder <jrnieder@gmail.com> writes:

> Kacper Kornet wrote:
>
>> Signed-off-by: Kacper Kornet <draenog@pld-linux.org>
>
> Thanks.
>
> Reviewed-by: Jonathan Nieder <jrnieder@gmail.com>
>
> Here's a style cleanup on top.
>
> -- >8 --
> Subject: reset test: modernize style
>
> Avoid command substitution and pipes to ensure that the exit status
> from each git command is tested (and in particular that any segfaults
> are caught).

Makes sense.

The change to the one that feeds 'y' to "reset -p" may be a bit too
pedantic, as we are not in the business of testing "echo y", though.

> Maintain the test setup (no commits, one file named "a", another named
> "b") even after the last test, to make it easier to rearrange tests or
> add new tests after the last in the future.
>
> Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
> ---
>  t/t7106-reset-unborn-branch.sh | 31 ++++++++++++++++++++++++-------
>  1 file changed, 24 insertions(+), 7 deletions(-)
>
> diff --git a/t/t7106-reset-unborn-branch.sh b/t/t7106-reset-unborn-branch.sh
> index 499cd88c..af00ab4d 100755
> --- a/t/t7106-reset-unborn-branch.sh
> +++ b/t/t7106-reset-unborn-branch.sh
> @@ -11,7 +11,10 @@ test_expect_success 'setup' '
>  test_expect_success 'reset' '
>  	git add a b &&
>  	git reset &&
> -	test "$(git ls-files)" = ""
> +
> +	>expect &&
> +	git ls-files >actual &&
> +	test_cmp expect actual
>  '
>  
>  test_expect_success 'reset HEAD' '
> @@ -24,28 +27,42 @@ test_expect_success 'reset $file' '
>  	rm .git/index &&
>  	git add a b &&
>  	git reset a &&
> -	test "$(git ls-files)" = "b"
> +
> +	echo b >expect &&
> +	git ls-files >actual &&
> +	test_cmp expect actual
>  '
>  
>  test_expect_success PERL 'reset -p' '
>  	rm .git/index &&
>  	git add a &&
> -	echo y | git reset -p &&
> -	test "$(git ls-files)" = ""
> +	echo y >yes &&
> +	git reset -p <yes &&
> +
> +	>expect &&
> +	git ls-files >actual &&
> +	test_cmp expect actual
>  '
>  
>  test_expect_success 'reset --soft is a no-op' '
>  	rm .git/index &&
>  	git add a &&
> -	git reset --soft
> -	test "$(git ls-files)" = "a"
> +	git reset --soft &&
> +
> +	echo a >expect &&
> +	git ls-files >actual &&
> +	test_cmp expect actual
>  '
>  
>  test_expect_success 'reset --hard' '
>  	rm .git/index &&
>  	git add a &&
> +	test_when_finished "echo a >a" &&
>  	git reset --hard &&
> -	test "$(git ls-files)" = "" &&
> +
> +	>expect &&
> +	git ls-files >actual &&
> +	test_cmp expect actual &&
>  	test_path_is_missing a
>  '

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 3/3] t/t7106-reset-unborn-branch.sh: Add PERL prerequisite
  2013-08-25  6:50     ` Junio C Hamano
@ 2013-08-25  7:09       ` Jonathan Nieder
  0 siblings, 0 replies; 13+ messages in thread
From: Jonathan Nieder @ 2013-08-25  7:09 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Kacper Kornet, git, Martin von Zweigbergk

Junio C Hamano wrote:

> The change to the one that feeds 'y' to "reset -p" may be a bit too
> pedantic, as we are not in the business of testing "echo y", though.

Yeah, that's true.  Here's a patch for squashing in.

diff --git i/t/t7106-reset-unborn-branch.sh w/t/t7106-reset-unborn-branch.sh
index af00ab4d..bd28feba 100755
--- i/t/t7106-reset-unborn-branch.sh
+++ w/t/t7106-reset-unborn-branch.sh
@@ -12,9 +12,8 @@ test_expect_success 'reset' '
 	git add a b &&
 	git reset &&
 
-	>expect &&
 	git ls-files >actual &&
-	test_cmp expect actual
+	test_must_be_empty actual
 '
 
 test_expect_success 'reset HEAD' '
@@ -36,12 +35,10 @@ test_expect_success 'reset $file' '
 test_expect_success PERL 'reset -p' '
 	rm .git/index &&
 	git add a &&
-	echo y >yes &&
-	git reset -p <yes &&
+	echo y | git reset -p &&
 
-	>expect &&
 	git ls-files >actual &&
-	test_cmp expect actual
+	test_must_be_empty actual
 '
 
 test_expect_success 'reset --soft is a no-op' '
@@ -60,9 +57,8 @@ test_expect_success 'reset --hard' '
 	test_when_finished "echo a >a" &&
 	git reset --hard &&
 
-	>expect &&
 	git ls-files >actual &&
-	test_cmp expect actual &&
+	test_must_be_empty actual &&
 	test_path_is_missing a
 '
 

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH 0/3] Fixes for tests run without perl
@ 2013-08-24  3:30 Kacper Kornet
  0 siblings, 0 replies; 13+ messages in thread
From: Kacper Kornet @ 2013-08-24  3:30 UTC (permalink / raw)
  To: git

This is a set of fixes for problems found while running
test suite without perl installed.

Kacper

^ permalink raw reply	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2013-08-25  7:09 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-08-24  4:01 [PATCH 0/3] Fixes for tests run without perl Kacper Kornet
2013-08-24  4:01 ` [PATCH 1/3] Make test "using invalid commit with -C" more strict Kacper Kornet
2013-08-24 20:01   ` Jonathan Nieder
2013-08-25  6:38     ` Junio C Hamano
2013-08-25  6:40       ` Jonathan Nieder
2013-08-24  4:01 ` [PATCH 2/3] t/t3701-add-interactive.sh: Add PERL prerequisite Kacper Kornet
2013-08-24 20:13   ` Jonathan Nieder
2013-08-24  4:01 ` [PATCH 3/3] t/t7106-reset-unborn-branch.sh: " Kacper Kornet
2013-08-24 20:34   ` Jonathan Nieder
2013-08-25  6:50     ` Junio C Hamano
2013-08-25  7:09       ` Jonathan Nieder
2013-08-24 19:58 ` [PATCH 0/3] Fixes for tests run without perl Jonathan Nieder
  -- strict thread matches above, loose matches on Subject: below --
2013-08-24  3:30 Kacper Kornet

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.