[PATCH] tests: use Git's test framework

[PATCH] tests: use Git's test framework

[john]

This allows tests to run in parallel as well as letting us use "prove"
or another TAP harness to run the tests.

Git's test framework requires Git to be fully built before letting any
tests run, so add a new target to the top-level Makefile which builds
all of Git instead of just libgit.a and make the "test" target depend on
that.

Signed-off-by: John Keeping <john at keeping.me.uk>
---
I think this is the right direction for the tests, but I'm not sure if
using Git's test framework is the best way to go due to the requirement
to build all of Git before running tests.

It may be better to pull in something like Sharness[1] and continue
using the system git.  The only change to this patch for that approach
would be to source a different test library in setup.sh (and the
Makefile changes would no longer be needed).

[1] http://mlafeldt.github.com/sharness/

Obviously there are a number of further changes that can be done on top
of this to avoid the now-unnecessary wrapper functions and use a test
prerequisite for HTML tidy but I don't want to work on those unless
there is agreement that this is a sensible direction.

 Makefile                             |  7 ++++--
 tests/setup.sh                       | 44 +++++++-----------------------------
 tests/t0001-validate-git-versions.sh | 11 +++++----
 tests/t0010-validate-html.sh         |  3 ++-
 tests/t0020-validate-cache.sh        |  3 ++-
 tests/t0101-index.sh                 |  3 ++-
 tests/t0102-summary.sh               |  3 ++-
 tests/t0103-log.sh                   |  3 ++-
 tests/t0104-tree.sh                  |  3 ++-
 tests/t0105-commit.sh                |  3 ++-
 tests/t0106-diff.sh                  |  3 ++-
 tests/t0107-snapshot.sh              |  3 ++-
 tests/t0108-patch.sh                 |  3 ++-
 13 files changed, 39 insertions(+), 53 deletions(-)

diff --git a/Makefile b/Makefile
index 83d4716..979b1bc 100644
--- a/Makefile
+++ b/Makefile
@@ -66,7 +66,10 @@ all:: cgit
 cgit:
 	$(QUIET_SUBDIR0)git $(QUIET_SUBDIR1) -f ../cgit.mk ../cgit NO_CURL=1
 
-test: all
+git:
+	$(QUIET_SUBDIR0)git $(QUIET_SUBDIR1) NO_CURL=1
+
+test: all git
 	$(QUIET_SUBDIR0)tests $(QUIET_SUBDIR1) all
 
 install: all
@@ -145,7 +148,7 @@ get-git:
 tags:
 	$(QUIET_TAGS)find . -name '*.[ch]' | xargs ctags
 
-.PHONY: all cgit get-git
+.PHONY: all cgit git get-git
 .PHONY: clean clean-doc cleanall
 .PHONY: doc doc-html doc-man doc-pdf
 .PHONY: install install-doc install-html install-man install-pdf
diff --git a/tests/setup.sh b/tests/setup.sh
index e3c6c17..015d55a 100755
--- a/tests/setup.sh
+++ b/tests/setup.sh
@@ -15,7 +15,8 @@
 # run_test 'repo index' 'cgit_url "/" | tidy -e'
 # run_test 'repo summary' 'cgit_url "/foo" | tidy -e'
 
