[PATCH 1/2] t/test-lib.sh: support Korn shell by converting GIT_EXIT_OK to GIT_EXIT_CODE

From: Brandon Casey <redacted>

Commit 6e7b5aaf introduced the concept of GIT_EXIT_OK as a way to indicate
to die(), the exit handler, whether the exit was initiated by the test
harness, or whether it was unexpected.  die() expects $? to contain the
value passed to exit(), and when GIT_EXIT_OK is set, die() calls exit with
the value in $?.  This works as expected when using the Bash shell.  For
the Korn shell, $? has the value of the last executed statement _before_
the call to exit.  If that statement completed successfully, then die()
would incorrectly exit with a successful status when GIT_EXIT_OK is set.

So, rather than relying on the behavior of Bash in order to get the exit
code from $? inside die(), change GIT_EXIT_OK into GIT_EXIT_CODE, and set
it to the code that we want to exit with.  This allows the test suite to
be run with the Korn shell.

Signed-off-by: Brandon Casey <redacted>
---
 t/test-lib.sh |   15 ++++++++-------
 1 files changed, 8 insertions(+), 7 deletions(-)

diff --git a/t/test-lib.sh b/t/test-lib.sh
index f2ca536..64e793a 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh

@@ -150,7 +150,7 @@ fi
 
 error () {
 	say_color error "error: $*"
-	GIT_EXIT_OK=t
+	GIT_EXIT_CODE=1
 	exit 1
 }
 

@@ -183,16 +183,16 @@ test_success=0
 
 die () {
 	code=$?
-	if test -n "$GIT_EXIT_OK"
+	if test -n "$GIT_EXIT_CODE"
 	then
-		exit $code
+		exit $GIT_EXIT_CODE
 	else
 		echo >&5 "FATAL: Unexpected exit with code $code"
 		exit 1
 	fi
 }
 
-GIT_EXIT_OK=
+GIT_EXIT_CODE=
 trap 'die' EXIT
 
 # The semantics of the editor variables are that of invoking

