Re: [PATCH 08/16] t5304: use helper to report failure of "test foo = bar"

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

Michael Haggerty <mhagger@alum.mit.edu> writes:

> I don't like the three-argument version of test_eq. Wouldn't using a
> comparison operator other than "=" would be very confusing, given that
> "eq" is in the name of the function? It also doesn't look like you use
> this feature.

> An alternative direction to go would be to specialize the function for
> equality testing and delegate to test_cmp to get better output for
> failures, but optimized to avoid excess process creation in the happy path:
>
> test_eq () {
> 	if test "$1" != "$2"
> 	then
> 		printf "%s" "$1" >expect &&
> 		printf "%s" "$2" >actual &&
> 		test_cmp expect actual
> 	fi
> }
>
> (but using properly-created temporary file names).

I agree that it would be a good idea to use a randomly generated
temporary filename that does not collide, as long as we make sure
not to leave cruft in the working tree of the test and we leave the
file there when the test script is run under "-i" or "-d" option.

The above superficially looks nice; "! test_eq a b" would give
useless output under "-v", and "test_ne a b" needs to be added if
you go that route, though.

Anyway, with the version posted, you cannot do "! test_eq a b",
either but with "test_eq a b !=", you do not have to.

	Side note. Yes, now I looked at it again, I agree that the
	three-arg form is awkwards in at least two ways.  The
	operator, if we are to take one, should be infix, and the
	name "eq" no longer matches what it does when it is given an
	operator.

The function is similar to test_cmp which takes two files but takes
two strings, so "test_cmp_str" or something perhaps (we already have
test_cmp_rev to compare two revisions, and the suggested name
follows that pattern)?