Re: [PATCH v2] diff --shortstat --dirstat: remove duplicate output

From: SZEDER Gábor <szeder@ira.uka.de>
Date: Mar 2, 2015, 01:00 UTC
Message-ID: <20150302020009.Horde.9sATpKnsrWQkGadaRTvxkA3@webmail.informatik.kit.edu>
In-Reply-To: <20150301155818.GA5307@laptop>

Hi,

Quoting Mårten Kongstad <marten.kongstad@gmail.com>:

Show 19 quoted lines

> On Sun, Mar 01, 2015 at 11:25:53AM +0100, Torsten Bögershausen wrote:
>> On 2015-03-01 08.39, Mårten Kongstad wrote:
>> []
>>> +test_expect_success '--shortstat --dirstat should output only
>> one dirstat' '
>>> +	git diff --shortstat --dirstat=changes HEAD^..HEAD
>>> actual_diff_shortstat_dirstat_changes &&
>>> +	test $(grep -c " dst/copy/changed/$"
>> actual_diff_shortstat_dirstat_changes) = 1 &&
>> How portable is the "grep -c" usage ?
>> (I don't now it either, do we have other opinions ?), but the
>> following seems to be more "Git-style":
>>
>> test_expect_success '--shortstat --dirstat should output only one dirstat' '
>> 	git diff --shortstat --dirstat=changes HEAD^..HEAD
>>> actual_diff_shortstat_dirstat_changes &&
>> 	grep " dst/copy/changed/$" actual_diff_shortstat_dirstat_changes >actual &&
>> 	test_line_count = 1 actual
>>

> Granted I didn't miss anything while trawling the tests for the above
> numbers, it feels like the 'grep -c' option is more in line with the
> existing tests. That said, I don't know if there is an ongoing trend to
> deprecate 'grep -c' in favour of 'test_line_count'.

It's not just 'grep -c' but the 'test' checking its output as well.

If something goes wrong and the line count doesn't match expectations 'test' fails silently leaving the developer clueless as to what went wrong.

'test_line_count', on the other hand, produces useful output in case of a failure:

    $ printf 'foo\nbar\n' >actual
    $ test_line_count = 1 actual
    test_line_count: line count for actual != 1
    foo
    bar

Since the name of the file in question is included in the output and since there are three separate checks in this test, I would also suggest writing 'grep's output into separate files 'actual_{changes,lines,files}'.

Gábor