Re: [PATCH v3 1/9] t5520: fixup file contents comparisons

From: Junio C Hamano <gitster@pobox.com>
Date: May 16, 2015, 23:32 UTC
Message-ID: <xmqqoalkdrop.fsf@gitster.dls.corp.google.com>
In-Reply-To: <xmqq617sfj05.fsf@gitster.dls.corp.google.com>

Junio C Hamano <gitster@pobox.com> writes:

Show 20 quoted lines

> Paul Tan <pyokagan@gmail.com> writes:
>
>> So the first example would be:
>>
>>     test_output "git show HEAD:file2" new
>
> Simple things like that look fine, but when a variable is involved,
> use of eval combined with the fact that the test body is inside sq,
> makes the callers unnecessarily ugly.
>
> 	test_expect_success 'some title' '
> 		var=$(...) &&
> 		test_output "git show \$var:file2 | sed -e \"s/$old/$new/\"" new
> 	'
>
> Which is the concern this shares with the other one I sent about
> counting the number of lines in the output from a command that made
> me hesitate to suggest it.
>
> So I dunno.

I actually think that "test" that compares output from command and a constant string, and "test" that compares outputs from two commands are lazyily written forms of these:

        echo constant string >expect &&
	command >actual &&
        test_cmp expect actual

	command1 >expect &&
        command2 >actual &&
        test_cmp expect actual

The examples you gave in the earlier message were

Show 7 quoted lines

>
>      test new = "$(git show HEAD:file2)"
>
> or these:
>
>      test $(git rev-parse HEAD^2) = $(git rev-parse keep-merge)
>

and I suspect they match my observation.

My earlier test_output_count was probably in the same "lazy" category. "test $(command | wc -l) = 20" is better written as

	command >output &&
        test_line_count = 20 output

instead of using the hypothetical

	test_output_count = 20 "command"

that evals the command argument, not only because the quoting of 'command part will become complex for real world uses, but because the output itself would be the first thing we would want to inspect once the command fails. For that reason, I'd rather not to add the test_output_count I suggested earlier, so that we would encourage the more straight-forward form, i.e.

	command >output &&
        test_line_count = 20 output

to be used.