[PATCH 0/8] Support coverage testing with GCC/gcov

[PATCH 0/8] Support coverage testing with GCC/gcov

[Thomas Rast]

So I found a new toy to play with ;-)

The first patch adds some Makefile rules to automate coverage testing.
This initially resulted in a list of 497 untested functions.  Then I
picked out some easy targets and made patches for them.  This shortens
the list by 20.

Note that the rules currently don't consider any of the
subdirectories.  I'm not sure if arm, ppc, and compat are worth
considering, but it would at least have been nice to support it for
xdiff.  Unfortunately I can't convince the --coverage code to save
results there, so no analysis is possible.  (Running gcc 4.3.2 here.)

BTW, I find it a bit scary that fsck (see 5/8) has so few tests, and
even scarier that out of four simple tests I could come up with it
only passes two.  Let's hope the other two are unreasonable.


Thomas Rast (8):
  Support coverage testing with GCC/gcov
  Test that diff can read from stdin
  Test diff --dirstat functionality
  Test log --graph
  Test fsck a bit harder
  Test log --decorate
  Test rev-list --parents/--children
  Test git-patch-id

 Makefile                                      |   23 ++++
 t/t1450-fsck.sh                               |   65 +++++++++++
 t/t4002-diff-basic.sh                         |    8 ++
 t/t4013-diff-various.sh                       |    5 +
 t/t4013/diff.diff_--dirstat_master~1_master~2 |    3 +
 t/t4013/diff.log_--decorate_--all             |   34 ++++++
 t/t4013/diff.rev-list_--children_HEAD         |    7 +
 t/t4013/diff.rev-list_--parents_HEAD          |    7 +
 t/t4202-log.sh                                |  148 +++++++++++++++++++++++++
 t/t4203-patch-id.sh                           |   38 +++++++
 10 files changed, 338 insertions(+), 0 deletions(-)
 create mode 100644 t/t4013/diff.diff_--dirstat_master~1_master~2
 create mode 100644 t/t4013/diff.log_--decorate_--all
 create mode 100644 t/t4013/diff.rev-list_--children_HEAD
 create mode 100644 t/t4013/diff.rev-list_--parents_HEAD
 create mode 100755 t/t4203-patch-id.sh

[PATCH 1/8] Support coverage testing with GCC/gcov

[Thomas Rast]

With gcc's --coverage option, we can perform automatic coverage data
collection for the test suite.

Add a new Makefile target 'coverage' that scraps all previous coverage
results, recompiles git with the required compiler/linker flags (in
addition to any flags you specify manually), then runs the test suite
and compiles a report.

The compilation must be done with all optimizations disabled, since
inlined functions (and for line-by-line coverage, also optimized
branches/loops) break coverage tracking.

The tests are run serially (with -j1).  The coverage code should
theoretically allow concurrent access to its data files, but the
author saw random test failures.  Obviously this could be improved.

The report currently consists of a list of functions that were never
executed during the tests, which is written to
'coverage-untested-functions'.  Once this list becomes reasonably
short, we would also want to look at branches that were never taken.

Currently only toplevel *.c files are considered.  It would be nice to
at least include xdiff, but --coverage did not save data to
subdirectories on the system used to write this (gcc 4.3.2).

Signed-off-by: Thomas Rast <trast@student.ethz.ch>
---
 Makefile |   23 +++++++++++++++++++++++
 1 files changed, 23 insertions(+), 0 deletions(-)

diff --git a/Makefile b/Makefile
index b040a96..1c514a9 100644
--- a/Makefile
+++ b/Makefile
@@ -1640,3 +1640,26 @@ check-docs::
 check-builtins::
 	./check-builtins.sh
 
