Re: [PATCH v2] test_cmp: diagnose incorrect arguments

[Eric Sunshine <sunshine@sunshineco.com>]

On Fri, Oct 16, 2020 at 7:14 PM Junio C Hamano <gitster@pobox.com> wrote:
> Junio C Hamano <gitster@pobox.com> writes:
> >  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)