[PATCH 0/4] Add a testsuite to stgit (take 2)

[PATCH 0/4] Add a testsuite to stgit (take 2)

[Yann Dirson]

This is an update of the previous patch series, including minor improvements
to the way the test engine is adapted to stgit, as well as a new testsuite
demonstrating robustness issues on series creation, and proposed fixes for all
those bugs.  And hopefully more standard patches.

-- 
Yann Dirson    <ydirson@altern.org> |
Debian-related: <dirson@debian.org> |   Support Debian GNU/Linux:
                                    |  Freedom, Power, Stability, Gratis
     http://ydirson.free.fr/        | Check <http://www.debian.org/>

[PATCH 1/4] Add a testsuite framework copied from git-core

[Yann Dirson]

From: Yann Dirson <ydirson@altern.org>

See git's t/README for details on how to use this framework.

There is no integration yet in the toplevel Makefile, I'll let
python masters take care of this.  Use "make -C t" to run the
tests for now.

A patch-naming policy should be defined for stgit, since the
git one does not apply.
---

 TODO             |    2 -
 t/Makefile       |   25 ++++++
 t/t0000-dummy.sh |   19 +++++
 t/test-lib.sh    |  208 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 253 insertions(+), 1 deletions(-)

diff --git a/TODO b/TODO
index e5affe0..d97ffd1 100644
--- a/TODO
+++ b/TODO
@@ -6,7 +6,7 @@ The TODO list until 1.0:
 - debian package support
 - man page
 - code execution allowed from templates
-- regression tests
+- more regression tests
 - release 1.0
 
 
