Ævar Arnfjörð Bjarmason [off-list ref] 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.
+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.
On Mon, Jul 19, 2010 at 21:09, Junio C Hamano [off-list ref] wrote:
Ævar Arnfjörð Bjarmason [off-list ref] writes:
quoted
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).
I didn't submit tests for -L in general because I'm not happy with how
it behaves (as noted in "[BUG?] blame: Odd -L 1,+0 behavior"). So this
patch only tests the regex portion of parse_loc().
I might add tests for -L in general it later, but that patch will
probably include a behavior change.
quoted
+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.
No, it's not just -L/regex/, but I try to in general to write tests
for thing that *might* break, not just things that are guaranteed not
to break as things are now implemented. If blame.c is rewritten in the
future "-L123", might continue to work but "-L/start/," might be
broken as a result of some code shuffling.
quoted
+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.
Yeah, I didn't test for ambiguities like that. I was going to expand
on that later, but see above about not having patched that yet.
quoted
+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.
Yup, but I think it should be tested for anyway.
quoted
+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.
Yeah, it's just a test for:
1. Invalid regexes being detected at all, i.e. git not ignoring
the return value of regcomp().
2. That we have an error message at all for it, and that the error
message includes the invalid regex that the user supplied.