git/list[1] front-page[2] threads[3] people[4] search[5] about
 

[PATCH 1/2] tests: eliminate unnecessary setup test assertions

From
Jonathan Nieder <jrnieder@gmail.com>
Date
May 6, 2011, 20:58 UTC
Message-ID
<20110506205851.GB20182@elie>
In-Reply-To
<20110506205441.GA20182@elie>

Most of git's tests write files and define shell functions and variables that will last throughout a test script at the top of the script, before all test assertions:

	. ./test-lib.sh
	VAR='some value'
	export VAR
	>empty
	fn () {
		do something
	}
	test_expect_success 'setup' '
		... nontrivial commands go here ...
	'

Two scripts use a different style with this kind of trivial code enclosed by a test assertion; fix them. The usual style is easier to read since there is less indentation to keep track of and no need to worry about nested quotes; and on the other hand, because the commands in question are trivial, it should not make the test suite any worse at catching future bugs in git.

While at it, make some other small tweaks:
 - spell function definitions with a space before () for consistency
   with other scripts;
 - use the self-contained command "git mktree </dev/null" in
   preference to "git write-tree" which looks at the index when
   writing an empty tree.
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
I should have done this long ago.  Sorry for the eyesore.
 t/t6010-merge-base.sh |   62 +++++++++++-----------
 t/t7600-merge.sh      |  134 ++++++++++++++++++++++++-------------------------
 2 files changed, 97 insertions(+), 99 deletions(-)
diff --git a/t/t6010-merge-base.sh b/t/t6010-merge-base.sh
index 082032e..f80bba8 100755
--- a/t/t6010-merge-base.sh
+++ b/t/t6010-merge-base.sh
@@ -8,38 +8,38 @@ test_description='Merge base and parent list computation.
 
 . ./test-lib.sh
 
+M=1130000000
+Z=+0000
+
+GIT_COMMITTER_EMAIL=git@comm.iter.xz
+GIT_COMMITTER_NAME='C O Mmiter'
+GIT_AUTHOR_NAME='A U Thor'
+GIT_AUTHOR_EMAIL=git@au.thor.xz
+export GIT_COMMITTER_EMAIL GIT_COMMITTER_NAME GIT_AUTHOR_NAME GIT_AUTHOR_EMAIL
+
+doit () {
+	OFFSET=$1 &&
+	NAME=$2 &&
+	shift 2 &&
+
+	PARENTS= &&
+	for P
+	do
+		PARENTS="${PARENTS}-p $P "
+	done &&
+
+	GIT_COMMITTER_DATE="$(($M + $OFFSET)) $Z" &&
+	GIT_AUTHOR_DATE=$GIT_COMMITTER_DATE &&
+	export GIT_COMMITTER_DATE GIT_AUTHOR_DATE &&
+
+	commit=$(echo $NAME | git commit-tree $T $PARENTS) &&
+
+	echo $commit >.git/refs/tags/$NAME &&
+	echo $commit
+}
+
 test_expect_success 'setup' '
-	T=$(git write-tree) &&
-
-	M=1130000000 &&
-	Z=+0000 &&
-
-	GIT_COMMITTER_EMAIL=git@comm.iter.xz &&
-	GIT_COMMITTER_NAME="C O Mmiter" &&
-	GIT_AUTHOR_NAME="A U Thor" &&
-	GIT_AUTHOR_EMAIL=git@au.thor.xz &&
-	export GIT_COMMITTER_EMAIL GIT_COMMITTER_NAME GIT_AUTHOR_NAME GIT_AUTHOR_EMAIL &&
-
-	doit() {
-		OFFSET=$1 &&
-		NAME=$2 &&
-		shift 2 &&
-
-		PARENTS= &&
-		for P
-		do
-			PARENTS="${PARENTS}-p $P "
-		done &&
-
-		GIT_COMMITTER_DATE="$(($M + $OFFSET)) $Z" &&
-		GIT_AUTHOR_DATE=$GIT_COMMITTER_DATE &&
-		export GIT_COMMITTER_DATE GIT_AUTHOR_DATE &&
-
-		commit=$(echo $NAME | git commit-tree $T $PARENTS) &&
-
-		echo $commit >.git/refs/tags/$NAME &&
-		echo $commit
-	}
+	T=$(git mktree </dev/null)
 '
 
 test_expect_success 'set up G and H' '
