[PATCH 0/3] t7400-submodule-basic.sh cleanups

[PATCH 0/3] t7400-submodule-basic.sh cleanups

[Jonathan Nieder]

The 'update --init' test from t7400 is failing on a few machines
I don’t have access to.  It’s hard to say what the dangerous
ingredient is --- maybe a libc bug or something. [1]

I took the opportunity to clarify what some of the tests do and to
make the output a little more helpful.

This series has three patches for easier review, but it might make
sense to squash them; alternatively, it might make sense to split the
test into several files.

What do you think?

Jonathan Nieder (3):
  t7400: split setup into multiple tests
  t7400: clarify 'submodule add' tests
  t7400: clarify submodule update tests

 t/t7400-submodule-basic.sh |  331 +++++++++++++++++++++++++++-----------------
 1 files changed, 201 insertions(+), 130 deletions(-)

[1] You can see some logged test failures at [2] [3], though it isn’t
very illuminating.  Well, that’s more of something for a Debian
Developer or someone with access to a similar machine to investigate.
[2] <https://buildd.debian.org/build.cgi?pkg=git-core>
[3] <http://bugs.debian.org/569594>.

[PATCH 1/3] t7400: split setup into multiple tests

[Jonathan Nieder]

The setup in t7400-submodule-basic does a number of different
things to support different tests.  Splitting it up makes the
test a little easier to read and should provide an opportunity
to move each piece of setup closer to the tests that require it.

Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
 t/t7400-submodule-basic.sh |   58 ++++++++++++++++++++++---------------------
 1 files changed, 30 insertions(+), 28 deletions(-)

diff --git a/t/t7400-submodule-basic.sh b/t/t7400-submodule-basic.sh
index 1a4dc5f..1f468e5 100755
--- a/t/t7400-submodule-basic.sh
+++ b/t/t7400-submodule-basic.sh
@@ -11,40 +11,42 @@ subcommands of git submodule.
 
 . ./test-lib.sh
 
-#
-# Test setup:
-#  -create a repository in directory init
-#  -add a couple of files
-#  -add directory init to 'superproject', this creates a DIRLINK entry
-#  -add a couple of regular files to enable testing of submodule filtering
-#  -mv init subrepo
-#  -add an entry to .gitmodules for submodule 'example'
-#
-test_expect_success 'Prepare submodule testing' '
-	: > t &&
+test_expect_success 'setup - initial commit' '
+	>t &&
 	git add t &&
 	git commit -m "initial commit" &&
-	git branch initial HEAD &&
+	git branch initial
+'
+
+test_expect_success 'setup - repository in init subdirectory' '
 	mkdir init &&
-	cd init &&
-	git init &&
-	echo a >a &&
-	git add a &&
-	git commit -m "submodule commit 1" &&
-	git tag -a -m "rev-1" rev-1 &&
-	rev1=$(git rev-parse HEAD) &&
-	if test -z "$rev1"
-	then
-		echo "[OOPS] submodule git rev-parse returned nothing"
-		false
-	fi &&
-	cd .. &&
+	(
+		cd init &&
+		git init &&
+		echo a >a &&
+		git add a &&
+		git commit -m "submodule commit 1" &&
+		git tag -a -m "rev-1" rev-1
+	)
+	rev1=$(cd init && git rev-parse HEAD) &&
+	printf "rev1: %s\n" "$rev1" &&
+	test -n "$rev1"
+'
+
+test_expect_success 'setup - commit with gitlink' '
 	echo a >a &&
 	echo z >z &&
 	git add a init z &&
-	git commit -m "super commit 1" &&
-	mv init .subrepo &&
-	GIT_CONFIG=.gitmodules git config submodule.example.url git://example.com/init.git
+	git commit -m "super commit 1"
+'
+
+test_expect_success 'setup - hide init subdirectory' '
+	mv init .subrepo
+'
+
+test_expect_success 'setup - add an example entry to .gitmodules' '
+	GIT_CONFIG=.gitmodules \
+	git config submodule.example.url git://example.com/init.git
 '
 
 test_expect_success 'Prepare submodule add testing' '
-- 
1.7.0.4

[PATCH 2/3] t7400: clarify 'submodule add' tests

[Jonathan Nieder]

A new reader may not realize what properties the $submodurl
repository needs to have.

One of the tests is checking that ‘submodule add -b foo’ creates
a ‘foo’ branch.  Put this test in context by checking that
without -b, no ‘foo’ branch is created.

While at it, make sure each added submodule is a reasonable
repository, with clean index, no stray files, and so on.

Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
 t/t7400-submodule-basic.sh |  103 +++++++++++++++++++++++++++++++++++++------
 1 files changed, 88 insertions(+), 15 deletions(-)

