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

From: Michael Haggerty <[email-masked]>
Date: Tue, 07 Oct 2014 15:21:15 +0200

On 10/03/2014 10:27 PM, Jeff King wrote:
> For small outputs, we sometimes use:
> 
>   test "$(some_cmd)" = "something we expect"
> 
> instead of a full test_cmp. The downside of this is that
> when it fails, there is no output at all from the script.
> Let's introduce a small helper to make tests easier to
> debug.
> 
> Signed-off-by: Jeff King <[email-masked]>
> ---
> This is in the same boat as the last commit; we can drop it without
> hurting the rest of the series.
> 
> Is test_eq too cutesy or obfuscated? I have often wanted it when
> debugging other tests, too. Our usual technique is to do:
> 
>   echo whatever >expect &&
>   do_something >actual &&
>   test_cmp expect actual
> 
> That's a bit verbose. We could hide it behind something like test_eq,
> too, but it introduces several extra new processes. And I know people on
> some fork-challenged platforms are very sensitive to the number of
> spawned processes in the test suite.

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.

If you want to write a helper that allows arbitrary comparator
operators, then I think it would be more readable to put the comparison
operator in the middle, like

    test_test foo = bar

And in fact once you've done that, couldn't we just make this a generic
wrapper for any `test` command?

test_test () {
	if ! test "$@"
	then
		echo >&2 "test failed: $*"
		false
	fi
}

Feel free to bikeshed the function name.

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).

Finally, if we want a function intended mainly for checking program
output (like the test_eql function suggested by Junio), it might be
simpler to use if it accepts the function output on its stdin:

	test_output () {
		echo "$1" >expect &&
                cat >actual &&
                test_cmp expect actual
	}
        ...
	do_something | test_output whatever

This would make it easier to generate the input using an arbitrary shell
pipeline.

> [...]

Michael

-- 
Michael Haggerty
[email-masked]