On 8/10/22 19:10, Eric Sunshine wrote:

quoted

All of this involves two files and that is how it is done almost everywhere
except in some places where it looks like an 'older way' (test_i18ngrep) of
doing it.  Is there any reason to do it this way and not using variables,
process substitution,..?

An invocation such as:

    test $(git foo) = $(git bar) &&

throws away the exit-code from the two commands, which means we'd miss
if one or the other (or both) crashed, especially if the crash was
after the command produced the correct output. These days we try to
avoid losing the exit command of Git commands. It's possible to avoid
losing the exit-code by using variables:

    expect=$(git foo) &&
    actual=$(git bar) &&
    test "$expect" = "$actual" &&

but, if the expected and actual output don't match, you don't learn
much (other than that they failed). You could address that by showing
a message saying what failed:

    expect=... &&
    actual=... &&
    if test "$expect" != "$actual"
    then
        echo "expect not match actual"
        # maybe emit $expect and $actual too
    fi

However, `test_cmp` gives you that behavior for free, and it emits a
helpful "diff" upon failure, so these days we usually go with
`test_cmp`.

This is already out of the subject, the patch, of this thread, so sorry,
but the context is worth of it.

I understand the possibility of losing exit codes or segfaults in
subcommands or pipes, but I'm more thinking in the element to compare to.
Having something like:
 
	test_cmp_str () {
		test -f "$1" || BUG "first argument must be a file"
		if test "$#" -gt 1
		then
			local F=$1
			shift
			printf "$@" | eval "$GIT_TEST_CMP" '"$F"' -
		else
			eval "$GIT_TEST_CMP" '"$1"' -
		fi
	}

to allow writing simpler tests like:

--- a/t/t3204-branch-name-interpretation.sh
+++ b/t/t3204-branch-name-interpretation.sh

@@ -142,8 +142,7 @@ test_expect_success 'edit-description via @{-1}' '
 	EDITOR=./editor git branch --edit-description @{-1} &&
 	test_must_fail git config branch.non-desc-branch.description &&
 	git config branch.desc-branch.description >actual &&
-	printf "Branch description\n\n" >expect &&
-	test_cmp expect actual
+	test_cmp_str actual "Branch description\n\n"
 '

--- a/t/t3204-branch-name-interpretation.sh
+++ b/t/t3204-branch-name-interpretation.sh

@@ -142,8 +142,10 @@ test_expect_success 'edit-description via @{-1}' '
        EDITOR=./editor git branch --edit-description @{-1} &&
        test_must_fail git config branch.non-desc-branch.description &&
        git config branch.desc-branch.description >actual &&
-       printf "Branch description\n\n" >expect &&
-       test_cmp expect actual
+       test_cmp_str actual <<-\EOF
+       Branch description
+
+       EOF
 '

My doubts are that maybe this can induce to some bad usages, is
unusable in some systems, it has already been explored and discarded,
using files gives the diff with nice names not "-",...

Maybe this needs a new RFC thread. I dunno.

Thanks for your comments and explanations.