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

From: Jonathan Nieder <jrnieder@gmail.com>
Date: Oct 3, 2010, 20:56 UTC
Message-ID: <20101003205615.GB22743@burratino>
In-Reply-To: <1286136014-7728-8-git-send-email-newren@gmail.com>

Elijah Newren wrote:

>> On Sun, Oct 3, 2010 at 8:28 AM, Jonathan Nieder <jrnieder@gmail.com> wrote:

Show 15 quoted lines

>>> 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 \