diff --git a/t/t7400-submodule-basic.sh b/t/t7400-submodule-basic.sh
index 1f468e5..dbc8338 100755
--- a/t/t7400-submodule-basic.sh
+++ b/t/t7400-submodule-basic.sh
@@ -49,63 +49,136 @@ test_expect_success 'setup - add an example entry to .gitmodules' '
 	git config submodule.example.url git://example.com/init.git
 '
 
-test_expect_success 'Prepare submodule add testing' '
-	submodurl=$(pwd)
+test_expect_success 'setup - repository to add submodules to' '
+	git init addtest
+'
+
+# The 'submodule add' tests need some repository to add as a submodule.
+# The trash directory is a good one as any.
+submodurl=$TRASH_DIRECTORY
+
+listbranches() {
+	git for-each-ref --format='%(refname)' 'refs/heads/*'
+}
+
+inspect() {
+	dir=$1 &&
+	dotdot="${2:-..}" &&
+
 	(
-		mkdir addtest &&
-		cd addtest &&
-		git init
+		cd "$dir" &&
+		listbranches >"$dotdot/heads" &&
+		{ git symbolic-ref HEAD || :; } >"$dotdot/head" &&
+		git update-index --refresh &&
+		git diff-files --exit-code &&
+		git clean -n -d -x >"$dotdot/untracked"
 	)
-'
+}
 
+rm -f heads head untracked
 test_expect_success 'submodule add' '
+	echo "refs/heads/master" >expect &&
+	>empty &&
+
 	(
 		cd addtest &&
 		git submodule add "$submodurl" submod &&
 		git submodule init
-	)
+	) &&
+
+	inspect addtest/submod ../.. &&
+	test_cmp expect heads &&
+	test_cmp expect head &&
+	test_cmp empty untracked
 '
 
+rm -f heads head untracked
 test_expect_success 'submodule add --branch' '
+	echo "refs/heads/initial" >expect-head &&
+	cat <<-\EOF >expect-heads &&
+	refs/heads/initial
+	refs/heads/master
+	EOF
+	>empty &&
+
 	(
 		cd addtest &&
 		git submodule add -b initial "$submodurl" submod-branch &&
-		git submodule init &&
-		cd submod-branch &&
-		git branch | grep initial
-	)
+		git submodule init
+	) &&
+
+	inspect addtest/submod-branch ../.. &&
+	test_cmp expect-heads heads &&
+	test_cmp expect-head head &&
+	test_cmp empty untracked
 '
 
+rm -f heads head untracked
 test_expect_success 'submodule add with ./ in path' '
+	echo "refs/heads/master" >expect &&
+	>empty &&
+
 	(
 		cd addtest &&
 		git submodule add "$submodurl" ././dotsubmod/./frotz/./ &&
 		git submodule init
-	)
+	) &&
+
+	inspect addtest/dotsubmod/frotz ../../.. &&
+	test_cmp expect heads &&
+	test_cmp expect head &&
+	test_cmp empty untracked
 '
 
+rm -f heads head untracked
 test_expect_success 'submodule add with // in path' '
+	echo "refs/heads/master" >expect &&
+	>empty &&
+
 	(
 		cd addtest &&
 		git submodule add "$submodurl" slashslashsubmod///frotz// &&
 		git submodule init
-	)
+	) &&
+
+	inspect addtest/slashslashsubmod/frotz ../../.. &&
+	test_cmp expect heads &&
+	test_cmp expect head &&
+	test_cmp empty untracked
 '
 
+rm -f heads head untracked
 test_expect_success 'submodule add with /.. in path' '
+	echo "refs/heads/master" >expect &&
+	>empty &&
+
 	(
 		cd addtest &&
 		git submodule add "$submodurl" dotdotsubmod/../realsubmod/frotz/.. &&
 		git submodule init
-	)
+	) &&
+
+	inspect addtest/realsubmod ../.. &&
+	test_cmp expect heads &&
+	test_cmp expect head &&
+	test_cmp empty untracked
 '
 
+rm -f heads head untracked
 test_expect_success 'submodule add with ./, /.. and // in path' '
+	echo "refs/heads/master" >expect &&
+	>empty &&
+
 	(
 		cd addtest &&
 		git submodule add "$submodurl" dot/dotslashsubmod/./../..////realsubmod2/a/b/c/d/../../../../frotz//.. &&
 		git submodule init
-	)
+	) &&
+
+	inspect addtest/realsubmod2 ../.. &&
+	test_cmp expect heads &&
+	test_cmp expect head &&
+	test_cmp empty untracked
 '
 
 test_expect_success 'status should fail for unmapped paths' '