diff --git a/t/Makefile b/t/Makefile
new file mode 100644
index 0000000..d5d7b6f
--- /dev/null
+++ b/t/Makefile
@@ -0,0 +1,25 @@
+# Run tests
+#
+# Copyright (c) 2005 Junio C Hamano
+#
+
+#GIT_TEST_OPTS=--verbose --debug
+SHELL_PATH ?= $(SHELL)
+TAR ?= $(TAR)
+
+# Shell quote;
+SHELL_PATH_SQ = $(subst ','\'',$(SHELL_PATH))
+
+T = $(wildcard t[0-9][0-9][0-9][0-9]-*.sh)
+
+all: $(T) clean
+
+$(T):
+	@echo "*** $@ ***"; '$(SHELL_PATH_SQ)' $@ $(GIT_TEST_OPTS)
+
+clean:
+	rm -fr trash
+
+.PHONY: $(T) clean
+.NOPARALLEL:
+
diff --git a/t/t0000-dummy.sh b/t/t0000-dummy.sh
new file mode 100755
index 0000000..8dc25d3
--- /dev/null
+++ b/t/t0000-dummy.sh
@@ -0,0 +1,19 @@
+#!/bin/sh
+#
+# Copyright (c) 2006 Yann Dirson
+#
+
+test_description='Dummy test.
+
+Only to test the testing environment.
+'
+
+. ./test-lib.sh
+
+test_stg_init
+
+test_expect_success \
+    'check stgit can be run' \
+    'stg version'
+
+test_done
diff --git a/t/test-lib.sh b/t/test-lib.sh
new file mode 100755
index 0000000..2580bcc
--- /dev/null
+++ b/t/test-lib.sh
@@ -0,0 +1,208 @@
+#!/bin/sh
+#
+# Copyright (c) 2005 Junio C Hamano
+# Copyright (c) 2006 Yann Dirson
+#
+
+# For repeatability, reset the environment to known value.
+LANG=C
+LC_ALL=C
+PAGER=cat
+TZ=UTC
+export LANG LC_ALL PAGER TZ
+unset AUTHOR_DATE
+unset AUTHOR_EMAIL
+unset AUTHOR_NAME
+unset COMMIT_AUTHOR_EMAIL
+unset COMMIT_AUTHOR_NAME
+unset GIT_ALTERNATE_OBJECT_DIRECTORIES
+unset GIT_AUTHOR_DATE
+GIT_AUTHOR_EMAIL=author@example.com
+GIT_AUTHOR_NAME='A U Thor'
+unset GIT_COMMITTER_DATE
+GIT_COMMITTER_EMAIL=committer@example.com
+GIT_COMMITTER_NAME='C O Mitter'
+unset GIT_DIFF_OPTS
+unset GIT_DIR
+unset GIT_EXTERNAL_DIFF
+unset GIT_INDEX_FILE
+unset GIT_OBJECT_DIRECTORY
+unset SHA1_FILE_DIRECTORIES
+unset SHA1_FILE_DIRECTORY
+export GIT_AUTHOR_EMAIL GIT_AUTHOR_NAME
+export GIT_COMMITTER_EMAIL GIT_COMMITTER_NAME
+
+# Each test should start with something like this, after copyright notices:
+#
+# test_description='Description of this test...
+# This test checks if command xyzzy does the right thing...
+# '
+# . ./test-lib.sh
+
+error () {
+	echo "* error: $*"
+	trap - exit
+	exit 1
+}
+
+say () {
+	echo "* $*"
+}
+
+test "${test_description}" != "" ||
+error "Test script did not set test_description."
+
+while test "$#" -ne 0
+do
+	case "$1" in
+	-d|--d|--de|--deb|--debu|--debug)
+		debug=t; shift ;;
+	-i|--i|--im|--imm|--imme|--immed|--immedi|--immedia|--immediat|--immediate)
+		immediate=t; shift ;;
+	-h|--h|--he|--hel|--help)
+		echo "$test_description"
+		exit 0 ;;
+	-v|--v|--ve|--ver|--verb|--verbo|--verbos|--verbose)
+		verbose=t; shift ;;
+	*)
+		break ;;
+	esac
+done
+
+exec 5>&1
+if test "$verbose" = "t"
+then
+	exec 4>&2 3>&1
+else
+	exec 4>/dev/null 3>/dev/null
+fi
+
+test_failure=0
+test_count=0
+
+trap 'echo >&5 "FATAL: Unexpected exit with code $?"; exit 1' exit
+
+
+# You are not expected to call test_ok_ and test_failure_ directly, use
+# the text_expect_* functions instead.
+
+test_ok_ () {
+	test_count=$(expr "$test_count" + 1)
+	say "  ok $test_count: $@"
+}
+
+test_failure_ () {
+	test_count=$(expr "$test_count" + 1)
+	test_failure=$(expr "$test_failure" + 1);
+	say "FAIL $test_count: $1"
+	shift
+	echo "$@" | sed -e 's/^/	/'
+	test "$immediate" = "" || { trap - exit; exit 1; }
+}
+
+
+test_debug () {
+	test "$debug" = "" || eval "$1"
+}
+
+test_run_ () {
+	eval >&3 2>&4 "$1"
+	eval_ret="$?"
+	return 0
+}
+
+test_expect_failure () {
+	test "$#" = 2 ||
+	error "bug in the test script: not 2 parameters to test-expect-failure"
+	say >&3 "expecting failure: $2"
+	test_run_ "$2"
+	if [ "$?" = 0 -a "$eval_ret" != 0 ]
+	then
+		test_ok_ "$1"
+	else
+		test_failure_ "$@"
+	fi
+}
+
+test_expect_success () {
+	test "$#" = 2 ||
+	error "bug in the test script: not 2 parameters to test-expect-success"
+	say >&3 "expecting success: $2"
+	test_run_ "$2"
+	if [ "$?" = 0 -a "$eval_ret" = 0 ]
+	then
+		test_ok_ "$1"
+	else
+		test_failure_ "$@"
+	fi
+}
+
+test_expect_code () {
+	test "$#" = 3 ||
+	error "bug in the test script: not 3 parameters to test-expect-code"
+	say >&3 "expecting exit code $1: $3"
+	test_run_ "$3"
+	if [ "$?" = 0 -a "$eval_ret" = "$1" ]
+	then
+		test_ok_ "$2"
+	else
+		test_failure_ "$@"
+	fi
+}
+
+# Most tests can use the created repository, but some amy need to create more.
+# Usage: test_create_repo <directory>
+test_create_repo () {
+	test "$#" = 1 ||
+	error "bug in the test script: not 1 parameter to test-create-repo"
+	owd=`pwd`
+	repo="$1"
+	mkdir "$repo"
+	cd "$repo" || error "Cannot setup test environment"
+	git-init-db 2>/dev/null ||
+	error "cannot run git-init-db -- have you installed git-core?"
+	mv .git/hooks .git/hooks-disabled
+	cd "$owd"
+}
+
+test_stg_init () {
+	echo "empty start" |
+	git-commit-tree `git-write-tree` >.git/refs/heads/master 2>/dev/null ||
+	error "cannot run git-commit -- is your git-core funtionning?"
+	stg init ||
+	error "cannot run stg init -- have you built things yet?"
+}
+
+test_done () {
+	trap - exit
+	case "$test_failure" in
+	0)
+		# We could:
+		# cd .. && rm -fr trash
+		# but that means we forbid any tests that use their own
+		# subdirectory from calling test_done without coming back
+		# to where they started from.
+		# The Makefile provided will clean this test area so
+		# we will leave things as they are.
+
+		say "passed all $test_count test(s)"
+		exit 0 ;;
+
+	*)
+		say "failed $test_failure among $test_count test(s)"
+		exit 1 ;;
+
+	esac
+}
+
+# Test the binaries we have just built.  The tests are kept in
+# t/ subdirectory and are run in trash subdirectory.
+PATH=$(pwd)/..:$PATH
+export PATH
+
+
+# Test repository
+test=trash
+rm -fr "$test"
+test_create_repo $test
+cd "$test"