+### Test suite coverage testing
+#
+.PHONY: coverage coverage-clean coverage-build coverage-report
+
+coverage:
+	$(MAKE) coverage-build
+	$(MAKE) coverage-report
+
+coverage-clean:
+	rm -f *.gcda *.gcno
+
+COVERAGE_CFLAGS = $(CFLAGS) -O0 -ftest-coverage -fprofile-arcs
+COVERAGE_LDFLAGS = $(CFLAGS)  -O0 -lgcov
+
+coverage-build: coverage-clean
+	$(MAKE) CFLAGS="$(COVERAGE_CFLAGS)" LDFLAGS="$(COVERAGE_LDFLAGS)" \
+		-j1 all test
+
+coverage-report:
+	gcov -b *.c
+	grep '^function.*called 0 ' *.c.gcov \
+		| sed -e 's/\([^:]*\)\.gcov: *function \([^ ]*\) called.*/\1: \2/' \
+		| tee coverage-untested-functions
-- 
1.6.2.rc0.335.g1a2b

[PATCH 2/8] Test that diff can read from stdin

[Thomas Rast]

Signed-off-by: Thomas Rast <trast@student.ethz.ch>
---

Not the best possible place to put this, but t4013-diff-various.sh
doesn't let me do input redirections in its framework.

 t/t4002-diff-basic.sh |    8 ++++++++
 1 files changed, 8 insertions(+), 0 deletions(-)

diff --git a/t/t4002-diff-basic.sh b/t/t4002-diff-basic.sh
index cc3681f..18695ce 100755
--- a/t/t4002-diff-basic.sh
+++ b/t/t4002-diff-basic.sh
@@ -258,4 +258,12 @@ test_expect_success \
     git diff-tree -r -R $tree_A $tree_B >.test-b &&
     cmp -s .test-a .test-b'
 
+test_expect_success \
+    'diff can read from stdin' \
+    'test_must_fail git diff --no-index -- MN - < NN |
+        grep -v "^index" | sed "s#/-#/NN#" >.test-a &&
+    test_must_fail git diff --no-index -- MN NN |
+        grep -v "^index" >.test-b &&
+    test_cmp .test-a .test-b'
+
 test_done
-- 
1.6.2.rc0.335.g1a2b

[PATCH 3/8] Test diff --dirstat functionality

[Thomas Rast]

This is only a very rudimentary test, but it was untested before.

Signed-off-by: Thomas Rast <trast@student.ethz.ch>
---
 t/t4013-diff-various.sh                       |    1 +
 t/t4013/diff.diff_--dirstat_master~1_master~2 |    3 +++
 2 files changed, 4 insertions(+), 0 deletions(-)
 create mode 100644 t/t4013/diff.diff_--dirstat_master~1_master~2

diff --git a/t/t4013-diff-various.sh b/t/t4013-diff-various.sh
index aba5320..f140b9c 100755
--- a/t/t4013-diff-various.sh
+++ b/t/t4013-diff-various.sh
@@ -263,6 +263,7 @@ diff --name-status dir2 dir
 diff --no-index --name-status dir2 dir
 diff --no-index --name-status -- dir2 dir
 diff master master^ side
+diff --dirstat master~1 master~2
 EOF
 
 test_done
diff --git a/t/t4013/diff.diff_--dirstat_master~1_master~2 b/t/t4013/diff.diff_--dirstat_master~1_master~2
new file mode 100644
index 0000000..b672e1c
--- /dev/null
+++ b/t/t4013/diff.diff_--dirstat_master~1_master~2
@@ -0,0 +1,3 @@
+$ git diff --dirstat master~1 master~2
+  40.0% dir/
+$
-- 
1.6.2.rc0.335.g1a2b

[PATCH 4/8] Test log --graph

[Thomas Rast]

So far there were no tests checking that log --graph actually works.

Note that the tests strip trailing whitespace, as the current --graph
emits trailing whitespace on lines that do not contain anything but
graph lines.

Signed-off-by: Thomas Rast <trast@student.ethz.ch>
---
 t/t4202-log.sh |  148 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 148 insertions(+), 0 deletions(-)

diff --git a/t/t4202-log.sh b/t/t4202-log.sh
index 7b976ee..93966f7 100755
--- a/t/t4202-log.sh
+++ b/t/t4202-log.sh
@@ -134,5 +134,153 @@ test_expect_success 'log --grep -i' '
 	test_cmp expect actual
 '
 