diff --git a/t/t7600-merge.sh b/t/t7600-merge.sh
index 87d5d78..c665acd 100755
--- a/t/t7600-merge.sh
+++ b/t/t7600-merge.sh
@@ -28,80 +28,78 @@ Testing basic merge operations/option parsing.
 
 . ./test-lib.sh
 
-test_expect_success 'set up test data and helpers' '
-	printf "%s\n" 1 2 3 4 5 6 7 8 9 >file &&
-	printf "%s\n" "1 X" 2 3 4 5 6 7 8 9 >file.1 &&
-	printf "%s\n" 1 2 3 4 "5 X" 6 7 8 9 >file.5 &&
-	printf "%s\n" 1 2 3 4 5 6 7 8 "9 X" >file.9 &&
-	printf "%s\n" "1 X" 2 3 4 5 6 7 8 9 >result.1 &&
-	printf "%s\n" "1 X" 2 3 4 "5 X" 6 7 8 9 >result.1-5 &&
-	printf "%s\n" "1 X" 2 3 4 "5 X" 6 7 8 "9 X" >result.1-5-9 &&
+printf '%s\n' 1 2 3 4 5 6 7 8 9 >file
+printf '%s\n' '1 X' 2 3 4 5 6 7 8 9 >file.1
+printf '%s\n' 1 2 3 4 '5 X' 6 7 8 9 >file.5
+printf '%s\n' 1 2 3 4 5 6 7 8 '9 X' >file.9
+printf '%s\n' '1 X' 2 3 4 5 6 7 8 9 >result.1
+printf '%s\n' '1 X' 2 3 4 '5 X' 6 7 8 9 >result.1-5
+printf '%s\n' '1 X' 2 3 4 '5 X' 6 7 8 '9 X' >result.1-5-9
 
-	create_merge_msgs() {
-		echo "Merge commit '\''c2'\''" >msg.1-5 &&
-		echo "Merge commit '\''c2'\''; commit '\''c3'\''" >msg.1-5-9 &&
-		{
-			echo "Squashed commit of the following:" &&
-			echo &&
-			git log --no-merges ^HEAD c1
-		} >squash.1 &&
-		{
-			echo "Squashed commit of the following:" &&
-			echo &&
-			git log --no-merges ^HEAD c2
-		} >squash.1-5 &&
-		{
-			echo "Squashed commit of the following:" &&
-			echo &&
-			git log --no-merges ^HEAD c2 c3
-		} >squash.1-5-9 &&
-		echo >msg.nolog &&
-		{
-			echo "* commit '\''c3'\'':" &&
-			echo "  commit 3" &&
-			echo
-		} >msg.log
-	} &&
+create_merge_msgs () {
+	echo "Merge commit 'c2'" >msg.1-5 &&
+	echo "Merge commit 'c2'; commit 'c3'" >msg.1-5-9 &&
+	{
+		echo "Squashed commit of the following:" &&
+		echo &&
+		git log --no-merges ^HEAD c1
+	} >squash.1 &&
+	{
+		echo "Squashed commit of the following:" &&
+		echo &&
+		git log --no-merges ^HEAD c2
+	} >squash.1-5 &&
+	{
+		echo "Squashed commit of the following:" &&
+		echo &&
+		git log --no-merges ^HEAD c2 c3
+	} >squash.1-5-9 &&
+	echo >msg.nolog &&
+	{
+		echo "* commit 'c3':" &&
+		echo "  commit 3" &&
+		echo
+	} >msg.log
+}
 
-	verify_merge() {
-		test_cmp "$2" "$1" &&
-		git update-index --refresh &&
-		git diff --exit-code &&
-		if test -n "$3"
-		then
-			git show -s --pretty=format:%s HEAD >msg.act &&
-			test_cmp "$3" msg.act
-		fi
-	} &&
+verify_merge () {
+	test_cmp "$2" "$1" &&
+	git update-index --refresh &&
+	git diff --exit-code &&
+	if test -n "$3"
+	then
+		git show -s --pretty=format:%s HEAD >msg.act &&
+		test_cmp "$3" msg.act
+	fi
+}
 
-	verify_head() {
-		echo "$1" >head.expected &&
-		git rev-parse HEAD >head.actual &&
-		test_cmp head.expected head.actual
-	} &&
+verify_head () {
+	echo "$1" >head.expected &&
+	git rev-parse HEAD >head.actual &&
+	test_cmp head.expected head.actual
+}
 
