[PATCH] t7006: guard cleanup with test_expect_success

From

Jonathan Nieder <jrnieder@gmail.com>

Date

Apr 15, 2010, 00:38 UTC

Message-ID

<20100415003803.GB14151@progeny.tock>

In-Reply-To

<7v4ojdpxls.fsf@alter.siamese.dyndns.org>

While at it, apply some other minor fixes:

 - do not rely on the shell to export variables defined with the same
   command as a function call

 - avoid whitespace immediately after the > redirection operator, for
   consistency with the style of other tests

Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
Junio C Hamano wrote:

> They are reasonable clean-up steps to start the real test (starting from
> setting and exporting the pager) in a known good state, and as long as you
> write them not to fail I don't see any reason to have them outside the
> test.

Good catch, thanks.  Here’s a patch for the existing test (against master).

 t/t7006-pager.sh |  149 ++++++++++++++++++++++++++++++++++++-----------------
 t/test-lib.sh    |   16 ++++++
 2 files changed, 117 insertions(+), 48 deletions(-)

diff --git a/t/t7006-pager.sh b/t/t7006-pager.sh
index d9202d5..62595ab 100755
--- a/t/t7006-pager.sh
+++ b/t/t7006-pager.sh
@@ -4,17 +4,24 @@ test_description='Test automatic use of a pager.'
 
 . ./test-lib.sh
 
-rm -f stdout_is_tty
+cleanup_fail() {
+	echo >&2 cleanup failed
+	exit 1
+}
+
 test_expect_success 'set up terminal for tests' '
+	rm -f stdout_is_tty ||
+	cleanup_fail &&
+
 	if test -t 1
 	then
-		: > stdout_is_tty
+		>stdout_is_tty
 	elif
 		test_have_prereq PERL &&
 		"$PERL_PATH" "$TEST_DIRECTORY"/t7006/test-terminal.perl \
 			sh -c "test -t 1"
 	then
-		: > test_terminal_works
+		>test_terminal_works
 	fi
 '
 
@@ -32,53 +39,68 @@ else
 	say no usable terminal, so skipping some tests
 fi
 
-unset GIT_PAGER GIT_PAGER_IN_USE
-git config --unset core.pager
-PAGER='cat > paginated.out'
-export PAGER
-
 test_expect_success 'setup' '
+	unset GIT_PAGER GIT_PAGER_IN_USE &&
+	test_might_fail git config --unset core.pager &&
+
+	PAGER="cat >paginated.out" &&
+	export PAGER &&
+
 	test_commit initial
 '
 
-rm -f paginated.out
 test_expect_success TTY 'some commands use a pager' '
+	rm -f paginated.out ||
+	cleanup_fail &&
+
 	test_terminal git log &&
 	test -e paginated.out
 '
 
-rm -f paginated.out
 test_expect_success TTY 'some commands do not use a pager' '
+	rm -f paginated.out ||
+	cleanup_fail &&
+
 	test_terminal git rev-list HEAD &&
 	! test -e paginated.out
 '
 
-rm -f paginated.out
 test_expect_success 'no pager when stdout is a pipe' '
+	rm -f paginated.out ||
+	cleanup_fail &&
+
 	git log | cat &&
 	! test -e paginated.out
 '
 
-rm -f paginated.out
 test_expect_success 'no pager when stdout is a regular file' '
-	git log > file &&
+	rm -f paginated.out ||
+	cleanup_fail &&
+
+	git log >file &&
 	! test -e paginated.out
 '
 
-rm -f paginated.out
 test_expect_success TTY 'git --paginate rev-list uses a pager' '
+	rm -f paginated.out ||
+	cleanup_fail &&
+
 	test_terminal git --paginate rev-list HEAD &&
 	test -e paginated.out
 '
 
-rm -f file paginated.out
 test_expect_success 'no pager even with --paginate when stdout is a pipe' '
+	rm -f file paginated.out ||
+	cleanup_fail &&
+
 	git --paginate log | cat &&
 	! test -e paginated.out
 '
 
-rm -f paginated.out
 test_expect_success TTY 'no pager with --no-pager' '
+	rm -f paginated.out ||
+	cleanup_fail &&
+
 	test_terminal git --no-pager log &&
 	! test -e paginated.out
 '
@@ -86,88 +108,119 @@ test_expect_success TTY 'no pager with --no-pager' '
 # A colored commit log will begin with an appropriate ANSI escape
 # for the first color; the text "commit" comes later.
 colorful() {
-	read firstline < $1
+	read firstline <$1
 	! expr "$firstline" : "^[a-zA-Z]" >/dev/null
 }
 
-rm -f colorful.log colorless.log
 test_expect_success 'tests can detect color' '
-	git log --no-color > colorless.log &&
-	git log --color > colorful.log &&
+	rm -f colorful.log colorless.log ||
+	cleanup_fail &&
+
+	git log --no-color >colorless.log &&
+	git log --color >colorful.log &&
 	! colorful colorless.log &&
 	colorful colorful.log
 '
 
