[PATCH] t9400: Add some tests for checkout

[PATCH] t9400: Add some tests for checkout

[Frank Lichtenheld]

Test the behaviour for non-branches (e.g. head) and
non-existant branches. Also make a test checkout with
packed refs.

Signed-off-by: Frank Lichtenheld <frank@lichtenheld.de>
---
 t/t9400-git-cvsserver-server.sh |   48 ++++++++++++++++++++++++++++++++++++--
 1 files changed, 45 insertions(+), 3 deletions(-)

diff --git a/t/t9400-git-cvsserver-server.sh b/t/t9400-git-cvsserver-server.sh
index 641303e..11def84 100755
--- a/t/t9400-git-cvsserver-server.sh
+++ b/t/t9400-git-cvsserver-server.sh
@@ -246,11 +246,50 @@ test_expect_success 'gitcvs.ext.dbname' \
    test ! -f "$SERVERDIR/gitcvs2.ext.master.sqlite" &&
    cmp "$SERVERDIR/gitcvs.master.sqlite" "$SERVERDIR/gitcvs1.ext.master.sqlite"'
 
+# clean up
+rm -fr cvswork2
+rm -fr "$SERVERDIR"
+cd "$WORKDIR" &&
+git clone -q --local --bare "$WORKDIR/.git" "$SERVERDIR" >/dev/null 2>&1 &&
+GIT_DIR="$SERVERDIR" git config --bool gitcvs.enabled true &&
+GIT_DIR="$SERVERDIR" git config gitcvs.logfile "$SERVERDIR/gitcvs.log" ||
+exit 1
 
-#------------
-# CVS UPDATE
-#------------
+#--------------
+# CVS CHECKOUT
+#--------------
+test_expect_success 'cvs checkout failure (HEAD)' \
+  'if GIT_CONFIG="$git_config" cvs -Q co -d cvswork2 HEAD >cvs.log 2>&1
+   then
+     echo unexpected cvs success
+     false
+   else
+     true
+   fi &&
+   cat cvs.log | grep -q "not a branch" &&
+   test ! -d cvswork2'
+rm -fr cvswork2
 
+test_expect_success 'cvs checkout failure (foobar)' \
+  'if GIT_CONFIG="$git_config" cvs -Q co -d cvswork2 foobar >cvs.log 2>&1
+   then
+     echo unexpected cvs success
+     false
+   else
+     true
+   fi &&
+   cat cvs.log | grep -q "not a branch" &&
+   test ! -d cvswork2'
+rm -fr cvswork2
+
+test_expect_success 'cvs checkout (packed ref)' \
+  'GIT_DIR="$SERVERDIR" git-gc &&
+   test ! -f "$SERVERDIR/refs/heads/master" &&
+   GIT_CONFIG="$git_config" cvs -Q co -d cvswork2 master >cvs.log 2>&1 &&
+   diff -qr cvswork cvswork2'
+
+# clean up
+rm -fr cvswork2
 rm -fr "$SERVERDIR"
 cd "$WORKDIR" &&
 git clone -q --local --bare "$WORKDIR/.git" "$SERVERDIR" >/dev/null 2>&1 &&
@@ -258,6 +297,9 @@ GIT_DIR="$SERVERDIR" git config --bool gitcvs.enabled true &&
 GIT_DIR="$SERVERDIR" git config gitcvs.logfile "$SERVERDIR/gitcvs.log" ||
 exit 1
 
+#------------
+# CVS UPDATE
+#------------
 test_expect_success 'cvs update (create new file)' \
   'echo testfile1 >testfile1 &&
    git add testfile1 &&
-- 
1.5.3.3

Re: [PATCH] t9400: Add some tests for checkout

[Johannes Schindelin]

Hi,

On Thu, 4 Oct 2007, Frank Lichtenheld wrote:

> +# clean up
> +rm -fr cvswork2
> +rm -fr "$SERVERDIR"
> +cd "$WORKDIR" &&
> +git clone -q --local --bare "$WORKDIR/.git" "$SERVERDIR" >/dev/null 2>&1 &&
> +GIT_DIR="$SERVERDIR" git config --bool gitcvs.enabled true &&
> +GIT_DIR="$SERVERDIR" git config gitcvs.logfile "$SERVERDIR/gitcvs.log" ||
> +exit 1

Should this not be in a test_expect_success, too?

> -#------------
> -# CVS UPDATE
> -#------------
> +#--------------
> +# CVS CHECKOUT
> +#--------------
> +test_expect_success 'cvs checkout failure (HEAD)' \
> +  'if GIT_CONFIG="$git_config" cvs -Q co -d cvswork2 HEAD >cvs.log 2>&1
> +   then
> +     echo unexpected cvs success
> +     false
> +   else
> +     true
> +   fi &&

