git/list[1] front-page[2] threads[3] people[4] search[5] about
 

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

From
Jeff King <peff@peff.net>
Date
Apr 3, 2013, 14:28 UTC
Message-ID
<20130403142804.GB10494@sigill.intra.peff.net>
In-Reply-To
<201304030754.03100.tboegi@web.de>
On Wed, Apr 03, 2013 at 07:54:02AM +0200, Torsten Bögershausen wrote:
Show 14 quoted lines
> 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
Previous: Torsten BögershausenNext: Torsten Bögershausen
Message 2 of 4 in “test-lint-duplicates: Only check for numbered test cases”
  1. test-lint-duplicates: Only check for numbered test casesTorsten Bögershausen, Apr 3, 2013
  2. Jeff KingApr 3, 2013
  3. Torsten BögershausenApr 10, 2013
  4. Jeff KingApr 10, 2013

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.