+cat > expect <<EOF
+* Second
+* sixth
+* fifth
+* fourth
+* third
+* second
+* initial
+EOF
+
+test_expect_success 'simple log --graph' '
+	git log --graph --pretty=tformat:%s >actual &&
+	test_cmp expect actual
+'
+
+test_expect_success 'set up merge history' '
+	git checkout -b side HEAD~4 &&
+	test_commit side-1 1 1 &&
+	test_commit side-2 2 2 &&
+	git checkout master &&
+	git merge side
+'
+
+cat > expect <<\EOF
+*   Merge branch 'side'
+|\
+| * side-2
+| * side-1
+* | Second
+* | sixth
+* | fifth
+* | fourth
+|/
+* third
+* second
+* initial
+EOF
+
+test_expect_success 'log --graph with merge' '
+	git log --graph --date-order --pretty=tformat:%s |
+		sed "s/ *$//" >actual &&
+	test_cmp expect actual
+'
+
+cat > expect <<\EOF
+*   commit master
+|\  Merge: A B
+| | Author: A U Thor <author@example.com>
+| |
+| |     Merge branch 'side'
+| |
+| * commit side
+| | Author: A U Thor <author@example.com>
+| |
+| |     side-2
+| |
+| * commit tags/side-1
+| | Author: A U Thor <author@example.com>
+| |
+| |     side-1
+| |
+* | commit master~1
+| | Author: A U Thor <author@example.com>
+| |
+| |     Second
+| |
+* | commit master~2
+| | Author: A U Thor <author@example.com>
+| |
+| |     sixth
+| |
+* | commit master~3
+| | Author: A U Thor <author@example.com>
+| |
+| |     fifth
+| |
+* | commit master~4
+|/  Author: A U Thor <author@example.com>
+|
+|       fourth
+|
+* commit tags/side-1~1
+| Author: A U Thor <author@example.com>
+|
+|     third
+|
+* commit tags/side-1~2
+| Author: A U Thor <author@example.com>
+|
+|     second
+|
+* commit tags/side-1~3
+  Author: A U Thor <author@example.com>
+
+      initial
+EOF
+
+test_expect_success 'log --graph with full output' '
+	git log --graph --date-order --pretty=short |
+		git name-rev --name-only --stdin |
+		sed "s/Merge:.*/Merge: A B/;s/ *$//" >actual &&
+	test_cmp expect actual
+'
+
+test_expect_success 'set up more tangled history' '
+	git checkout -b tangle HEAD~6 &&
+	test_commit tangle-a tangle-a a &&
+	git merge master~3 &&
+	git merge side~1 &&
+	git checkout master &&
+	git merge tangle
+'
+
+cat > expect <<\EOF
+*   Merge branch 'tangle'
+|\
+| *   Merge branch 'side' (early part) into tangle
+| |\
+| * \   Merge branch 'master' (early part) into tangle
+| |\ \
+| * | | tangle-a
+* | | |   Merge branch 'side'
+|\ \ \ \
+| * | | | side-2
+| | | |/
+| | |/|
+| |/| |
+| * | | side-1
+* | | | Second
+* | | | sixth
+| | |/
+| |/|
+|/| |
+* | | fifth
+* | | fourth
+|/ /
+* | third
+|/
+* second
+* initial
+EOF
+
+test_expect_success 'log --graph with merge' '
+	git log --graph --date-order --pretty=tformat:%s |
+		sed "s/ *$//" >actual &&
+	test_cmp expect actual
+'
+
 test_done
 
-- 
1.6.2.rc0.335.g1a2b

[PATCH 5/8] Test fsck a bit harder

[Thomas Rast]

git-fsck, of all tools, has very few tests.  This adds some more:
* a corrupted object;
* a branch pointing to a non-commit;
* a tag pointing to a nonexistent object;
* and a tag pointing to an object of a type other than what the tag
  itself claims.

Only the first two are caught.  At least the third probably should,
too, but currently slips through.

Signed-off-by: Thomas Rast <trast@student.ethz.ch>
---
 t/t1450-fsck.sh |   65 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 65 insertions(+), 0 deletions(-)