-- 
1.7.0.4

[PATCH 3/3] t7400: clarify submodule update tests

[Jonathan Nieder]

In particular, add a missing && to the update --init test.

The goal is to make it clearer what happened when one of these
tests fails.  The update --init test is currently (consistently)
failing on a few unusual machines.

Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
 t/t7400-submodule-basic.sh |  186 +++++++++++++++++++++----------------------
 1 files changed, 91 insertions(+), 95 deletions(-)

diff --git a/t/t7400-submodule-basic.sh b/t/t7400-submodule-basic.sh
index dbc8338..55590ab 100755
--- a/t/t7400-submodule-basic.sh
+++ b/t/t7400-submodule-basic.sh
@@ -28,9 +28,6 @@ test_expect_success 'setup - repository in init subdirectory' '
 		git commit -m "submodule commit 1" &&
 		git tag -a -m "rev-1" rev-1
 	)
-	rev1=$(cd init && git rev-parse HEAD) &&
-	printf "rev1: %s\n" "$rev1" &&
-	test -n "$rev1"
 '
 
 test_expect_success 'setup - commit with gitlink' '
@@ -44,11 +41,6 @@ test_expect_success 'setup - hide init subdirectory' '
 	mv init .subrepo
 '
 
-test_expect_success 'setup - add an example entry to .gitmodules' '
-	GIT_CONFIG=.gitmodules \
-	git config submodule.example.url git://example.com/init.git
-'
-
 test_expect_success 'setup - repository to add submodules to' '
 	git init addtest
 '