-	verify_parents() {
-		printf "%s\n" "$@" >parents.expected &&
-		>parents.actual &&
-		i=1 &&
-		while test $i -le $#
-		do
-			git rev-parse HEAD^$i >>parents.actual &&
-			i=$(expr $i + 1) ||
-			return 1
-		done &&
-		test_cmp parents.expected parents.actual
-	} &&
+verify_parents () {
+	printf '%s\n' "$@" >parents.expected &&
+	>parents.actual &&
+	i=1 &&
+	while test $i -le $#
+	do
+		git rev-parse HEAD^$i >>parents.actual &&
+		i=$(expr $i + 1) ||
+		return 1
+	done &&
+	test_cmp parents.expected parents.actual
+}
 
-	verify_mergeheads() {
-		printf "%s\n" "$@" >mergehead.expected &&
-		test_cmp mergehead.expected .git/MERGE_HEAD
-	} &&
+verify_mergeheads () {
+	printf '%s\n' "$@" >mergehead.expected &&
+	test_cmp mergehead.expected .git/MERGE_HEAD
+}
 
-	verify_no_mergehead() {
-		! test -e .git/MERGE_HEAD
-	}
-'
+verify_no_mergehead () {
+	! test -e .git/MERGE_HEAD
+}
 
 test_expect_success 'setup' '
 	git add file &&

-- 
1.7.5.1
Previous: Jonathan NiederNext: Jeff King
Message 30 of 46 in “Add default merge options for all branches”
  1. Add default merge options for all branchesMichael Grubb, May 2, 2011
  2. Miklos VajnaMay 2, 2011
  3. Junio C HamanoMay 2, 2011
  4. Michael GrubbMay 3, 2011
  5. Add default merge options for all branchesMichael Grubb, May 3, 2011
  6. Jonathan NiederMay 3, 2011
  7. Jonathan NiederMay 3, 2011
  8. Michael GrubbMay 3, 2011
  9. Junio C HamanoMay 3, 2011
  10. Michael GrubbMay 3, 2011
  11. Jonathan NiederMay 3, 2011
  12. Jens LehmannMay 3, 2011
  13. Add default merge options for all branchesMichael Grubb, May 3, 2011
  14. Michael GrubbMay 3, 2011
  15. Jonathan NiederMay 3, 2011
  16. Junio C HamanoMay 3, 2011
  17. Junio C HamanoMay 4, 2011
  18. Michael GrubbMay 4, 2011
  19. Jonathan NiederMay 4, 2011
  20. Michael GrubbMay 4, 2011
  21. Junio C HamanoMay 4, 2011
  22. John SzakmeisterMay 4, 2011
  23. Junio C HamanoMay 3, 2011
  24. Add default merge options for all branchesMichael Grubb, May 3, 2011
  25. Add default merge options for all branchesMichael Grubb, May 4, 2011
  26. Junio C HamanoMay 5, 2011
  27. Junio C HamanoMay 6, 2011
  28. Jonathan NiederMay 6, 2011
  29. 0/2 tests: make verify_merge check that the number of parents is rightJonathan Nieder, May 6, 2011
  30. 1/2 tests: eliminate unnecessary setup test assertionsJonathan Nieder, May 6, 2011
  31. Jeff KingMay 6, 2011
  32. Jeff KingMay 6, 2011
  33. Junio C HamanoMay 6, 2011
  34. Jeff KingMay 6, 2011
  35. Junio C HamanoMay 7, 2011
  36. 0/3 blame --line-porcelainJeff King, May 9, 2011
  37. 1/3 add tests for various blame formatsJeff King, May 9, 2011
  38. 2/3 blame: refactor porcelain outputJeff King, May 9, 2011
  39. Thiago FarinaMay 9, 2011
  40. 3/3 blame: add --line-porcelain output formatJeff King, May 9, 2011
  41. Jonathan NiederMay 6, 2011
  42. 2/2 tests: teach verify_parents to check for extra parentsJonathan Nieder, May 6, 2011
  43. Junio C HamanoMay 6, 2011
  44. Jonathan NiederMay 6, 2011
  45. Jonathan NiederMay 6, 2011
  46. Junio C HamanoMay 6, 2011

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.