diff --git a/t/t1450-fsck.sh b/t/t1450-fsck.sh
index 4597af0..302c910 100755
--- a/t/t1450-fsck.sh
+++ b/t/t1450-fsck.sh
@@ -28,4 +28,69 @@ test_expect_success 'loose objects borrowed from alternate are not missing' '
 	)
 '
 
+# Corruption tests follow.  Make sure to remove all traces of the
+# specific corruption you test afterwards, lest a later test trip over
+# it.
+
+test_expect_success 'object with bad sha1' '
+	sha=$(echo blob | git hash-object -w --stdin) &&
+	echo $sha &&
+	old=$(echo $sha | sed "s+^..+&/+") &&
+	new=$(dirname $old)/ffffffffffffffffffffffffffffffffffffff &&
+	sha="$(dirname $new)$(basename $new)"
+	mv .git/objects/$old .git/objects/$new &&
+	git update-index --add --cacheinfo 100644 $sha foo &&
+	tree=$(git write-tree) &&
+	cmt=$(echo bogus | git commit-tree $tree) &&
+	git update-ref refs/heads/bogus $cmt &&
+	(git fsck 2>out; true) &&
+	grep "$sha.*corrupt" out &&
+	rm -f .git/objects/$new &&
+	git update-ref -d refs/heads/bogus &&
+	git read-tree -u --reset HEAD
+'
+
+test_expect_success 'branch pointing to non-commit' '
+	git rev-parse HEAD^{tree} > .git/refs/heads/invalid &&
+	git fsck 2>out &&
+	grep "not a commit" out &&
+	git update-ref -d refs/heads/invalid
+'
+
+cat > invalid-tag <<EOF
+object ffffffffffffffffffffffffffffffffffffffff
+type commit
+tag invalid
+tagger T A Gger <tagger@example.com> 1234567890 -0000
+
+This is an invalid tag.
+EOF
+
+test_expect_failure 'tag pointing to nonexistent' '
+	tag=$(git hash-object -w --stdin < invalid-tag) &&
+	echo $tag > .git/refs/tags/invalid &&
+	git fsck --tags 2>out &&
+	grep "could not load tagged object" out &&
+	rm .git/refs/tags/invalid
+'
+
+cat > wrong-tag <<EOF
+object $(echo blob | git hash-object -w)
+type commit
+tag wrong
+tagger T A Gger <tagger@example.com> 1234567890 -0000
+
+This is an invalid tag.
+EOF
+
+test_expect_failure 'tag pointing to something else than its type' '
+	tag=$(git hash-object -w --stdin < wrong-tag) &&
+	echo $tag > .git/refs/tags/wrong &&
+	git fsck --tags 2>out &&
+	grep "some sane error message" out &&
+	rm .git/refs/tags/wrong
+'
+
+
+
 test_done
-- 
1.6.2.rc0.335.g1a2b

[PATCH 6/8] Test log --decorate

[Thomas Rast]

Signed-off-by: Thomas Rast <trast@student.ethz.ch>
---
 t/t4013-diff-various.sh           |    1 +
 t/t4013/diff.log_--decorate_--all |   34 ++++++++++++++++++++++++++++++++++
 2 files changed, 35 insertions(+), 0 deletions(-)
 create mode 100644 t/t4013/diff.log_--decorate_--all

diff --git a/t/t4013-diff-various.sh b/t/t4013-diff-various.sh
index f140b9c..e5715f3 100755
--- a/t/t4013-diff-various.sh
+++ b/t/t4013-diff-various.sh
@@ -203,6 +203,7 @@ log --root -c --patch-with-stat --summary master
 log --root --cc --patch-with-stat --summary master
 log -SF master
 log -SF -p master
+log --decorate --all
 
 whatchanged master
 whatchanged -p master
