{"thread":{"id":"25458","subject":"[PATCH] Make test script annotate-tests.sh handle missing authors","startedAt":"2010-10-16T05:50:15Z","lastAt":"2010-10-16T12:22:16Z","messageCount":5,"participants":["Kevin Ballard","Jakub Narebski"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"153617","messageId":"1287208215-91901-1-git-send-email-kevin@sb.org","threadId":"25458","inReplyTo":null,"subject":"[PATCH] Make test script annotate-tests.sh handle missing authors","fromName":"Kevin Ballard","fromEmail":"kevin@sb.org","sentAt":"2010-10-16T05:50:15Z","receivedAt":"2010-10-16T05:50:15Z","isPatch":true,"sender":{"key":"kevin@sb.org","avatar":"https://avatars.githubusercontent.com/u/714?v=4"},"body":"Initialize the %count hash to contain all the expected authors already.\nThis allows the script to print an error if an expected author was omitted\nentirely from the blame/annotate output.\n\nSigned-off-by: Kevin Ballard <kevin@sb.org>\n---\nI discovered this omission when trying to write a test for a change to\ngit-blame that I will be submitting shortly. Without this change, if an\nauthor never showed up in the blame output, it would errneously consider\nthat to be ok. It still ignores authors that were never specified as\nexpected in the first place, but I wasn't so sure that was an error.\n\nAlso, I'm not a Perl programmer, so it's possible there's a better idiom\nfor this sort of thing.\n\n t/annotate-tests.sh |    3 +++\n 1 files changed, 3 insertions(+), 0 deletions(-)\n\ndiff --git a/t/annotate-tests.sh b/t/annotate-tests.sh\nindex 396b965..4e37a66 100644\n--- a/t/annotate-tests.sh\n+++ b/t/annotate-tests.sh\n@@ -9,6 +9,9 @@ check_count () {\n \tcat .result | perl -e '\n \t\tmy %expect = (@ARGV);\n \t\tmy %count = ();\n+\t\twhile (my ($author, $count) = each %expect) {\n+\t\t\t$count{$author} = 0;\n+\t\t}\n \t\twhile (<STDIN>) {\n \t\t\tif (/^[0-9a-f]+\\t\\(([^\\t]+)\\t/) {\n \t\t\t\tmy $author = $1;\n-- \n1.7.3.1.186.gc0af9.dirty\n"},{"id":"153620","messageId":"m3sk06617a.fsf@localhost.localdomain","threadId":"25458","inReplyTo":"1287208215-91901-1-git-send-email-kevin@sb.org","subject":"Re: [PATCH] Make test script annotate-tests.sh handle missing authors","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2010-10-16T07:34:58Z","receivedAt":"2010-10-16T07:34:58Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"Kevin Ballard <kevin@sb.org> writes:\n\n> Also, I'm not a Perl programmer, so it's possible there's a better idiom\n> for this sort of thing.\n> \n>  t/annotate-tests.sh |    3 +++\n>  1 files changed, 3 insertions(+), 0 deletions(-)\n> \n> diff --git a/t/annotate-tests.sh b/t/annotate-tests.sh\n> index 396b965..4e37a66 100644\n> --- a/t/annotate-tests.sh\n> +++ b/t/annotate-tests.sh\n> @@ -9,6 +9,9 @@ check_count () {\n>  \tcat .result | perl -e '\n>  \t\tmy %expect = (@ARGV);\n>  \t\tmy %count = ();\n> +\t\twhile (my ($author, $count) = each %expect) {\n> +\t\t\t$count{$author} = 0;\n> +\t\t}\n\n\nFirst, it is a very bad practice to have variables of different type\nnamed the same way, here %count (hash) and $count (scalar, unused).\n\nPerl idiom way would be\n\n  -\t\tmy %count = ();\n  +\t\tmy %count = map { $_ => 0 } keys %expect;\n\n>  \t\twhile (<STDIN>) {\n>  \t\t\tif (/^[0-9a-f]+\\t\\(([^\\t]+)\\t/) {\n>  \t\t\t\tmy $author = $1;\n> -- \n> 1.7.3.1.186.gc0af9.dirty\n> \n\n-- \nJakub Narebski\nPoland\nShadeHawk on #git\n"},{"id":"153623","messageId":"09193539-B5AD-4574-9FE4-983566A34355@sb.org","threadId":"25458","inReplyTo":"m3sk06617a.fsf@localhost.localdomain","subject":"Re: [PATCH] Make test script annotate-tests.sh handle missing authors","fromName":"Kevin Ballard","fromEmail":"kevin@sb.org","sentAt":"2010-10-16T10:43:51Z","receivedAt":"2010-10-16T10:43:51Z","isPatch":true,"sender":{"key":"kevin@sb.org","avatar":"https://avatars.githubusercontent.com/u/714?v=4"},"body":"On Oct 16, 2010, at 12:34 AM, Jakub Narebski wrote:\n\n> Kevin Ballard <kevin@sb.org> writes:\n> \n>> Also, I'm not a Perl programmer, so it's possible there's a better idiom\n>> for this sort of thing.\n>> \n>> t/annotate-tests.sh |    3 +++\n>> 1 files changed, 3 insertions(+), 0 deletions(-)\n>> \n>> diff --git a/t/annotate-tests.sh b/t/annotate-tests.sh\n>> index 396b965..4e37a66 100644\n>> --- a/t/annotate-tests.sh\n>> +++ b/t/annotate-tests.sh\n>> @@ -9,6 +9,9 @@ check_count () {\n>> \tcat .result | perl -e '\n>> \t\tmy %expect = (@ARGV);\n>> \t\tmy %count = ();\n>> +\t\twhile (my ($author, $count) = each %expect) {\n>> +\t\t\t$count{$author} = 0;\n>> +\t\t}\n> \n> \n> First, it is a very bad practice to have variables of different type\n> named the same way, here %count (hash) and $count (scalar, unused).\n\nThanks for the pointer, but $count is already used in the while loop below:\n\n\t\twhile (my ($author, $count) = each %count) {\n\t\t\tmy $ok;\n\t\t\tif ($expect{$author} != $count) {\n\t\t\t\t$bad = 1;\n\t\t\t\t$ok = \"bad\";\n\t\t\t}\n\t\t\telse {\n\t\t\t\t$ok = \"good\";\n\t\t\t}\n\t\t\tprint STDERR \"Author $author (expected $expect{$author}, attributed $count) $ok\\n\";\n\t\t}\n\nI'll go and change the one in my new while loop though, to use the Perl idiom way you listed below.\n\n> Perl idiom way would be\n> \n>  -\t\tmy %count = ();\n>  +\t\tmy %count = map { $_ => 0 } keys %expect;\n\n-Kevin Ballard"},{"id":"153624","messageId":"1287227360-2610-1-git-send-email-kevin@sb.org","threadId":"25458","inReplyTo":"09193539-B5AD-4574-9FE4-983566A34355@sb.org","subject":"[PATCH v2] Update test script annotate-tests.sh to handle missing/extra authors","fromName":"Kevin Ballard","fromEmail":"kevin@sb.org","sentAt":"2010-10-16T11:09:20Z","receivedAt":"2010-10-16T11:09:20Z","isPatch":true,"sender":{"key":"kevin@sb.org","avatar":"https://avatars.githubusercontent.com/u/714?v=4"},"body":"The current script used by annotate-tests.sh (used by t8001 and t8002) fails\nto emit a warning if any of the expected authors never show up in the output\nor if authors that show up in the output were never specified as expected.\nUpdate the script to fail in both of these scenarios.\n\nHelped-by: Jakub Narebski <jnareb@gmail.com>\nSigned-off-by: Kevin Ballard <kevin@sb.org>\n---\n t/annotate-tests.sh |   12 ++++++------\n 1 files changed, 6 insertions(+), 6 deletions(-)\n\ndiff --git a/t/annotate-tests.sh b/t/annotate-tests.sh\nindex 396b965..141b60c 100644\n--- a/t/annotate-tests.sh\n+++ b/t/annotate-tests.sh\n@@ -8,27 +8,27 @@ check_count () {\n \t$PROG file $head >.result || return 1\n \tcat .result | perl -e '\n \t\tmy %expect = (@ARGV);\n-\t\tmy %count = ();\n+\t\tmy %count = map { $_ => 0 } keys %expect;\n \t\twhile (<STDIN>) {\n \t\t\tif (/^[0-9a-f]+\\t\\(([^\\t]+)\\t/) {\n \t\t\t\tmy $author = $1;\n \t\t\t\tfor ($author) { s/^\\s*//; s/\\s*$//; }\n-\t\t\t\tif (exists $expect{$author}) {\n-\t\t\t\t\t$count{$author}++;\n-\t\t\t\t}\n+\t\t\t\t$count{$author}++;\n \t\t\t}\n \t\t}\n \t\tmy $bad = 0;\n \t\twhile (my ($author, $count) = each %count) {\n \t\t\tmy $ok;\n-\t\t\tif ($expect{$author} != $count) {\n+\t\t\tmy $value = 0;\n+\t\t\t$value = $expect{$author} if defined $expect{$author};\n+\t\t\tif ($value != $count) {\n \t\t\t\t$bad = 1;\n \t\t\t\t$ok = \"bad\";\n \t\t\t}\n \t\t\telse {\n \t\t\t\t$ok = \"good\";\n \t\t\t}\n-\t\t\tprint STDERR \"Author $author (expected $expect{$author}, attributed $count) $ok\\n\";\n+\t\t\tprint STDERR \"Author $author (expected $value, attributed $count) $ok\\n\";\n \t\t}\n \t\texit($bad);\n \t' \"$@\"\n-- \n1.7.3.1.208.g53ea2\n"},{"id":"153626","messageId":"201010161422.17483.jnareb@gmail.com","threadId":"25458","inReplyTo":"09193539-B5AD-4574-9FE4-983566A34355@sb.org","subject":"Re: [PATCH] Make test script annotate-tests.sh handle missing authors","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2010-10-16T12:22:16Z","receivedAt":"2010-10-16T12:22:16Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"On Sat, 16 Oct 2010, Kevin Ballard wrote:\n> On Oct 16, 2010, at 12:34 AM, Jakub Narebski wrote:\n>> Kevin Ballard <kevin@sb.org> writes:\n>> \n>>> Also, I'm not a Perl programmer, so it's possible there's a better idiom\n>>> for this sort of thing.\n>>> \n>>> t/annotate-tests.sh |    3 +++\n>>> 1 files changed, 3 insertions(+), 0 deletions(-)\n>>> \n>>> diff --git a/t/annotate-tests.sh b/t/annotate-tests.sh\n>>> index 396b965..4e37a66 100644\n>>> --- a/t/annotate-tests.sh\n>>> +++ b/t/annotate-tests.sh\n>>> @@ -9,6 +9,9 @@ check_count () {\n>>> \tcat .result | perl -e '\n>>> \t\tmy %expect = (@ARGV);\n>>> \t\tmy %count = ();\n>>> +\t\twhile (my ($author, $count) = each %expect) {\n>>> +\t\t\t$count{$author} = 0;\n>>> +\t\t}\n>> \n>> \n>> First, it is a very bad practice to have variables of different type\n>> named the same way, here %count (hash) and $count (scalar, unused).\n> \n> Thanks for the pointer, but $count is already used in the while loop below:\n> \n> \t\twhile (my ($author, $count) = each %count) {\n> \t\t\tmy $ok;\n> \t\t\tif ($expect{$author} != $count) {\n> \t\t\t\t$bad = 1;\n> \t\t\t\t$ok = \"bad\";\n> \t\t\t}\n> \t\t\telse {\n> \t\t\t\t$ok = \"good\";\n> \t\t\t}\n> \t\t\tprint STDERR \"Author $author (expected $expect{$author}, attributed $count) $ok\\n\";\n> \t\t}\n\nHmmm... the %count hash should probably be named %actual (to complement\n%expect), or %attributed (like in output).\n\n-- \nJakub Narebski\nPoland\n"}]}