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

[Junio C Hamano <gitster@pobox.com>]

Junio C Hamano <gitster@pobox.com> writes:

> SZEDER Gábor <szeder.dev@gmail.com> writes:
>
>> 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"