-unset CDPATH
+: ${TEST_DIRECTORY=$(pwd)/../git/t}
+. "$TEST_DIRECTORY"/test-lib.sh
 
 mkrepo() {
 	name=$1
@@ -88,61 +89,32 @@ EOF
 prepare_tests()
 {
 	setup_repos
-	rm -f test-output.log 2>/dev/null
-	test_count=0
-	test_failed=0
-	echo "[$0]" "$@" >test-output.log
-	echo "$@" "($0)"
 }
 
 tests_done()
 {
-	printf "\n"
-	if test $test_failed -gt 0
-	then
-		printf "test: *** %s failure(s), logfile=%s\n" \
-			$test_failed "$(pwd)/test-output.log"
-		false
-	fi
+	test_done
 }
 
 run_test()
 {
-	bug=0
+	func=test_expect_success
 	if test "$1" = "BUG"
 	then
-		bug=1
+		func=test_expect_failure
 		shift
 	fi
 	desc=$1
 	script=$2
-	test_count=$(expr $test_count + 1)
-	printf "\ntest %d: name='%s'\n" $test_count "$desc" >>test-output.log
-	printf "test %d: eval='%s'\n" $test_count "$2" >>test-output.log
-	eval "$2" >>test-output.log 2>>test-output.log
-	res=$?
-	printf "test %d: exitcode=%d\n" $test_count $res >>test-output.log
-	if test $res = 0 -a $bug = 0
-	then
-		printf " %2d) %-60s [ok]\n" $test_count "$desc"
-	elif test $res = 0 -a $bug = 1
-	then
-		printf " %2d) %-60s [BUG FIXED]\n" $test_count "$desc"
-	elif test $bug = 1
-	then
-		printf " %2d) %-60s [KNOWN BUG]\n" $test_count "$desc"
-	else
-		test_failed=$(expr $test_failed + 1)
-		printf " %2d) %-60s [failed]\n" $test_count "$desc"
-	fi
+	$func "$desc" "$script"
 }
 
 cgit_query()
 {
-	CGIT_CONFIG="$PWD/trash/cgitrc" QUERY_STRING="$1" "$PWD/../cgit"
+	CGIT_CONFIG="$PWD/trash/cgitrc" QUERY_STRING="$1" "$PWD/../../cgit"
 }
 
 cgit_url()
 {
-	CGIT_CONFIG="$PWD/trash/cgitrc" QUERY_STRING="url=$1" "$PWD/../cgit"
+	CGIT_CONFIG="$PWD/trash/cgitrc" QUERY_STRING="url=$1" "$PWD/../../cgit"
 }
diff --git a/tests/t0001-validate-git-versions.sh b/tests/t0001-validate-git-versions.sh
index 3378358..444e4a0 100755
--- a/tests/t0001-validate-git-versions.sh
+++ b/tests/t0001-validate-git-versions.sh
@@ -1,29 +1,30 @@
 #!/bin/sh
 
+test_description='Check Git version is correct'
 . ./setup.sh
 
-prepare_tests 'Check Git version is correct'
+prepare_tests
 
 run_test 'extract Git version from Makefile' '
 	sed -n -e "/^GIT_VER[ 	]*=/ {
 		s/^GIT_VER[ 	]*=[ 	]*//
 		p
-	}" ../Makefile >trash/makefile_version
+	}" ../../Makefile >trash/makefile_version
 '
 
 run_test 'test Git version matches Makefile' '
-	( cat ../git/GIT-VERSION-FILE || echo "No GIT-VERSION-FILE" ) |
+	( cat ../../git/GIT-VERSION-FILE || echo "No GIT-VERSION-FILE" ) |
 	sed -e "s/GIT_VERSION[ 	]*=[ 	]*//" >trash/git_version &&
 	diff -u trash/git_version trash/makefile_version
 '
 
 run_test 'test submodule version matches Makefile' '
-	if ! test -e ../git/.git
+	if ! test -e ../../git/.git
 	then
 		echo "git/ is not a Git repository" >&2
 	else
 		(
-			cd .. &&
+			cd ../.. &&
 			sm_sha1=$(git ls-files --stage -- git |
 				sed -e "s/^[0-9]* \\([0-9a-f]*\\) [0-9]	.*$/\\1/") &&
 			cd git &&
diff --git a/tests/t0010-validate-html.sh b/tests/t0010-validate-html.sh
index 3fe4800..97b1377 100755
--- a/tests/t0010-validate-html.sh
+++ b/tests/t0010-validate-html.sh
@@ -1,5 +1,6 @@
 #!/bin/sh
 
+test_description='Validate html with tidy'
 . ./setup.sh
 
 
@@ -21,7 +22,7 @@ test_url()
 	fi
 }
 
