Re: [PATCH] blame: Add tests for -L/start/,/end/

From: Junio C Hamano <gitster@pobox.com>
Date: Jul 19, 2010, 21:09 UTC
Message-ID: <7voce3nppy.fsf@alter.siamese.dyndns.org>
In-Reply-To: <1279294507-9238-1-git-send-email-avarab@gmail.com>

Ævar Arnfjörð Bjarmason  <avarab@gmail.com> writes:

> git-pickaxe (later git-blame) gained support for the -L/start/,/end/
> form in 2006 (931233bc66 by Junio C Hamano), but nothing was added to
> test this functionality. Change that by adding more -L tests to
> t8003-blame.sh.

If we look at the existing tests carefully, there is no tests for the range notation -L<start>,<end> in general, not just the regexp variant. A few existing uses are only to limit the output for testing other features and take it granted that -L works correctly (meaning they may detect it if you break -L implementation but that is merely a side effect).

> +for comma in '' ','
> +do
> +	# The comma in -L/regex/, is optional

Is it just -L/regex/? I thought -L<start> regardless of the shape of <start> was equivalent to -L<start>,END-OF-FILE.

> +test_expect_success 'blame -LINT,/end/' '
> +	git blame -L3,/GH/ cow  >current 2>errors &&

This is somewhat an interesting one. It asks for a range that begins at the third line in the file, extending to a line after that line that matches the given expression. Unfortunately the test vector does not have a line that contains GH before the third line, nor more than one lines that contain GH after the third line (we should stop at the first hit), so this test is not as effective as it could be.

> +test_expect_success 'blame -L,/end/' '
> +	git blame -L",/^X/" cow >current 2>errors &&

Missing <start> defaults to the beginning of file; the same as -L,INT and not specific to the regex variant.

Show 5 quoted lines

> +test_expect_success 'blame -L/invalid regex/' '
> +	# At least GNU, Solaris and FreeBSD (and by extension, Mac OS X)
> +	# complain about this
> +	! git blame -L/[b-a]/ cow >current 2>errors &&
> +	grep b-a errors &&

The b-a in the error message comes from us, not the regex library, so this test should be portable, I think, as long as the library detects the empty range correctly.