@@ -69,6 +61,7 @@ inspect() {
 		cd "$dir" &&
 		listbranches >"$dotdot/heads" &&
 		{ git symbolic-ref HEAD || :; } >"$dotdot/head" &&
+		git rev-parse HEAD >"$dotdot/head-sha1" &&
 		git update-index --refresh &&
 		git diff-files --exit-code &&
 		git clean -n -d -x >"$dotdot/untracked"
@@ -181,131 +174,129 @@ test_expect_success 'submodule add with ./, /.. and // in path' '
 	test_cmp empty untracked
 '
 
+test_expect_success 'setup - add an example entry to .gitmodules' '
+	GIT_CONFIG=.gitmodules \
+	git config submodule.example.url git://example.com/init.git
+'
+
 test_expect_success 'status should fail for unmapped paths' '
-	if git submodule status
-	then
-		echo "[OOPS] submodule status succeeded"
-		false
-	elif ! GIT_CONFIG=.gitmodules git config submodule.example.path init
-	then
-		echo "[OOPS] git config failed to update .gitmodules"
-		false
-	fi
+	test_must_fail git submodule status
+'
+
+test_expect_success 'setup - map path in .gitmodules' '
+	cat <<\EOF >expect &&
+[submodule "example"]
+	url = git://example.com/init.git
+	path = init
+EOF
+
+	GIT_CONFIG=.gitmodules git config submodule.example.path init &&
+
+	test_cmp expect .gitmodules
 '
 
 test_expect_success 'status should only print one line' '
-	lines=$(git submodule status | wc -l) &&
-	test $lines = 1
+	git submodule status >lines &&
+	test $(wc -l <lines) = 1
+'
+
+test_expect_success 'setup - fetch commit name from submodule' '
+	rev1=$(cd .subrepo && git rev-parse HEAD) &&
+	printf "rev1: %s\n" "$rev1" &&
+	test -n "$rev1"
 '
 
 test_expect_success 'status should initially be "missing"' '
-	git submodule status | grep "^-$rev1"
+	git submodule status >lines &&
+	grep "^-$rev1" lines
 '
 
 test_expect_success 'init should register submodule url in .git/config' '
+	echo git://example.com/init.git >expect &&
+
 	git submodule init &&
-	url=$(git config submodule.example.url) &&
-	if test "$url" != "git://example.com/init.git"
-	then
-		echo "[OOPS] init succeeded but submodule url is wrong"
-		false
-	elif test_must_fail git config submodule.example.url ./.subrepo
-	then
-		echo "[OOPS] init succeeded but update of url failed"
-		false
-	fi
+	git config submodule.example.url >url &&
+	git config submodule.example.url ./.subrepo &&
+
+	test_cmp expect url
 '
 
 test_expect_success 'update should fail when path is used by a file' '
+	echo hello >expect &&
+
 	echo "hello" >init &&
-	if git submodule update
-	then
-		echo "[OOPS] update should have failed"
-		false
-	elif test "$(cat init)" != "hello"
-	then
-		echo "[OOPS] update failed but init file was molested"
-		false
-	else
-		rm init
-	fi
+	test_must_fail git submodule update &&
+
+	test_cmp expect init
 '
 
+rm -fr init
 test_expect_success 'update should fail when path is used by a nonempty directory' '
+	echo hello >expect &&
+
 	mkdir init &&
 	echo "hello" >init/a &&
-	if git submodule update
-	then
-		echo "[OOPS] update should have failed"
-		false
-	elif test "$(cat init/a)" != "hello"
-	then
-		echo "[OOPS] update failed but init/a was molested"
-		false
-	else
-		rm init/a
-	fi
+
+	test_must_fail git submodule update &&
+
+	test_cmp expect init/a
 '
 
+rm -fr init
+rm -f head-sha1
 test_expect_success 'update should work when path is an empty dir' '
-	rm -rf init &&
+	echo "$rev1" >expect
+
 	mkdir init &&
 	git submodule update &&
-	head=$(cd init && git rev-parse HEAD) &&
-	if test -z "$head"
-	then
-		echo "[OOPS] Failed to obtain submodule head"
-		false
-	elif test "$head" != "$rev1"
-	then
-		echo "[OOPS] Submodule head is $head but should have been $rev1"
-		false
-	fi
+
+	inspect init &&
+	test_cmp expect head-sha1
 '
 
 test_expect_success 'status should be "up-to-date" after update' '
-	git submodule status | grep "^ $rev1"
+	git submodule status >list &&
+	grep "^ $rev1" list
 '
 
 test_expect_success 'status should be "modified" after submodule commit' '
-	cd init &&
-	echo b >b &&
-	git add b &&
-	git commit -m "submodule commit 2" &&
-	rev2=$(git rev-parse HEAD) &&
-	cd .. &&
-	if test -z "$rev2"
-	then
-		echo "[OOPS] submodule git rev-parse returned nothing"
-		false
-	fi &&
-	git submodule status | grep "^+$rev2"
+	(
+		cd init &&
+		echo b >b &&
+		git add b &&
+		git commit -m "submodule commit 2"
+	) &&
+
+	rev2=$(cd init && git rev-parse HEAD) &&
+	test -n "$rev2" &&
+	git submodule status >list &&
+
+	grep "^+$rev2" list
 '
 
 test_expect_success 'the --cached sha1 should be rev1' '
-	git submodule --cached status | grep "^+$rev1"
+	git submodule --cached status >list &&
+	grep "^+$rev1" list
 '
 
 test_expect_success 'git diff should report the SHA1 of the new submodule commit' '
-	git diff | grep "^+Subproject commit $rev2"
+	git diff >diff &&
+	grep "^+Subproject commit $rev2" diff
 '
 
+rm -f head-sha1
 test_expect_success 'update should checkout rev1' '
+	echo "$rev1" >expect &&
+
 	git submodule update init &&
-	head=$(cd init && git rev-parse HEAD) &&
-	if test -z "$head"
-	then
-		echo "[OOPS] submodule git rev-parse returned nothing"
-		false
-	elif test "$head" != "$rev1"
-	then
-		echo "[OOPS] init did not checkout correct head"
-		false
-	fi
+	inspect init &&
+
+	test_cmp expect head-sha1
 '
 
 test_expect_success 'status should be "up-to-date" after update' '
-	git submodule status | grep "^ $rev1"
+	git submodule status >list &&
+	grep "^ $rev1" list
 '
 
 test_expect_success 'checkout superproject with subproject already present' '
@@ -314,6 +305,8 @@ test_expect_success 'checkout superproject with subproject already present' '
 '
 
 test_expect_success 'apply submodule diff' '
+	>empty &&
+
 	git branch second &&
 	(
 		cd init &&
@@ -326,21 +319,24 @@ test_expect_success 'apply submodule diff' '
 	git format-patch -1 --stdout >P.diff &&
 	git checkout second &&
 	git apply --index P.diff &&
-	D=$(git diff --cached master) &&
-	test -z "$D"
+
+	git diff --cached master >staged &&
+	test_cmp empty staged
 '
 
 test_expect_success 'update --init' '
-
 	mv init init2 &&
 	git config -f .gitmodules submodule.example.url "$(pwd)/init2" &&
-	git config --remove-section submodule.example
+	git config --remove-section submodule.example &&
+	test_must_fail git config submodule.example.url &&
+
 	git submodule update init > update.out &&
+	cat update.out &&
 	grep "not initialized" update.out &&
-	test ! -d init/.git &&
+	! test -d init/.git &&
+
 	git submodule update --init init &&
 	test -d init/.git
-
 '
 
 test_expect_success 'do not add files from a submodule' '
-- 
1.7.0.4