Re: [PATCH v2] test_cmp: diagnose incorrect arguments

Junio C Hamano [off-list ref] writes:

Hmph, I agree that the "both must be file" is a bit too eager and
ignores that "they must match, but the possible reasons they may not
include one of them may be missing" use case.

quoted

Ævar ran into the same issue recently[1] and came up with the same
workaround. Despite its good intention (trying to catch bugs in
'test_expect_failure' tests), this change[2] doesn't seem to have
caught any genuine bugs (it wouldn't even have caught the bug which
served as its inspiration[3]), but has nevertheless caused a couple
hiccups already. As such, I would not be opposed to seeing the change
reverted.

Sounds good.  Anybody wants to do the honors?

For now I've queued this directly on top of the offending change and
merged the result in 'seen', perhaps to be advanced to 'next' and
'master' after the 2.29 final unless a better version comes.

Thanks.

-- >8 --
commit b0b2d8b4e00fd34c0031b6dbcd67b4bcf22d864c
Author: Junio C Hamano [off-list ref]
Date:   Fri Oct 16 13:51:04 2020 -0700

    Revert "test_cmp: diagnose incorrect arguments"
    
    This reverts commit d572f52a64c6a69990f72ad6a09504b9b615d2e4; the
    idea to detect that "test_cmp expect actual" was fed a misspelt
    filename meant well, but when the version of Git tested exhibits a
    bug, the reason why these two files do not match may be because one
    of them did not get created as expected, in which case missing file
    is not a sign of misspelt filename but is a genuine test failure.
---
 t/test-lib-functions.sh | 16 ++--------------
 1 file changed, 2 insertions(+), 14 deletions(-)

diff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh
index 21225330c2..3103be8a32 100644
--- a/t/test-lib-functions.sh
+++ b/t/test-lib-functions.sh

@@ -905,13 +905,7 @@ test_expect_code () {
 # - not all diff versions understand "-u"
 
 test_cmp() {
-	test $# -eq 2 || BUG "test_cmp requires two arguments"
-	if ! eval "$GIT_TEST_CMP" '"$@"'
-	then
-		test "x$1" = x- || test -e "$1" || BUG "test_cmp '$1' missing"
-		test "x$2" = x- || test -e "$2" || BUG "test_cmp '$2' missing"
-		return 1
-	fi
+	eval "$GIT_TEST_CMP" '"$@"'
 }
 
 # Check that the given config key has the expected value.

@@ -940,13 +934,7 @@ test_cmp_config() {
 # test_cmp_bin - helper to compare binary files
 
 test_cmp_bin() {
-	test $# -eq 2 || BUG "test_cmp_bin requires two arguments"
-	if ! cmp "$@"
-	then
-		test "x$1" = x- || test -e "$1" || BUG "test_cmp_bin '$1' missing"
-		test "x$2" = x- || test -e "$2" || BUG "test_cmp_bin '$2' missing"
-		return 1
-	fi
+	cmp "$@"
 }
 
 # Use this instead of test_cmp to compare files that contain expected and