[StGit PATCH] Remove broken branch creation subtest

[StGit PATCH] Remove broken branch creation subtest

[Karl Hasselström]

This subtest has started to cause subsequent subtests to fail with
recent versions of git. And I don't think we can blame this one on
git. What the subtest does is:

  1. Remove all files or directories called "foo" under .git/. This is
     supposed to delete the "foo" branch and associated StGit files,
     but what about packed refs? This isn't actually malfunctioning
     yet as far as I can tell, but it's a ticking bomb.

  2. Create an empty file .git/refs/heads/foo. This is supposed to be
     a "broken branch", and indeed it is -- for example, git show-ref
     barfs on such a repository even if asked to only show a branch
     other than foo!

  3. Makes sure that stg branch won't successfully create a "foo"
     branch. I'm pretty sure this fails because git thinks the repo is
     broken, not because stg handles it gracefully. This is what the
     test is supposed to be testing, but if we wanted that, we'd need
     a more detailed test.

  4. Doesn't clean up the broken ref, which causes some subsequent
     subtests to fail.

What probably happened is that git got ever so slightly fussier about
broken refs, so that (4) became a problem.

Signed-off-by: Karl Hasselström <kha@treskal.com>

---

This should go to the stable branch. (master is affected too, but a
merge will fix that.)

 t/t1000-branch-create.sh |    7 -------
 1 files changed, 0 insertions(+), 7 deletions(-)


diff --git a/t/t1000-branch-create.sh b/t/t1000-branch-create.sh
index d6cf34a..298eb1a 100755
--- a/t/t1000-branch-create.sh
+++ b/t/t1000-branch-create.sh
@@ -54,13 +54,6 @@ test_expect_success \
 '
 
 test_expect_success \
-    'Create an invalid refs/heads/ entry' '
-    find .git -name foo | xargs rm -rf &&
-    touch .git/refs/heads/foo &&
-    ! stg branch -c foo
-'
-
-test_expect_success \
     'Setup two commits including removal of generated files' '
     git init &&
     touch a.c a.o &&

Re: [StGit PATCH] Remove broken branch creation subtest

[Junio C Hamano]

Karl Hasselström <kha@treskal.com> writes:

> This subtest has started to cause subsequent subtests to fail with
> recent versions of git. And I don't think we can blame this one on
> git. What the subtest does is:
>
>   1. Remove all files or directories called "foo" under .git/. This is
>      supposed to delete the "foo" branch and associated StGit files,
>      but what about packed refs? This isn't actually malfunctioning
>      yet as far as I can tell, but it's a ticking bomb.

With an additional "git branch -D foo" perhaps upfront before you manually
"corrupt" the repository, this can be resurrected without disabling the
test, can't it?

>   2. Create an empty file .git/refs/heads/foo. This is supposed to be
>      a "broken branch", and indeed it is -- for example, git show-ref
>      barfs on such a repository even if asked to only show a branch
>      other than foo!

You got me worried here.

 * "git show-ref" issues "error: refs/heads/foo points nowhere!" in all
   cases, which is not bad.

 * broken foo does not prevent "git show-ref" (no patterns) from carrying
   out its primary task.  It goes on showing others.  There is no bad here
   either.

 * "git show-ref refs/heads/foo" errors out with 1, which is Ok.

 * "git show-ref master" shows all the ones ending with "master", exits
   with 0, which is Ok.

>   3. Makes sure that stg branch won't successfully create a "foo"
>      branch. I'm pretty sure this fails because git thinks the repo is
>      broken, not because stg handles it gracefully. This is what the
>      test is supposed to be testing, but if we wanted that, we'd need
>      a more detailed test.
>
>   4. Doesn't clean up the broken ref, which causes some subsequent
>      subtests to fail.

It may be worth fixing the test than working it around, if only cleaning
up is the issue.

Re: [StGit PATCH] Remove broken branch creation subtest

[Karl Hasselström]

On 2008-04-12 11:06:13 -0700, Junio C Hamano wrote:

> You got me worried here.
>
>  * "git show-ref" issues "error: refs/heads/foo points nowhere!" in all
>    cases, which is not bad.
>
>  * broken foo does not prevent "git show-ref" (no patterns) from carrying
>    out its primary task.  It goes on showing others.  There is no bad here
>    either.
>
>  * "git show-ref refs/heads/foo" errors out with 1, which is Ok.
>
>  * "git show-ref master" shows all the ones ending with "master", exits
>    with 0, which is Ok.

Sorry for making you all worried, when I could have tested this
behavior myself and saved you the trouble ...

> It may be worth fixing the test than working it around, if only
> cleaning up is the issue.

Yes, that does seem more constructive. Will try to whip up a
replacement patch later today.

Thanks for the review.

-- 
Karl Hasselström, kha@treskal.com
      www.treskal.com/kalle

[StGit PATCH] Fix problems in t1000-branch-create

[Karl Hasselström]

