Re: [PATCH] Portability: returning void

Jeff King wrote:

While we're at it, let's make the "kill" process a little
more robust. Specifically:

  1. Wrap the "kill" in a test_when_finished, since we want
     to clean up the process whether the test succeeds or not.

  2. The "kill" is part of our && chain for test success. It
     probably won't fail, but it can if the process has
     expired before we manage to kill it. So let's mark it
     as OK to fail.

Signed-off-by: Jeff King <redacted>
---
Actually, my (2) above is unlikely to trigger, since the test would have
to hang for 100 seconds first, which probably means it is failing
anyway. But I did run across it when I screwed up my fix.

That is actually how the tests distinguish between success and
failure.  Any idea about a better way?

I was in the process of writing a commit message for the same "exec"
trick, but I'm glad you got to it first. ;-)

Also, is it just me, or does it seem weird that test_when_finished
blocks failing can produce test failure? I would think you would be able
to put any old cleanup crap into them and not care whether it worked or
not.

I'm generally happy that it catches mistakes in cleanup code, but I
could easily be convinced to change it anyway.

[...]

The $! gets expanded when this is executed, so later background
processes won't interfere.  Good.

Maybe it would be good to factor out a helper to make future
improvements a little easier, like so:

-- 8< --
Subject: t0081: introduce helper to fill a pipe in the background

The fill_input function generates a fifo and runs a command to write
to it and wait a while.  The intended use is to test programs that
need to be able to cope with input of limited length without relying
on EOF or SIGPIPE to detect its end.

For example:

	fill_input "echo hi" &&
	head -1 input

will succeed, while

	fill_input "echo hi" &&
	head -2 input

will hang waiting for more input.

It works by running the indicated commands followed by
"exec sleep 100" in a background process and registering a clean-up
command to kill it and check if it was killed by SIGPIPE (good) or
ran to completion (bad).  This patch adds a definition for this
function and updates existing tests that used that trick to use it.

Signed-off-by: Jonathan Nieder <redacted>
---
The commit message assumes test_when_finished "kill $!", while the
patch uses "kill $! || :"; one of the two would need to be fixed
before applying this.

 t/t0081-line-buffer.sh |   51 +++++++++++++++++++----------------------------
 1 files changed, 21 insertions(+), 30 deletions(-)

diff --git a/t/t0081-line-buffer.sh b/t/t0081-line-buffer.sh
index 416ccc1..218da98 100755
--- a/t/t0081-line-buffer.sh
+++ b/t/t0081-line-buffer.sh

@@ -14,6 +14,21 @@ correctly.
 
 test -n "$GIT_REMOTE_SVN_TEST_BIG_FILES" && test_set_prereq EXPENSIVE
 
+fill_input () {
+	if ! test_declared_prereq PIPE
+	then
+		echo >&4 "fill_input: need to declare PIPE prerequisite"
+		return 127
+	fi &&
+	rm -f input &&
+	mkfifo input &&
+	(
+		eval "$*" &&
+		exec sleep 100
+	) >input &
+	test_when_finished "kill $! || true"
+}
+
 generate_tens_of_lines () {
 	tens=$1 &&
 	line=$2 &&

@@ -35,24 +50,11 @@ long_read_test () {
 	line=abcdefghi &&
 	echo "$line" >expect &&
 
-	if ! test_declared_prereq PIPE
-	then
-		echo >&4 "long_read_test: need to declare PIPE prerequisite"
-		return 127
-	fi &&
 	tens_of_lines=$(($1 / 100 + 1)) &&
 	lines=$(($tens_of_lines * 10)) &&
 	readsize=$((($lines - 1) * 10 + 3)) &&
 	copysize=7 &&
-	rm -f input &&
-	mkfifo input &&
-	{
-		(
-			generate_tens_of_lines $tens_of_lines "$line" &&
-			exec sleep 100
-		) >input &
-	} &&
-	test_when_finished "kill $! || true" &&
+	fill_input "generate_tens_of_lines $tens_of_lines $line" &&
 	test-line-buffer input <<-EOF >output &&
 	binary $readsize
 	copy $copysize

@@ -81,12 +83,7 @@ test_expect_success 'hello world' '
 
 test_expect_success PIPE '0-length read, no input available' '
 	printf ">" >expect &&
-	rm -f input &&
-	mkfifo input &&
-	{
-		sleep 100 >input &
-	} &&
-	test_when_finished "kill $! || true" &&
+	fill_input : &&
 	test-line-buffer input <<-\EOF >actual &&
 	binary 0
 	copy 0

@@ -106,16 +103,10 @@ test_expect_success '0-length read, send along greeting' '
 
 test_expect_success PIPE '1-byte read, no input available' '
 	printf ">%s" ab >expect &&
-	rm -f input &&
-	mkfifo input &&
-	{
-		(
-			printf "%s" a &&
-			printf "%s" b &&
-			exec sleep 100
-		) >input &
-	} &&
-	test_when_finished "kill $! || true" &&
+	fill_input "
+		printf a &&
+		printf b
+	" &&
 	test-line-buffer input <<-\EOF >actual &&
 	binary 1
 	copy 1

-- 
1.7.4.2