[PATCH 2/4] Add list of bugs to TODO

[Yann Dirson]

From: Yann Dirson <ydirson@altern.org>

Since there is no formal place to register bugs, other than the git ml,
I have added a couple of them, even mentionned on the ml, but which are
still around.
---

 TODO |    9 +++++++++
 1 files changed, 9 insertions(+), 0 deletions(-)

diff --git a/TODO b/TODO
index d97ffd1..7dd099c 100644
--- a/TODO
+++ b/TODO
@@ -17,3 +17,12 @@ The future, when time allows or if someo
   synchronising with other patches (diff format or in other
   repositories)
 - write bash-completion script for the StGIT commands
+- support for branches with / in names
+  (ml: "Handle branch names with slashes")
+- "pull" argument should default to a sane value, "origin" is wrong in
+  many cases
+
+Bugs:
+
+- the following commands break in subdirs:
+  - refresh (ml: "Running StGIT in subdirectories")

[PATCH 3/4] Correctly handle refs/patches on series rename

[Yann Dirson]

From: Yann Dirson <ydirson@altern.org>

When renaming a series, the refs/patches dir was not moved, and
by chance a new one was created by the repository-upgrade code, but
that left the old one behind as cruft.

Also added a regression test to assert that nothing by the old name
is left behind.
---

 stgit/stack.py           |    2 ++
 t/t1001-branch-rename.sh |   33 +++++++++++++++++++++++++++++++++
 2 files changed, 35 insertions(+), 0 deletions(-)

diff --git a/stgit/stack.py b/stgit/stack.py
index f4d7490..92407e7 100644
--- a/stgit/stack.py
+++ b/stgit/stack.py
@@ -497,6 +497,8 @@ class Series:
             os.rename(self.__series_dir, to_stack.__series_dir)
         if os.path.exists(self.__base_file):
             os.rename(self.__base_file, to_stack.__base_file)
+        if os.path.exists(self.__refs_dir):
+            os.rename(self.__refs_dir, to_stack.__refs_dir)
 
         self.__init__(to_name)
 
diff --git a/t/t1001-branch-rename.sh b/t/t1001-branch-rename.sh
new file mode 100755
index 0000000..65a5280
--- /dev/null
+++ b/t/t1001-branch-rename.sh
@@ -0,0 +1,33 @@
+#!/bin/sh
+#
+# Copyright (c) 2006 Yann Dirson
+#
+
+test_description='Branch operations.
+
+Exercises the "stg branch" commands.
+'
+
+. ./test-lib.sh
+
+test_stg_init
+
+test_expect_success \
+    'Create an stgit branch from scratch' \
+    'stg branch -c foo &&
+     stg new p1 -m "p1"
+'
+
+test_expect_failure \
+    'Rename the current stgit branch' \
+    'stg branch -r foo bar
+'
+
+test_expect_success \
+    'Rename an stgit branch' \
+    'stg branch -c buz &&
+     stg branch -r foo bar &&
+     test -z `find .git -name foo | tee /dev/stderr`
+'
+
+test_done