There were a number of problems with t1000-branch-create:

  * It assumed that refs were not packed. I added calls to show-ref to
    spot any packed refs.

  * It reused the same name for several consecutive branch creation
    tests. This causes a domino effect if one of the subtests fail. I
    made the subtests use different names.

  * It tried to make sure that we couldn't create a branch if there
    was a broken ref by the same name. Unfortunately that test fails
    -- we don't create the branch, but we point HEAD to it. This
    causes many subsequent operations to fail. I marked this test as a
    known failure, and added some reset code so that subsequent
    subtests don't die because of this.

Signed-off-by: Karl Hasselström <kha@treskal.com>

---

Thanks to Junio for suggesting that the test should be repaired rather
than deleted. It finds a real bug! (A rather harmless one, but still.)

 t/t1000-branch-create.sh |   56 ++++++++++++++++++++++++++++++----------------
 1 files changed, 36 insertions(+), 20 deletions(-)


diff --git a/t/t1000-branch-create.sh b/t/t1000-branch-create.sh
index d6cf34a..5a097a4 100755
--- a/t/t1000-branch-create.sh
+++ b/t/t1000-branch-create.sh
@@ -21,65 +21,81 @@ test_expect_success \
     'Create a spurious patches/ entry' '
     stg branch master &&
     stg init &&
-    find .git -name foo | xargs rm -rf &&
-    mkdir -p .git/patches && touch .git/patches/foo
+    mkdir -p .git/patches && touch .git/patches/foo1
 '
 
 test_expect_success \
     'Try to create an stgit branch with a spurious patches/ entry' '
-    ! stg branch -c foo
+    ! stg branch -c foo1
 '
 
 test_expect_success \
     'Check that no part of the branch was created' '
-    test "`find .git -name foo | tee /dev/stderr`" = ".git/patches/foo" &&
-    ( grep foo .git/HEAD; test $? = 1 )
+    test "$(find .git -name foo1 | tee /dev/stderr)" = ".git/patches/foo1" &&
+    test "$(git show-ref | grep foo1 | wc -l)" = 0 &&
+    test "$(git symbolic-ref HEAD)" = "refs/heads/master"
 '
 
 test_expect_success \
     'Create a git branch' '
-    find .git -name foo | xargs rm -rf &&
-    cp .git/refs/heads/master .git/refs/heads/foo
+    git update-ref refs/heads/foo2 refs/heads/master
 '
 
 test_expect_success \
     'Try to create an stgit branch with an existing git branch by that name' '
-    ! stg branch -c foo
+    ! stg branch -c foo2
 '
 
 test_expect_success \
     'Check that no part of the branch was created' '
-    test "`find .git -name foo | tee /dev/stderr`" = ".git/refs/heads/foo" &&
-    ( grep foo .git/HEAD; test $? = 1 )
+    test "$(find .git -name foo2 | tee /dev/stderr \
+        | grep -v ^\\.git/refs/heads/foo2$ \
+        | grep -v ^\\.git/logs/refs/heads/foo2$ | wc -l)" = 0 &&
+    test "$(git show-ref | grep foo2 | wc -l)" = 1 &&
+    test "$(git symbolic-ref HEAD)" = "refs/heads/master"
 '
 
 test_expect_success \
     'Create an invalid refs/heads/ entry' '
-    find .git -name foo | xargs rm -rf &&
-    touch .git/refs/heads/foo &&
-    ! stg branch -c foo
+    touch .git/refs/heads/foo3 &&
+    ! stg branch -c foo3
 '
 
+test_expect_failure \
+    'Check that no part of the branch was created' '
+    test "$(find .git -name foo3 | tee /dev/stderr \
+        | grep -v ^\\.git/refs/heads/foo3$ | wc -l)" = 0 &&
+    test "$(git show-ref | grep foo3 | wc -l)" = 0 &&
+    test "$(git symbolic-ref HEAD)" = "refs/heads/master"
+'
+
+# Workaround for the test failure to make the rest of the subtests
+# succeed. (HEAD was erroneously overwritten with the bad foo3 ref, so
+# we need to reset it.)
+git symbolic-ref HEAD refs/heads/master
+
 test_expect_success \
     'Setup two commits including removal of generated files' '
     git init &&
-    touch a.c a.o &&
-    git add a.c a.o &&
+    touch file1 file2 &&
+    git add file1 file2 &&
     git commit -m 1 &&
-    git rm a.c a.o &&
+    git rm file1 file2 &&
     git commit -m 2 &&
-    touch a.o
+    touch file2
 '
 
 test_expect_success \
     'Create branch down the stack, behind the conflict caused by the generated file' '
-    ! stg branch --create bar master^
+    ! stg branch --create foo4 master^
 '
 
 test_expect_success \
     'Check the branch was not created' '
-    test ! -r .git/refs/heads/bar &&
-    test ! -r a.c
+    test ! -e file1 &&
+    test "$(find .git -name foo4 | tee /dev/stderr | wc -l)" = 0 &&
+    test "$(git show-ref | grep foo4 | wc -l)" = 0 &&
+    test "$(git symbolic-ref HEAD)" = "refs/heads/master"
 '
 
 test_done