-rm -f colorless.log
-git config color.ui auto
 test_expect_success 'no color when stdout is a regular file' '
-	git log > colorless.log &&
+	rm -f colorless.log &&
+	git config color.ui auto ||
+	cleanup_fail &&
+
+	git log >colorless.log &&
 	! colorful colorless.log
 '
 
-rm -f paginated.out
-git config color.ui auto
 test_expect_success TTY 'color when writing to a pager' '
-	TERM=vt100 test_terminal git log &&
+	rm -f paginated.out &&
+	git config color.ui auto ||
+	cleanup_fail &&
+
+	(
+		TERM=vt100 &&
+		export TERM &&
+		test_terminal git log
+	) &&
 	colorful paginated.out
 '
 
-rm -f colorful.log
-git config color.ui auto
 test_expect_success 'color when writing to a file intended for a pager' '
-	TERM=vt100 GIT_PAGER_IN_USE=true git log > colorful.log &&
+	rm -f colorful.log &&
+	git config color.ui auto ||
+	cleanup_fail &&
+
+	(
+		TERM=vt100 &&
+		GIT_PAGER_IN_USE=true &&
+		export TERM GIT_PAGER_IN_USE &&
+		git log >colorful.log
+	) &&
 	colorful colorful.log
 '
 
-unset PAGER GIT_PAGER
-git config --unset core.pager
 test_expect_success 'determine default pager' '
+	unset PAGER GIT_PAGER &&
+	test_might_fail git config --unset core.pager ||
+	cleanup_fail &&
+
 	less=$(git var GIT_PAGER) &&
 	test -n "$less"
 '
 
-if expr "$less" : '^[a-z]*$' > /dev/null && test_have_prereq TTY
+if expr "$less" : '^[a-z][a-z]*$' >/dev/null && test_have_prereq TTY
 then
 	test_set_prereq SIMPLEPAGER
 fi
 
-unset PAGER GIT_PAGER
-git config --unset core.pager
-rm -f default_pager_used
 test_expect_success SIMPLEPAGER 'default pager is used by default' '
-	cat > $less <<-EOF &&
-	#!$SHELL_PATH
-	wc > default_pager_used
+	unset PAGER GIT_PAGER &&
+	test_might_fail git config --unset core.pager &&
+	rm -f default_pager_used ||
+	cleanup_fail &&
+
+	cat >$less <<-\EOF &&
+	#!/bin/sh
+	wc >default_pager_used
 	EOF
 	chmod +x $less &&
-	PATH=.:$PATH test_terminal git log &&
+	(
+		PATH=.:$PATH &&
+		export PATH &&
+		test_terminal git log
+	) &&
 	test -e default_pager_used
 '
 
-unset GIT_PAGER
-git config --unset core.pager
-rm -f PAGER_used
 test_expect_success TTY 'PAGER overrides default pager' '
-	PAGER="wc > PAGER_used" &&
+	unset GIT_PAGER &&
+	test_might_fail git config --unset core.pager &&
+	rm -f PAGER_used ||
+	cleanup_fail &&
+
+	PAGER="wc >PAGER_used" &&
 	export PAGER &&
 	test_terminal git log &&
 	test -e PAGER_used
 '
 
-unset GIT_PAGER
-rm -f core.pager_used
 test_expect_success TTY 'core.pager overrides PAGER' '
+	unset GIT_PAGER &&
+	rm -f core.pager_used ||
+	cleanup_fail &&
+
 	PAGER=wc &&
 	export PAGER &&
-	git config core.pager "wc > core.pager_used" &&
+	git config core.pager "wc >core.pager_used" &&
 	test_terminal git log &&
 	test -e core.pager_used
 '
 
-rm -f GIT_PAGER_used
 test_expect_success TTY 'GIT_PAGER overrides core.pager' '
+	rm -f GIT_PAGER_used ||
+	cleanup_fail &&
+
 	git config core.pager wc &&
-	GIT_PAGER="wc > GIT_PAGER_used" &&
+	GIT_PAGER="wc >GIT_PAGER_used" &&
 	export GIT_PAGER &&
 	test_terminal git log &&
 	test -e GIT_PAGER_used
diff --git a/t/test-lib.sh b/t/test-lib.sh
index c582964..f807625 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -516,6 +516,22 @@ test_must_fail () {
 	test $? -gt 0 -a $? -le 129 -o $? -gt 192
 }
 
+# Similar to test_must_fail, but tolerates success, too.  This is
+# meant to be used in contexts like:
+#
+#	test_expect_success 'some command works without configuration' '
+#		test_might_fail git config --unset all.configuration &&
+#		do something
+#	'
+#
+# Writing "git config --unset all.configuration || :" would be wrong,
+# because we want to notice if it fails due to segv.
+
+test_might_fail () {
+	"$@"
+	test $? -ge 0 -a $? -le 129 -o $? -gt 192
+}
+
 # test_cmp is a helper function to compare actual and expected output.
 # You can use it like:
 #

-- 
1.7.1.rc1.18.g08771.dirty