[PATCH 4/4] Add a couple of safety checks to series creation

[Yann Dirson]

From: Yann Dirson <ydirson@altern.org>

Check first whether the operation can complete, instead of
bombing out halfway.
---

 stgit/commands/branch.py |    5 +++
 stgit/stack.py           |    7 ++++-
 t/t1000-branch-create.sh |   66 ++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 76 insertions(+), 2 deletions(-)

diff --git a/stgit/commands/branch.py b/stgit/commands/branch.py
index c4b5945..c95e529 100644
--- a/stgit/commands/branch.py
+++ b/stgit/commands/branch.py
@@ -122,12 +122,15 @@ def func(parser, options, args):
         check_conflicts()
         check_head_top_equal()
 
+        if git.branch_exists(args[0]):
+            raise CmdException, 'Branch "%s" already exists' % args[0]
+        
         tree_id = None
         if len(args) == 2:
             tree_id = git_id(args[1])
 
-        git.create_branch(args[0], tree_id)
         stack.Series(args[0]).init()
+        git.create_branch(args[0], tree_id)
 
         print 'Branch "%s" created.' % args[0]
         return
diff --git a/stgit/stack.py b/stgit/stack.py
index 92407e7..236e67f 100644
--- a/stgit/stack.py
+++ b/stgit/stack.py
@@ -431,8 +431,13 @@ class Series:
         """
         bases_dir = os.path.join(self.__base_dir, 'refs', 'bases')
 
-        if self.is_initialised():
+        if os.path.exists(self.__patch_dir):
             raise StackException, self.__patch_dir + ' already exists'
+        if os.path.exists(self.__refs_dir):
+            raise StackException, self.__refs_dir + ' already exists'
+        if os.path.exists(self.__base_file):
+            raise StackException, self.__base_file + ' already exists'
+
         os.makedirs(self.__patch_dir)
 
         if not os.path.isdir(bases_dir):
diff --git a/t/t1000-branch-create.sh b/t/t1000-branch-create.sh
new file mode 100755
index 0000000..bee0b1c
--- /dev/null
+++ b/t/t1000-branch-create.sh
@@ -0,0 +1,66 @@
+#!/bin/sh
+#
+# Copyright (c) 2006 Yann Dirson
+#
+
+test_description='Branch operations.
+
+Exercises the "stg branch" commands.
+'
+
+. ./test-lib.sh
+
+test_stg_init
+
+test_expect_failure \
+    'Try to create an stgit branch with a spurious refs/patches/ entry' \
+    'find .git -name foo | xargs rm -rf &&
+     touch .git/refs/patches/foo &&
+     stg branch -c foo
+'
+
+test_expect_success \
+    'Check no part of the branch was created' \
+    'test "`find .git -name foo | tee /dev/stderr`" = ".git/refs/patches/foo"
+'
+
+
+test_expect_failure \
+    'Try to create an stgit branch with a spurious patches/ entry' \
+    'find .git -name foo | xargs rm -rf &&
+     touch .git/patches/foo &&
+     stg branch -c foo
+'
+
+test_expect_success \
+    'Check no part of the branch was created' \
+    'test "`find .git -name foo | tee /dev/stderr`" = ".git/patches/foo"
+'
+
+
+test_expect_failure \
+    'Try to create an stgit branch with a spurious refs/bases/ entry' \
+    'find .git -name foo | xargs rm -rf &&
+     touch .git/refs/bases/foo &&
+     stg branch -c foo
+'
+
+test_expect_success \
+    'Check no part of the branch was created' \
+    'test "`find .git -name foo | tee /dev/stderr`" = ".git/refs/bases/foo"
+'
+
+
+# test_expect_failure \
+#     'Try to create an stgit branch with a spurious refs/heads/ entry' \
+#     'find .git -name foo | xargs rm -rf &&
+#      touch .git/refs/heads/foo &&
+#      stg branch -c foo
+# '
+
+# test_expect_success \
+#     'Check no part of the branch was created' \
+#     'test "`find .git -name foo | tee /dev/stderr`" = ".git/refs/heads/foo"
+# '
+
+test_done