{"thread":{"id":"24413","subject":"[PATCH] blame: Add tests for -L/start/,/end/","startedAt":"2010-07-16T15:35:07Z","lastAt":"2010-07-19T21:21:49Z","messageCount":3,"participants":["Ævar Arnfjörð Bjarmason","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"145695","messageId":"1279294507-9238-1-git-send-email-avarab@gmail.com","threadId":"24413","inReplyTo":null,"subject":"[PATCH] blame: Add tests for -L/start/,/end/","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2010-07-16T15:35:07Z","receivedAt":"2010-07-16T15:35:07Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"git-pickaxe (later git-blame) gained support for the -L/start/,/end/\nform in 2006 (931233bc66 by Junio C Hamano), but nothing was added to\ntest this functionality. Change that by adding more -L tests to\nt8003-blame.sh.\n\nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n t/t8003-blame.sh | 53 +++++++++++++++++++++++++++++++++++++++++++++++++++++\n 1 files changed, 53 insertions(+), 0 deletions(-)\n\ndiff --git a/t/t8003-blame.sh b/t/t8003-blame.sh\nindex 230143c..2a3e169 100755\n--- a/t/t8003-blame.sh\n+++ b/t/t8003-blame.sh\n@@ -175,6 +175,59 @@ test_expect_success 'blame -L with invalid end' '\n \tgrep \"has only 2 lines\" errors\n '\n \n+for comma in '' ','\n+do\n+\t# The comma in -L/regex/, is optional\n+\ttest_expect_success \"blame -L/start/$comma\" \"\n+\t\tgit blame -L'/[E]F/$comma' cow >current 2>errors &&\n+\t\t! test -s errors &&\n+\t\thead -n 1 current | grep DEF &&\n+\t\ttail -n 1 current | grep GHIJK\n+\t\"\n+done\n+\n+test_expect_success 'blame -L/start/,/end/' '\n+\tgit blame -L\"/[E]F/,/^X/\" cow >current 2>errors &&\n+\t! test -s errors &&\n+\thead -n 1 current | grep DEF &&\n+\ttail -n 1 current | grep XXXX\n+'\n+\n+test_expect_success 'blame -L/start/,INT' '\n+\tgit blame -L\"/[C]/,2\" cow >current 2>errors &&\n+\t! test -s errors &&\n+\thead -n 1 current | grep ABC &&\n+\ttail -n 1 current | grep DEF\n+'\n+\n+test_expect_success 'blame -LINT,/end/' '\n+\tgit blame -L3,/GH/ cow >current 2>errors &&\n+\t! test -s errors &&\n+\thead -n 1 current | grep XXXX &&\n+\ttail -n 1 current | grep GHIJK\n+'\n+\n+test_expect_success 'blame -L,/end/' '\n+\tgit blame -L\",/^X/\" cow >current 2>errors &&\n+\t! test -s errors &&\n+\thead -n 1 current | grep ABC &&\n+\ttail -n 1 current | grep XXXX\n+'\n+\n+test_expect_success 'blame -L/no match/' '\n+\t! git blame -L/hlagh/ cow >current 2>errors &&\n+\tgrep hlagh errors &&\n+\t! test -s current\n+'\n+\n+test_expect_success 'blame -L/invalid regex/' '\n+\t# At least GNU, Solaris and FreeBSD (and by extension, Mac OS X)\n+\t# complain about this\n+\t! git blame -L/[b-a]/ cow >current 2>errors &&\n+\tgrep b-a errors &&\n+\t! test -s current\n+'\n+\n test_expect_success 'indent of line numbers, nine lines' '\n \tgit blame nine_lines >actual &&\n \ttest $(grep -c \" \" actual) = 0\n-- \n1.7.0.4\n"},{"id":"145821","messageId":"7voce3nppy.fsf@alter.siamese.dyndns.org","threadId":"24413","inReplyTo":"1279294507-9238-1-git-send-email-avarab@gmail.com","subject":"Re: [PATCH] blame: Add tests for -L/start/,/end/","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2010-07-19T21:09:29Z","receivedAt":"2010-07-19T21:09:29Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ævar Arnfjörð Bjarmason writes:\n\n> git-pickaxe (later git-blame) gained support for the -L/start/,/end/\n> form in 2006 (931233bc66 by Junio C Hamano), but nothing was added to\n> test this functionality. Change that by adding more -L tests to\n> t8003-blame.sh.\n\nIf we look at the existing tests carefully, there is no tests for the\nrange notation -L, in general, not just the regexp variant. A\nfew existing uses are only to limit the output for testing other features\nand take it granted that -L works correctly (meaning they may detect it if\nyou break -L implementation but that is merely a side effect).\n\n> +for comma in '' ','\n> +do\n> +\t# The comma in -L/regex/, is optional\n\nIs it just -L/regex/? I thought -L regardless of the shape of \nwas equivalent to -L,END-OF-FILE.\n\n> +test_expect_success 'blame -LINT,/end/' '\n> +\tgit blame -L3,/GH/ cow >current 2>errors &&\n\nThis is somewhat an interesting one. It asks for a range that begins at\nthe third line in the file, extending to a line after that line that\nmatches the given expression. Unfortunately the test vector does not have\na line that contains GH before the third line, nor more than one lines\nthat contain GH after the third line (we should stop at the first hit), so\nthis test is not as effective as it could be.\n\n> +test_expect_success 'blame -L,/end/' '\n> +\tgit blame -L\",/^X/\" cow >current 2>errors &&\n\nMissing defaults to the beginning of file; the same as -L,INT and\nnot specific to the regex variant.\n\n> +test_expect_success 'blame -L/invalid regex/' '\n> +\t# At least GNU, Solaris and FreeBSD (and by extension, Mac OS X)\n> +\t# complain about this\n> +\t! git blame -L/[b-a]/ cow >current 2>errors &&\n> +\tgrep b-a errors &&\n\nThe b-a in the error message comes from us, not the regex library, so this\ntest should be portable, I think, as long as the library detects the empty\nrange correctly.\n"},{"id":"145826","messageId":"AANLkTinyTFzZgL2U7cvwnRVJeZTAhS3xW4v3TT6llVAn@mail.gmail.com","threadId":"24413","inReplyTo":"7voce3nppy.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] blame: Add tests for -L/start/,/end/","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2010-07-19T21:21:49Z","receivedAt":"2010-07-19T21:21:49Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"On Mon, Jul 19, 2010 at 21:09, Junio C Hamano wrote:\n> Ævar Arnfjörð Bjarmason   writes:\n>\n>> git-pickaxe (later git-blame) gained support for the -L/start/,/end/\n>> form in 2006 (931233bc66 by Junio C Hamano), but nothing was added to\n>> test this functionality. Change that by adding more -L tests to\n>> t8003-blame.sh.\n>\n> If we look at the existing tests carefully, there is no tests for the\n> range notation -L, in general, not just the regexp variant.  A\n> few existing uses are only to limit the output for testing other features\n> and take it granted that -L works correctly (meaning they may detect it if\n> you break -L implementation but that is merely a side effect).\n\nI didn't submit tests for -L in general because I'm not happy with how\nit behaves (as noted in \"[BUG?] blame: Odd -L 1,+0 behavior\"). So this\npatch only tests the regex portion of parse_loc().\n\nI might add tests for -L in general it later, but that patch will\nprobably include a behavior change.\n\n>> +for comma in '' ','\n>> +do\n>> +     # The comma in -L/regex/, is optional\n>\n> Is it just -L/regex/?  I thought -L regardless of the shape of \n> was equivalent to -L,END-OF-FILE.\n\nNo, it's not just -L/regex/, but I try to in general to write tests\nfor thing that *might* break, not just things that are guaranteed not\nto break as things are now implemented. If blame.c is rewritten in the\nfuture \"-L123\", might continue to work but \"-L/start/,\" might be\nbroken as a result of some code shuffling.\n\n>> +test_expect_success 'blame -LINT,/end/' '\n>> +     git blame -L3,/GH/ cow  >current 2>errors &&\n>\n> This is somewhat an interesting one.  It asks for a range that begins at\n> the third line in the file, extending to a line after that line that\n> matches the given expression.  Unfortunately the test vector does not have\n> a line that contains GH before the third line, nor more than one lines\n> that contain GH after the third line (we should stop at the first hit), so\n> this test is not as effective as it could be.\n\nYeah, I didn't test for ambiguities like that. I was going to expand\non that later, but see above about not having patched that yet.\n\n>> +test_expect_success 'blame -L,/end/' '\n>> +     git blame -L\",/^X/\" cow >current 2>errors &&\n>\n> Missing defaults to the beginning of file; the same as -L,INT and\n> not specific to the regex variant.\n\nYup, but I think it should be tested for anyway.\n\n>> +test_expect_success 'blame -L/invalid regex/' '\n>> +     # At least GNU, Solaris and FreeBSD (and by extension, Mac OS X)\n>> +     # complain about this\n>> +     ! git blame -L/[b-a]/ cow >current 2>errors &&\n>> +     grep b-a errors &&\n>\n> The b-a in the error message comes from us, not the regex library, so this\n> test should be portable, I think, as long as the library detects the empty\n> range correctly.\n\nYeah, it's just a test for:\n\n 1. Invalid regexes being detected at all, i.e. git not ignoring\n the return value of regcomp().\n\n 2. That we have an error message at all for it, and that the error\n message includes the invalid regex that the user supplied.\n"}]}