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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




Hi,

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

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

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]