Re: [PATCH 04/11] t3030-merge-recursive: don't check the stderr of a subshell

Junio C Hamano [off-list ref] writes:

SZEDER Gábor [off-list ref] writes:

quoted

The two test checking 'git mmerge-recursive' in an empty worktree in
...
 		GIT_INDEX_FILE="$PWD/ours-has-rename-index" &&
 		export GIT_INDEX_FILE &&
 		mkdir "$GIT_WORK_TREE" &&
-		git read-tree -i -m $c7 &&
-		git update-index --ignore-missing --refresh &&
-		git merge-recursive $c0 -- $c7 $c3 &&
-		git ls-files -s >actual-files
-	) 2>actual-err &&
-	>expected-err &&
+		git read-tree -i -m $c7 2>actual-err &&
+		test_must_be_empty expected-err &&
+		git update-index --ignore-missing --refresh 2>actual-err &&
+		test_must_be_empty expected-err &&
+		git merge-recursive $c0 -- $c7 $c3 2>actual-err &&
+		test_must_be_empty expected-err &&
+		git ls-files -s >actual-files 2>actual-err &&
+		test_must_be_empty expected-err

Where do the contents of all of these expected-err files come from?
Should all of the test_must_be_empty checks be checking actual-err
instead?

And the reason why your pre-submission testing did not catch may be
because test_must_be_empty is broken?  I wonder if this is a good
way forward to catch a possible bug like this.

Of course, if somebody was using the helepr for "must be either
missing or empty", this change will break it, but I somehow doubt
it.  A program that creates/opens and writes an error message only
when an error is detected is certainly possible, and could be tested
with the current test_must_be_empty this way:

	rm -f actual-err &&
	git frotz --error-to=actual-err &&
	test_must_be_empty actual-err

but then the last step in such a test like the above is more natural
to check if actual-err _exists_ in the first place anyway, so...

 t/test-lib-functions.sh | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh
index 37eb34044a..6cfbee60e4 100644
--- a/t/test-lib-functions.sh
+++ b/t/test-lib-functions.sh

@@ -772,7 +772,11 @@ verbose () {
 # otherwise.
 
 test_must_be_empty () {
-	if test -s "$1"
+	if ! test -f "$1"
+	then
+		echo "'$1' is missing"
+		return 1
+	elif test -s "$1"
 	then
 		echo "'$1' is not empty, it contains:"
 		cat "$1"