diff --git a/t/t4013/diff.log_--decorate_--all b/t/t4013/diff.log_--decorate_--all
new file mode 100644
index 0000000..12da8ac
--- /dev/null
+++ b/t/t4013/diff.log_--decorate_--all
@@ -0,0 +1,34 @@
+$ git log --decorate --all
+commit 59d314ad6f356dd08601a4cd5e530381da3e3c64 (refs/heads/master)
+Merge: 9a6d494 c7a2ab9
+Author: A U Thor <author@example.com>
+Date:   Mon Jun 26 00:04:00 2006 +0000
+
+    Merge branch 'side'
+
+commit c7a2ab9e8eac7b117442a607d5a9b3950ae34d5a (refs/heads/side)
+Author: A U Thor <author@example.com>
+Date:   Mon Jun 26 00:03:00 2006 +0000
+
+    Side
+
+commit 9a6d4949b6b76956d9d5e26f2791ec2ceff5fdc0
+Author: A U Thor <author@example.com>
+Date:   Mon Jun 26 00:02:00 2006 +0000
+
+    Third
+
+commit 1bde4ae5f36c8d9abe3a0fce0c6aab3c4a12fe44
+Author: A U Thor <author@example.com>
+Date:   Mon Jun 26 00:01:00 2006 +0000
+
+    Second
+    
+    This is the second commit.
+
+commit 444ac553ac7612cc88969031b02b3767fb8a353a (refs/heads/initial)
+Author: A U Thor <author@example.com>
+Date:   Mon Jun 26 00:00:00 2006 +0000
+
+    Initial
+$
-- 
1.6.2.rc0.335.g1a2b

[PATCH 7/8] Test rev-list --parents/--children

[Thomas Rast]

Signed-off-by: Thomas Rast <trast@student.ethz.ch>
---

rev-list doesn't really belong in diff-various, but it can be done
very easily in the existing framework, and there are log tests too...


 t/t4013-diff-various.sh               |    3 +++
 t/t4013/diff.rev-list_--children_HEAD |    7 +++++++
 t/t4013/diff.rev-list_--parents_HEAD  |    7 +++++++
 3 files changed, 17 insertions(+), 0 deletions(-)
 create mode 100644 t/t4013/diff.rev-list_--children_HEAD
 create mode 100644 t/t4013/diff.rev-list_--parents_HEAD

diff --git a/t/t4013-diff-various.sh b/t/t4013-diff-various.sh
index e5715f3..0f359ca 100755
--- a/t/t4013-diff-various.sh
+++ b/t/t4013-diff-various.sh
@@ -205,6 +205,9 @@ log -SF master
 log -SF -p master
 log --decorate --all
 
+rev-list --parents HEAD
+rev-list --children HEAD
+
 whatchanged master
 whatchanged -p master
 whatchanged --root master
diff --git a/t/t4013/diff.rev-list_--children_HEAD b/t/t4013/diff.rev-list_--children_HEAD
new file mode 100644
index 0000000..e7f17d5
--- /dev/null
+++ b/t/t4013/diff.rev-list_--children_HEAD
@@ -0,0 +1,7 @@
+$ git rev-list --children HEAD
+59d314ad6f356dd08601a4cd5e530381da3e3c64
+c7a2ab9e8eac7b117442a607d5a9b3950ae34d5a 59d314ad6f356dd08601a4cd5e530381da3e3c64
+9a6d4949b6b76956d9d5e26f2791ec2ceff5fdc0 59d314ad6f356dd08601a4cd5e530381da3e3c64
+1bde4ae5f36c8d9abe3a0fce0c6aab3c4a12fe44 9a6d4949b6b76956d9d5e26f2791ec2ceff5fdc0
+444ac553ac7612cc88969031b02b3767fb8a353a 1bde4ae5f36c8d9abe3a0fce0c6aab3c4a12fe44 c7a2ab9e8eac7b117442a607d5a9b3950ae34d5a
+$
diff --git a/t/t4013/diff.rev-list_--parents_HEAD b/t/t4013/diff.rev-list_--parents_HEAD
new file mode 100644
index 0000000..65d2a80
--- /dev/null
+++ b/t/t4013/diff.rev-list_--parents_HEAD
@@ -0,0 +1,7 @@
+$ git rev-list --parents HEAD
+59d314ad6f356dd08601a4cd5e530381da3e3c64 9a6d4949b6b76956d9d5e26f2791ec2ceff5fdc0 c7a2ab9e8eac7b117442a607d5a9b3950ae34d5a
+c7a2ab9e8eac7b117442a607d5a9b3950ae34d5a 444ac553ac7612cc88969031b02b3767fb8a353a
+9a6d4949b6b76956d9d5e26f2791ec2ceff5fdc0 1bde4ae5f36c8d9abe3a0fce0c6aab3c4a12fe44
+1bde4ae5f36c8d9abe3a0fce0c6aab3c4a12fe44 444ac553ac7612cc88969031b02b3767fb8a353a
+444ac553ac7612cc88969031b02b3767fb8a353a
+$
-- 
1.6.2.rc0.335.g1a2b

