Re: [PATCH 1/6] t4051: add test for comments preceding function lines

["René Scharfe" <l.s.r@web.de>]

Am 19.11.2017 um 02:18 schrieb Eric Sunshine:
> On Sat, Nov 18, 2017 at 1:04 PM, René Scharfe <l.s.r@web.de> wrote:
>> When showing function context it would be helpful to show comments
>> immediately before declarations, as they are most likely relevant.  Add
>> a test for that.
>>
>> Signed-off-by: Rene Scharfe <l.s.r@web.de>
>> ---
>> diff --git a/t/t4051-diff-function-context.sh b/t/t4051-diff-function-context.sh
>> @@ -85,6 +85,10 @@ test_expect_success 'setup' '
>>> +test_expect_failure ' context includes comment' '
>> +       grep "^ .*Hello comment" changed_hello.diff
>> +'
> 
> This shows only that the the "* Hello comment." line is in the context
> but says nothing about the entire comment block being included (which
> patch 3/6 implements). Perhaps the test could be more thorough?

You say that as if it was a bad thing.  That line represents 33% of
your suggested full-comment metric, i.e. more than nothing.  But more
importantly it's the actual comment payload.  The leading "/*" line is
included as a consequence of the employed heuristic, but a more
refined one might omit it as it doesn't actually contain any comment.

Not sure if we actually want to tweak our heuristic further, but the
test is meant to check only if the interesting part is present and
give leeway regarding other implementation details.

> 
>> diff --git a/t/t4051/hello.c b/t/t4051/hello.c
>> index 63b1a1e4ef..73e767e178 100644
>> --- a/t/t4051/hello.c
>> +++ b/t/t4051/hello.c
>> @@ -1,4 +1,7 @@
>>
>> +/*
>> + * Hello comment.
>> + */
>>   static void hello(void)        // Begin of hello
>>   {