How about "! GIT_CONFIG..." instead of the "if..then..else..fi" ?

> +   cat cvs.log | grep -q "not a branch" &&
> +   test ! -d cvswork2'
> +rm -fr cvswork2

Again, for consistency, I'd include this in the test case.

Ciao,
Dscho

Re: [PATCH] t9400: Add some tests for checkout

[Frank Lichtenheld]

On Thu, Oct 04, 2007 at 07:24:05PM +0100, Johannes Schindelin wrote:
> On Thu, 4 Oct 2007, Frank Lichtenheld wrote:
> 
> > +# clean up
> > +rm -fr cvswork2
> > +rm -fr "$SERVERDIR"
> > +cd "$WORKDIR" &&
> > +git clone -q --local --bare "$WORKDIR/.git" "$SERVERDIR" >/dev/null 2>&1 &&
> > +GIT_DIR="$SERVERDIR" git config --bool gitcvs.enabled true &&
> > +GIT_DIR="$SERVERDIR" git config gitcvs.logfile "$SERVERDIR/gitcvs.log" ||
> > +exit 1
> 
> Should this not be in a test_expect_success, too?

Since I do this several times and since it is easier to see what tests
it belongs to if it isn't buried in one of them, I would say "no".

> > -#------------
> > -# CVS UPDATE
> > -#------------
> > +#--------------
> > +# CVS CHECKOUT
> > +#--------------
> > +test_expect_success 'cvs checkout failure (HEAD)' \
> > +  'if GIT_CONFIG="$git_config" cvs -Q co -d cvswork2 HEAD >cvs.log 2>&1
> > +   then
> > +     echo unexpected cvs success
> > +     false
> > +   else
> > +     true
> > +   fi &&
> 
> How about "! GIT_CONFIG..." instead of the "if..then..else..fi" ?

I don't really care. IIRC I took the idiom from another testfile.
Several other tests in this file already use it, too. So unless
Junio prefers I change all occourences, I will not.

> > +   cat cvs.log | grep -q "not a branch" &&
> > +   test ! -d cvswork2'
> > +rm -fr cvswork2
> 
> Again, for consistency, I'd include this in the test case.

Again, I don't really care. But for consistency with the rest of the
file I will only change it if I'm asked to change all occourences.

Gruesse,
-- 
Frank Lichtenheld <frank@lichtenheld.de>
www: http://www.djpig.de/

Re: [PATCH] t9400: Add some tests for checkout

[Johannes Schindelin]

Hi,

On Thu, 4 Oct 2007, Frank Lichtenheld wrote:

> On Thu, Oct 04, 2007 at 07:24:05PM +0100, Johannes Schindelin wrote:
> > On Thu, 4 Oct 2007, Frank Lichtenheld wrote:
> > 
> > > +# clean up
> > > +rm -fr cvswork2
> > > +rm -fr "$SERVERDIR"
> > > +cd "$WORKDIR" &&
> > > +git clone -q --local --bare "$WORKDIR/.git" "$SERVERDIR" >/dev/null 2>&1 &&
> > > +GIT_DIR="$SERVERDIR" git config --bool gitcvs.enabled true &&
> > > +GIT_DIR="$SERVERDIR" git config gitcvs.logfile "$SERVERDIR/gitcvs.log" ||
> > > +exit 1
> > 
> > Should this not be in a test_expect_success, too?
> 
> Since I do this several times and since it is easier to see what tests
> it belongs to if it isn't buried in one of them, I would say "no".

Just to clarify: I meant making an own "test_expect_success", a la "setup" 
in many other test scripts.

But I'm not the maintainer, so I'll just shut up.

Ciao,
Dscho

Re: [PATCH] t9400: Add some tests for checkout

[Frank Lichtenheld]

On Thu, Oct 04, 2007 at 08:17:07PM +0100, Johannes Schindelin wrote:
> On Thu, 4 Oct 2007, Frank Lichtenheld wrote:
> > On Thu, Oct 04, 2007 at 07:24:05PM +0100, Johannes Schindelin wrote:
> > > Should this not be in a test_expect_success, too?
> > 
> > Since I do this several times and since it is easier to see what tests
> > it belongs to if it isn't buried in one of them, I would say "no".
> 
> Just to clarify: I meant making an own "test_expect_success", a la "setup" 
> in many other test scripts.

Hmm, ok, will consider it. But as said, this should then be done for the
whole file at once, so I will definetly not change this particular
patch.

Gruesse,
-- 
Frank Lichtenheld <frank@lichtenheld.de>
www: http://www.djpig.de/