[PATCH 8/8] Test git-patch-id

[Thomas Rast]

So far, git-patch-id was untested.  Add some simple checks for output
format and patch (in)equality.

Signed-off-by: Thomas Rast <trast@student.ethz.ch>
---
 t/t4203-patch-id.sh |   38 ++++++++++++++++++++++++++++++++++++++
 1 files changed, 38 insertions(+), 0 deletions(-)
 create mode 100755 t/t4203-patch-id.sh

diff --git a/t/t4203-patch-id.sh b/t/t4203-patch-id.sh
new file mode 100755
index 0000000..04f7bae
--- /dev/null
+++ b/t/t4203-patch-id.sh
@@ -0,0 +1,38 @@
+#!/bin/sh
+
+test_description='git patch-id'
+
+. ./test-lib.sh
+
+test_expect_success 'setup' '
+	test_commit initial foo a &&
+	test_commit first foo b &&
+	git checkout -b same HEAD^ &&
+	test_commit same-msg foo b &&
+	git checkout -b notsame HEAD^ &&
+	test_commit notsame-msg foo c
+'
+
+test_expect_success 'patch-id output is well-formed' '
+	git log -p -1 | git patch-id > output &&
+	grep "^[a-f0-9]\{40\} $(git rev-parse HEAD)$" output
+'
+
+get_patch_id () {
+	git log -p -1 "$1" | git patch-id |
+		sed "s# .*##" > patch-id_"$1"
+}
+
+test_expect_success 'patch-id detects equality' '
+	get_patch_id master &&
+	get_patch_id same &&
+	test_cmp patch-id_master patch-id_same
+'
+
+test_expect_success 'patch-id detects inequality' '
+	get_patch_id master &&
+	get_patch_id notsame &&
+	! test_cmp patch-id_master patch-id_notsame
+'
+
+test_done
-- 
1.6.2.rc0.335.g1a2b

Re: [PATCH 1/8] Support coverage testing with GCC/gcov

[Matthieu Moy]

detail:

Thomas Rast <trast@student.ethz.ch> writes:

> With gcc's --coverage option, we can perform automatic coverage data

I don't think gcc has a --coverage ...

> +COVERAGE_CFLAGS = $(CFLAGS) -O0 -ftest-coverage -fprofile-arcs
> +COVERAGE_LDFLAGS = $(CFLAGS)  -O0 -lgcov

... and this is not the one you are using anyway.

(but great thing to use gcov! thanks)

-- 
Matthieu

Re: [PATCH 1/8] Support coverage testing with GCC/gcov

[Thomas Rast]

[-- Attachment #1: Type: text/plain, Size: 784 bytes --]

Matthieu Moy wrote:
> detail:
> 
> Thomas Rast <trast@student.ethz.ch> writes:
> 
> > With gcc's --coverage option, we can perform automatic coverage data
> 
> I don't think gcc has a --coverage ...

Mine does:

  --coverage
    This option is used to compile and link code instrumented for
    coverage analysis.  The option is a synonym for -fprofile-arcs
    -ftest-coverage (when compiling) and -lgcov (when linking).  See
    the documentation for those options for more details.

> > +COVERAGE_CFLAGS = $(CFLAGS) -O0 -ftest-coverage -fprofile-arcs
> > +COVERAGE_LDFLAGS = $(CFLAGS)  -O0 -lgcov
> 
> ... and this is not the one you are using anyway.

Indeed, I guess the log message is somewhat misleading.

-- 
Thomas Rast
trast@{inf,student}.ethz.ch

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 197 bytes --]