Re: [PATCH v2] test_cmp: diagnose incorrect arguments

On Fri, Oct 16, 2020 at 7:14 PM Junio C Hamano [off-list ref] wrote:

Junio C Hamano [off-list ref] writes:

quoted

 test_cmp() {
-     test $# -eq 2 || BUG "test_cmp requires two arguments"
-     if ! eval "$GIT_TEST_CMP" '"$@"'

Looking at this again, I think we could keep the "we should have two
arguments, no more than, no less than, but exactly two".  But I think
those who write new tests are working to eventually make them pass,
so hopefully they'll notice and investigate test_cmp that yields false
anyway, I guess.

The purpose of the change in question[1] was specifically to help
catch bugs in the test_expect_failure() case, which is quite rare in
the Git test suite (and hopefully becomes rarer over time). So, I
think it's fine to revert the change fully, including the 2-argument
check since, as you say, test writers are normally trying to make
their tests pass, and the 2-argument check doesn't provide much, if
any, value for the test_expect_success() case.

[1]: d572f52a64 (test_cmp: diagnose incorrect arguments, 2020-08-09)