-prepare_tests 'Validate html with tidy'
+prepare_tests
 
 tidy=`which tidy`
 test -n "$tidy" || {
diff --git a/tests/t0020-validate-cache.sh b/tests/t0020-validate-cache.sh
index 53ec2eb..d8f7219 100755
--- a/tests/t0020-validate-cache.sh
+++ b/tests/t0020-validate-cache.sh
@@ -1,8 +1,9 @@
 #!/bin/sh
 
+test_description='Validate cache'
 . ./setup.sh
 
-prepare_tests 'Validate cache'
+prepare_tests
 
 run_test 'verify cache-size=0' '
 
diff --git a/tests/t0101-index.sh b/tests/t0101-index.sh
index ab63aca..b17dabd 100755
--- a/tests/t0101-index.sh
+++ b/tests/t0101-index.sh
@@ -1,8 +1,9 @@
 #!/bin/sh
 
+test_description='Check content on index page'
 . ./setup.sh
 
-prepare_tests "Check content on index page"
+prepare_tests
 
 run_test 'generate index page' 'cgit_url "" >trash/tmp'
 run_test 'find foo repo' 'grep "foo" trash/tmp'
diff --git a/tests/t0102-summary.sh b/tests/t0102-summary.sh
index f778cb4..e68852c 100755
--- a/tests/t0102-summary.sh
+++ b/tests/t0102-summary.sh
@@ -1,8 +1,9 @@
 #!/bin/sh
 
+test_description='Check content on summary page'
 . ./setup.sh
 
-prepare_tests "Check content on summary page"
+prepare_tests
 
 run_test 'generate foo summary' 'cgit_url "foo" >trash/tmp'
 run_test 'find commit 1' 'grep "commit 1" trash/tmp'
diff --git a/tests/t0103-log.sh b/tests/t0103-log.sh
index 67fcba0..9b86758 100755
--- a/tests/t0103-log.sh
+++ b/tests/t0103-log.sh
@@ -1,8 +1,9 @@
 #!/bin/sh
 
+test_description='Check content on log page'
 . ./setup.sh
 
-prepare_tests "Check content on log page"
+prepare_tests
 
 run_test 'generate foo/log' 'cgit_url "foo/log" >trash/tmp'
 run_test 'find commit 1' 'grep "commit 1" trash/tmp'
diff --git a/tests/t0104-tree.sh b/tests/t0104-tree.sh
index 7aa3b8d..c15752c 100755
--- a/tests/t0104-tree.sh
+++ b/tests/t0104-tree.sh
@@ -1,8 +1,9 @@
 #!/bin/sh
 
+test_description='Check content on tree page'
 . ./setup.sh
 
-prepare_tests "Check content on tree page"
+prepare_tests
 
 run_test 'generate bar/tree' 'cgit_url "bar/tree" >trash/tmp'
 run_test 'find file-1' 'grep "file-1" trash/tmp'
diff --git a/tests/t0105-commit.sh b/tests/t0105-commit.sh
index 31b554b..7a0bd13 100755
--- a/tests/t0105-commit.sh
+++ b/tests/t0105-commit.sh
@@ -1,8 +1,9 @@
 #!/bin/sh
 
+test_description='Check content on commit page'
 . ./setup.sh
 
-prepare_tests "Check content on commit page"
+prepare_tests
 
 run_test 'generate foo/commit' 'cgit_url "foo/commit" >trash/tmp'
 run_test 'find tree link' 'grep "<a href=./foo/tree/.>" trash/tmp'
diff --git a/tests/t0106-diff.sh b/tests/t0106-diff.sh
index eee0c8c..ed4fb80 100755
--- a/tests/t0106-diff.sh
+++ b/tests/t0106-diff.sh
@@ -1,8 +1,9 @@
 #!/bin/sh
 
+test_description='Check content on diff page'
 . ./setup.sh
 
-prepare_tests "Check content on diff page"
+prepare_tests
 
 run_test 'generate foo/diff' 'cgit_url "foo/diff" >trash/tmp'
 run_test 'find diff header' 'grep "a/file-5 b/file-5" trash/tmp'
diff --git a/tests/t0107-snapshot.sh b/tests/t0107-snapshot.sh
index 132d2e9..7eeb77a 100755
--- a/tests/t0107-snapshot.sh
+++ b/tests/t0107-snapshot.sh
@@ -1,8 +1,9 @@
 #!/bin/sh
 
+test_description='Verify snapshot'
 . ./setup.sh
 
-prepare_tests "Verify snapshot"
+prepare_tests
 
 run_test 'get foo/snapshot/master.tar.gz' '
 	cgit_url "foo/snapshot/master.tar.gz" >trash/tmp
diff --git a/tests/t0108-patch.sh b/tests/t0108-patch.sh
index f92f69c..580fd43 100755
--- a/tests/t0108-patch.sh
+++ b/tests/t0108-patch.sh
@@ -1,8 +1,9 @@
 #!/bin/sh
 
+test_description='Check content on patch page'
 . ./setup.sh
 
-prepare_tests "Check content on patch page"
+prepare_tests
 
 run_test 'generate foo/patch' '
 	cgit_query "url=foo/patch" >trash/tmp
-- 
1.8.2.540.gf023cfe

[PATCH] tests: use Git's test framework

[Jason]

On Mon, Apr 1, 2013 at 4:09 PM, John Keeping <john at keeping.me.uk> wrote:
> This allows tests to run in parallel as well as letting us use "prove"
> or another TAP harness to run the tests.
>
> Git's test framework requires Git to be fully built before letting any
> tests run, so add a new target to the top-level Makefile which builds
> all of Git instead of just libgit.a and make the "test" target depend on
> that.

I don't think that's a problem. Technically, the tests should be
against the git revision that cgit uses, not the system git.

My only hesitation is whether or not the pretty printing benefits are
worth the added complexity.

[PATCH] tests: use Git's test framework

[Jason]

On Mon, Apr 1, 2013 at 4:09 PM, John Keeping <john at keeping.me.uk> wrote:
> Obviously there are a number of further changes that can be done on top
> of this to avoid the now-unnecessary wrapper functions and use a test
> prerequisite for HTML tidy but I don't want to work on those unless
> there is agreement that this is a sensible direction.

If moving forward with this would indeed result in reducing the
complexity of our own test harness, I'm okay with that. Removing our
wrapper would be quite nice. AFAIK, git's testing infrastructure is
fairly stable, so I don't anticipate using it will introduce any
unforeseen hiccups down the road.

Maybe, though, we ought to be having a larger discussion about this.
We're in the process of bringing cgit up to speed with git head. We're
now relying on git's Makefile. And now we're discussing using git's
test infrastructure as well. We're getting cozier with git.

Is this something we want to do? Or should we say, "let's just be
friends", before a child is on the way, and then a fight followed by a
nasty divorce?

My feeling is that reusing as much of git's existing infrastructure is
ideal and should be encouraged. One of the strengths of cgit has
always been that we use the actual meat of git and are tied to it in
an intimate way. In fact, down the road, if cgit is in a ripe position
for merging in some respect with git-itself, I think this is also
something I'd be in favor of. (But hey, it's only the third date;
let's not get too excited about such futures ;-).

So from this perspective, I'd be in support of relying on git's test harness.

But there might be other considerations I've overlooked; my ears are
open to dissenting opinions.

[PATCH] tests: use Git's test framework

[john]

On Thu, Apr 04, 2013 at 04:24:57PM +0200, Jason A. Donenfeld wrote:
> On Mon, Apr 1, 2013 at 4:09 PM, John Keeping <john at keeping.me.uk> wrote:
> > This allows tests to run in parallel as well as letting us use "prove"
> > or another TAP harness to run the tests.
> >
> > Git's test framework requires Git to be fully built before letting any
> > tests run, so add a new target to the top-level Makefile which builds
> > all of Git instead of just libgit.a and make the "test" target depend on
> > that.
> 
> I don't think that's a problem. Technically, the tests should be
> against the git revision that cgit uses, not the system git.
> 
> My only hesitation is whether or not the pretty printing benefits are
> worth the added complexity.

Personally I think the big improvement here is the ability to run the
tests in parallel.  I quite often run "make -j4 test" only to remember
that CGit's tests don't work when run in parallel.

The prettier output and ability to do things like
"prove --state=host,all,save" is just a bonus.

[PATCH] tests: use Git's test framework

[mailings]

On 04/04/13 16:32, Jason A. Donenfeld wrote:
> On Mon, Apr 1, 2013 at 4:09 PM, John Keeping <john at keeping.me.uk> wrote:
>> Obviously there are a number of further changes that can be done on top
>> of this to avoid the now-unnecessary wrapper functions and use a test
>> prerequisite for HTML tidy but I don't want to work on those unless
>> there is agreement that this is a sensible direction.
> 
> If moving forward with this would indeed result in reducing the
> complexity of our own test harness, I'm okay with that. Removing our
> wrapper would be quite nice. AFAIK, git's testing infrastructure is
> fairly stable, so I don't anticipate using it will introduce any
> unforeseen hiccups down the road.
> 
> Maybe, though, we ought to be having a larger discussion about this.
> We're in the process of bringing cgit up to speed with git head. We're
> now relying on git's Makefile. And now we're discussing using git's
> test infrastructure as well. We're getting cozier with git.
> 
> Is this something we want to do? Or should we say, "let's just be
> friends", before a child is on the way, and then a fight followed by a
> nasty divorce?
> 
> My feeling is that reusing as much of git's existing infrastructure is
> ideal and should be encouraged. One of the strengths of cgit has
> always been that we use the actual meat of git and are tied to it in
> an intimate way. In fact, down the road, if cgit is in a ripe position
> for merging in some respect with git-itself, I think this is also
> something I'd be in favor of. (But hey, it's only the third date;
> let's not get too excited about such futures ;-).
> 

I'm completely in favor of doing this, it will make upstreaming cgit
into git way easier once cgit is in a good enough position

> So from this perspective, I'd be in support of relying on git's test harness.
> 
> But there might be other considerations I've overlooked; my ears are
> open to dissenting opinions.
> 
> _______________________________________________
> cgit mailing list
> cgit at hjemli.net
> http://hjemli.net/mailman/listinfo/cgit
> 

-- 
Ferry Huberts

[PATCH] tests: use Git's test framework

[Jason]

On Mon, Apr 1, 2013 at 4:09 PM, John Keeping <john at keeping.me.uk> wrote:
> Obviously there are a number of further changes that can be done on top
> of this to avoid the now-unnecessary wrapper functions and use a test
> prerequisite for HTML tidy but I don't want to work on those unless
> there is agreement that this is a sensible direction.

I'm okay with merging this patch. But before I merge from wip to
master, it'd be nice to see the further changes -- eliminating a few
wrapper functions, using test prerequisites, etc. I applied this on
top of yours --
http://git.zx2c4.com/cgit/commit/?h=wip&id=2b9829453790425c98d3ad7427f82ba9f6446d01
-- but this is just a start.

[PATCH] tests: use Git's test framework

[Jason]

On Sun, Apr 7, 2013 at 5:28 PM, Jason A. Donenfeld <Jason at zx2c4.com> wrote:
> I'm okay with merging this patch. But before I merge from wip to
> master, it'd be nice to see the further changes -- eliminating a few
> wrapper functions, using test prerequisites, etc.

Please submit further patches for this against this branch:
http://git.zx2c4.com/cgit/log/?h=jk/use-git-test-suite