@@ -295,7 +295,7 @@ test_failure_ () {
 	say_color error "FAIL $test_count: $1"
 	shift
 	echo "$@" | sed -e 's/^/	/'
-	test "$immediate" = "" || { GIT_EXIT_OK=t; exit 1; }
+	test "$immediate" = "" || { GIT_EXIT_CODE=1; exit 1; }
 }
 
 test_known_broken_ok_ () {

@@ -508,7 +508,6 @@ test_create_repo () {
 }
 
 test_done () {
-	GIT_EXIT_OK=t
 	test_results_dir="$TEST_DIRECTORY/test-results"
 	mkdir -p "$test_results_dir"
 	test_results_path="$test_results_dir/${0%.sh}-$$"

@@ -539,9 +538,11 @@ test_done () {
 		cd "$(dirname "$remove_trash")" &&
 		rm -rf "$(basename "$remove_trash")"
 
+		GIT_EXIT_CODE=0
 		exit 0 ;;
 
 	*)
+		GIT_EXIT_CODE=1
 		say_color error "failed $test_failure among $msg"
 		exit 1 ;;
 

@@ -655,7 +656,7 @@ case "$test" in
 esac
 test ! -z "$debug" || remove_trash=$TRASH_DIRECTORY
 rm -fr "$test" || {
-	GIT_EXIT_OK=t
+	GIT_EXIT_CODE=1
 	echo >&5 "FATAL: Cannot prepare test area"
 	exit 1
 }

-- 
1.6.5.rc3

From: Brandon Casey <redacted>

Now that the test suite supports the Korn shell, we can use it as the
default on platforms that do not ship with Bash.

Signed-off-by: Brandon Casey <redacted>
---
 Makefile |    6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/Makefile b/Makefile
index 629ab35..9ada196 100644
--- a/Makefile
+++ b/Makefile

@@ -726,7 +726,7 @@ endif
 ifeq ($(uname_S),SunOS)
 	NEEDS_SOCKET = YesPlease
 	NEEDS_NSL = YesPlease
-	SHELL_PATH = /bin/bash
+	SHELL_PATH = /usr/xpg4/bin/sh
 	SANE_TOOL_PATH = /usr/xpg6/bin:/usr/xpg4/bin
 	NO_STRCASESTR = YesPlease
 	NO_MEMMEM = YesPlease

@@ -850,7 +850,7 @@ ifeq ($(uname_S),IRIX)
 	NO_MMAP = YesPlease
 	NO_EXTERNAL_GREP = UnfortunatelyYes
 	SNPRINTF_RETURNS_BOGUS = YesPlease
-	SHELL_PATH = /usr/gnu/bin/bash
+	SHELL_PATH = /bin/ksh
 	NEEDS_LIBGEN = YesPlease
 	THREADED_DELTA_SEARCH = YesPlease
 endif

@@ -870,7 +870,7 @@ ifeq ($(uname_S),IRIX64)
 	NO_MMAP = YesPlease
 	NO_EXTERNAL_GREP = UnfortunatelyYes
 	SNPRINTF_RETURNS_BOGUS = YesPlease
-	SHELL_PATH=/usr/gnu/bin/bash
+	SHELL_PATH=/bin/ksh
 	NEEDS_LIBGEN = YesPlease
 	THREADED_DELTA_SEARCH = YesPlease
 endif

-- 
1.6.5.rc3

Brandon Casey wrote:

So, rather than relying on the behavior of Bash in order to get the exit
code from $? inside die(), change GIT_EXIT_OK into GIT_EXIT_CODE, and set
it to the code that we want to exit with.  This allows the test suite to
be run with the Korn shell.

Signed-off-by: Brandon Casey <redacted>

Sounds like a good idea.  A few thoughts:

start_httpd() from lib-httpd.sh uses a similar pattern:

lib-httpd.sh:96:        trap 'code=$?; stop_httpd; (exit $code); die' EXIT

It is probably worth changing that, too, unless GIT_TEST_HTTPD would not
work on these platforms for some other reason.

"GIT_EXIT_CODE=1; exit 1" sounds repetitive to my ear.  It’s probably just
me, but if not, it might be worth adding a function like

	expected_exit() {
		GIT_EXIT_CODE=$1
		exit "$1"
	}

I’m not sure.

[...]

$code can be removed now, right?

Jonathan

Sorry to reply to self, just want to correct something I missed.

Jonathan Nieder wrote:

Brandon Casey wrote:

quoted

diff --git a/t/test-lib.sh b/t/test-lib.sh
index f2ca536..64e793a 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh

[...]

quoted

@@ -183,16 +183,16 @@ test_success=0
 
 die () {
 	code=$?
-	if test -n "$GIT_EXIT_OK"
+	if test -n "$GIT_EXIT_CODE"
 	then
-		exit $code
+		exit $GIT_EXIT_CODE
 	else
 		echo >&5 "FATAL: Unexpected exit with code $code"
 		exit 1
 	fi
 }

$code can be removed now, right?

Sloppy reading on my part here: $code is still used in the error
message on unexpected exits.  The $code will be inaccurate on Suns
in some cases, but this is only a cosmetic problem and the wrong
value should be better than nothing for debugging.

lib-httpd.sh:96:        trap 'code=$?; stop_httpd; (exit $code); die' EXIT

It is probably worth changing that, too, unless GIT_TEST_HTTPD would not
work on these platforms for some other reason.

This is used to support that error message, so it should not be
changed, either.

Your patch looks good.  Maybe the commit message could explain this,
though?

Apologies for the noise,
Jonathan

Hi,

On Fri, Oct 09, 2009 at 01:39:56PM -0500, Brandon Casey wrote:

For the Korn shell, $? has the value of the last executed statement
_before_ the call to exit.

I just installed ksh/stable (version 93s+ 2008-01-31) on Debian and it
behaves correctly. Maybe you need to upgrade? This really looks like a bug
in your shell to me.

 die () {
 	code=$?
-	if test -n "$GIT_EXIT_OK"
+	if test -n "$GIT_EXIT_CODE"
 	then
-		exit $code
+		exit $GIT_EXIT_CODE
 	else
 		echo >&5 "FATAL: Unexpected exit with code $code"
 		exit 1
 	fi
 }

So in your shell an unexpected exit will always output this?

 FATAL: Unexpected exit with code <some command>

If we can't rely on the value of $?, we should not use it.

Clemens