Re: [PATCHv6 07/16] t3600 (rm): add lots of missing &&

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Elijah Newren wrote:
>> On Sun, Oct 3, 2010 at 8:28 AM, Jonathan Nieder <jrnieder@xxxxxxxxx> wrote:

>>> Why not
>>>        -       echo content > foo
>>>        -       git add foo
>>>        -       git commit -m foo
>>>        +       echo content > foo &&
>>>        +       git add foo &&
>>>        +       git commit --allow-empty -m foo &&
>>> ?
>>
>> What advantage does using these three commands have over 'git checkout
>> HEAD -- foo'?  Perhaps I'm missing something, but I don't see it.
>> It's three commands to one, and the tests don't depend on foo starting
>> with contents of 'content'; just that foo matches HEAD to start.
>
> Is there an advantage of the three-command version I'm just missing

What if the content of foo in HEAD were "other content"?  Then the test
would not be testing what it is supposed to.

Maybe you would prefer something like this on top?  My only concern is
to make sure the test is robust (even if people add new tests before
these without paying much attention) and easy to read.

I suppose I am nitpicking excessively because I do not like to see
regressions, even in out-of-the-way code like this.
---
diff --git a/t/t3600-rm.sh b/t/t3600-rm.sh
index 9660ae0..ae469df 100755
--- a/t/t3600-rm.sh
+++ b/t/t3600-rm.sh
@@ -7,6 +7,15 @@ test_description='Test of the various options to git rm.'
 
 . ./test-lib.sh
 
+prepare_foo () {
+	echo "$1" >foo &&
+	git add foo &&
+	git commit --allow-empty -m "set HEAD to $1" &&
+	echo "$2" >foo &&
+	git add foo &&
+	echo "$3" >foo
+}
+
 # Setup some files to be removed, some with funny characters
 test_expect_success \
     'Initialize test directory' \
@@ -44,27 +53,18 @@ test_expect_success \
 
 test_expect_success \
     'Test that git rm --cached foo succeeds if the index matches the file' \
-    'echo content > foo &&
-     git add foo &&
-     git commit -m foo &&
-     echo "other content" > foo &&
+    'prepare_foo content content "other content" &&
      git rm --cached foo'
 
 test_expect_success \
     'Test that git rm --cached foo fails if the index matches neither the file nor HEAD' '
-     git checkout HEAD -- foo &&
-     echo "other content" > foo &&
-     git add foo &&
-     echo "yet another content" > foo &&
+     prepare_foo content "other content" "yet another content" &&
      test_must_fail git rm --cached foo
 '
 
 test_expect_success \
     'Test that git rm --cached -f foo works in case where --cached only did not' \
-    'git checkout HEAD -- foo &&
-     echo "other content" > foo &&
-     git add foo &&
-     echo "yet another content" > foo &&
+    'prepare_foo content "other content" "yet another content" &&
      git rm --cached -f foo'
 
 test_expect_success \
-- 
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]