Re: [PATCHv4 06/15] t3600 (rm): add lots of missing &&

On Fri, Oct 1, 2010 at 4:48 AM, Jonathan Nieder [off-list ref] wrote:

Elijah Newren wrote:

quoted

Also delete unnecessary, failing code and add some test_when_finished
calls to make sure tests clean up after themselves.

I don't understand this one.

If tests clean up after themselves, each will restore some pristine
state as though (at least as far as the other tests care) it had never
run at all, right?

quoted

--- a/t/t3600-rm.sh
+++ b/t/t3600-rm.sh

@@ -38,37 +38,33 @@ 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
+    'echo content > foo &&
+     git add foo &&
      git rm --cached foo'

Before this runs: foo is empty, absent from index.
After it runs: foo has content, still absent from index.
Requires: nothing.

quoted

 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
+    'echo content > foo &&
+     git add foo &&
+     git commit -m foo &&
+     echo "other content" > foo &&
+     test_when_finished "git checkout HEAD -- foo" &&
      git rm --cached foo'

Before this runs: foo has content, absent from index.
After this runs (without patch): foo has other content, absent from index.
After this runs (with patch): foo has content, matching index.
Requires: nothing.

quoted

 test_expect_success \
     'Test that git rm --cached foo fails if the index matches neither the file nor HEAD' '
-     echo content > foo
-     git add foo
-     git commit -m foo
-     echo "other content" > foo
-     git add foo
-     echo "yet another content" > foo
+     echo "other content" > foo &&
+     git add foo &&
+     echo "yet another content" > foo &&
+     test_when_finished "git checkout HEAD -- foo" &&
      test_must_fail git rm --cached foo
 '

Before this runs: see above.
After this runs (without patch): foo has yet another content, absent from index.
After this runs (with patch): foo has content, matching index.
Requires (without patch): nothing.
Requires (with patch): HEAD:foo is content.

[etc]

So if I understand correctly, this sacrifices the ability to
skip individual tests.  What does one gain in return?

Good eye.  It does remove failing code and add correct && chaining,
but yes, it will fail if you skip sufficiently many individual tests.
I believe my original patch submission
(http://article.gmane.org/gmane.comp.version-control.git/157080) does
not suffer from this same problem.  Should I revert to it?

Elijah