Re: [PATCH] test-lint-duplicates: Only check for numbered test cases

["Torsten Bögershausen" <tboegi@web.de>]

On 03.04.13 16:28, Jeff King wrote:
> On Wed, Apr 03, 2013 at 07:54:02AM +0200, Torsten Bögershausen wrote:
> 
>> Running make inside contrib/remote-helpers fails in "test-lint-duplicates"
>>
>> This was because the regexp checking for duplicate numbers strips everything
>> after the first "-" in the filename, including the prefix.
>>
>> As a result, 2 pathnames like
>> "xxxx/contrib/remote-helpers/test-bzr.sh" and
>> "xxxx/contrib/remote-helpers/test-hg-bidi.sh"
>>
>> are both converted into
>> "xxxx/contrib/remote", and reported as duplicate.
>>
>> Improve the regexp:
>> Remove everything after tNNNN- (where X stand for a digit)
> 
> I think the approach to just make test-lint-duplicates a no-op on
> non-numbered tests is reasonable, but this is side-stepping half of the
> issue. The problems are:
> 
>   1. We do not have numbers in our test names.
> 
>   2. We _do_ have full paths in the test names, which might have
>      elements which look like test script names.
> 
> Your patch tightens the match so that a hyphen in the path name does not
> confuse our script. But it trades it for being confused by tNNNN in the
> pathname. Which is admittedly less likely, but is not addressing the
> fundamental issues that we should only be processing basenames.
> 
> So something like "sed 's,.*/,,'" would fix that. But that still leaves
> us with a bunch of tests called "test-foo", "test-bar", etc, which will
> appear as duplicates. So we would still want to tighten the number
> parsing.
> 
> Like:
> 
> diff --git a/t/Makefile b/t/Makefile
> index 5c6de81..e5afa4c 100644
> --- a/t/Makefile
> +++ b/t/Makefile
> @@ -47,7 +47,9 @@ test-lint-duplicates:
>  test-lint: test-lint-duplicates test-lint-executable
>  
>  test-lint-duplicates:
> -	@dups=`echo $(T) | tr ' ' '\n' | sed 's/-.*//' | sort | uniq -d` && \
> +	@dups=`echo $(T) | tr ' ' '\n' | \
> +		sed -e 's,.*/,,' -e 's/\(t[0-9][0-9][0-9][0-9]\)-.*/\1/' | \
> +		sort | uniq -d` && \
>  		test -z "$$dups" || { \
>  		echo >&2 "duplicate test numbers:" $$dups; exit 1; }
>  
> 
> -Peff
I thinkg we need both the striping of the path and the "grepping" for
numbered test cases only.
I'll send a patch in a minute