threads / patch / 20683

patchtest suite: add a check that all test numbers are unique

Subject: [PATCH] test suite: add a check that all test numbers are unique

## tl;dr

4 messages between Aug 21, 2009 and Aug 21, 2009. Diffs are folded; open one to read it.

replies: 3people: 2as markdown or json

Johannes Sixt· Aug 21, 2009, 09:28 UTC · lore

From: Johannes Sixt <j6t@kdbg.org>

The test runs only if 'make all' is used. Its purpose is to alert our valued integrator if different branches are merged that happen to introduce the same test number.

Signed-off-by: Johannes Sixt <j6t@kdbg.org>
---
 t/Makefile                |    1 +
 t/check_unique_numbers.sh |   27 +++++++++++++++++++++++++++
 2 files changed, 28 insertions(+), 0 deletions(-)
 create mode 100755 t/check_unique_numbers.sh

Show changes to 2 files +28 −0

t/Makefile, t/check_unique_numbers.sh

diff --git a/t/Makefile b/t/Makefile
index bd09390..2a451b7 100644
--- a/t/Makefile
+++ b/t/Makefile
@@ -17,6 +17,7 @@ T = $(wildcard t[0-9][0-9][0-9][0-9]-*.sh)
 TSVN = $(wildcard t91[0-9][0-9]-*.sh)

 all: pre-clean
+	'$(SHELL_PATH_SQ)' ./check_unique_numbers.sh
 	$(MAKE) aggregate-results-and-cleanup

 $(T):
diff --git a/t/check_unique_numbers.sh b/t/check_unique_numbers.sh
new file mode 100755
index 0000000..e767275
--- /dev/null
+++ b/t/check_unique_numbers.sh
@@ -0,0 +1,27 @@
+#!/bin/sh
+
+# checks whether test case numbers are unique;
+# returns non-zero if any duplicates were found
+
+check_numbers () {
+	last= dup=
+	while read name
+	do
+		case $name in
+		t[0-9][0-9][0-9][0-9]-*.sh)
+			number=${name%%-*}
+			if test "$number" = "$last"; then
+				dup="$dup $number"
+			fi
+			last=$number
+			;;
+		esac
+	done
+	test -z "$dup" || {
+		echo >&2 "error: duplicate test numbers:" $dup
+		return 1
+	}
+}
+
+ls -1 |	# no wildcard to avoid overflow of command line
+check_numbers

-- 
1.6.4.204.g6aad7

Jeff King· Aug 21, 2009, 12:53 UTC · re: Johannes Sixt · lore

Re: [PATCH] test suite: add a check that all test numbers are unique

On Fri, Aug 21, 2009 at 11:28:15AM +0200, Johannes Sixt wrote:

> The test runs only if 'make all' is used. Its purpose is to alert our
> valued integrator if different branches are merged that happen to
> introduce the same test number.

I think this is probably useful. In addition to the t7406 you posted a patch for, it looks like t4037 has a duplicate in next.

As for the implementation:

Show 29 quoted lines

> +++ b/t/check_unique_numbers.sh
> @@ -0,0 +1,27 @@
> +#!/bin/sh
> +
> +# checks whether test case numbers are unique;
> +# returns non-zero if any duplicates were found
> +
> +check_numbers () {
> +	last= dup=
> +	while read name
> +	do
> +		case $name in
> +		t[0-9][0-9][0-9][0-9]-*.sh)
> +			number=${name%%-*}
> +			if test "$number" = "$last"; then
> +				dup="$dup $number"
> +			fi
> +			last=$number
> +			;;
> +		esac
> +	done
> +	test -z "$dup" || {
> +		echo >&2 "error: duplicate test numbers:" $dup
> +		return 1
> +	}
> +}
> +
> +ls -1 |	# no wildcard to avoid overflow of command line
> +check_numbers

Why not the much shorter:

tests() {
  ls | sed -n 's/^\(t[0-9][0-9][0-9][0-9]\)-.*\.sh$/\1/p'
}
dups=`tests | uniq -d`

instead of the long shell loop?

-Peff

Johannes Sixt· Aug 21, 2009, 13:13 UTC · re: Jeff King · lore

[PATCH v2] test suite: add a check that all test numbers are unique

Jeff King schrieb:

Show 8 quoted lines

> Why not the much shorter:
> 
> tests() {
>   ls | sed -n 's/^\(t[0-9][0-9][0-9][0-9]\)-.*\.sh$/\1/p'
> }
> dups=`tests | uniq -d`
> 
> instead of the long shell loop?

Because it's faster on Windows ;-) No, seriously, I didn't know about uniq -d.

Here is a replacement patch.

--- 8< ---
From: Johannes Sixt <j6t@kdbg.org>
Subject: [PATCH] t/Makefile: add a check that all test numbers are unique

The test runs only if 'make all' is used. Its purpose is to alert our valued integrator if different branches are merged that happen to introduce the same test number.

Signed-off-by: Johannes Sixt <j6t@kdbg.org>
---
 t/Makefile |    4 ++++
 1 files changed, 4 insertions(+), 0 deletions(-)

Show changes to t/Makefile +4 −0

diff --git a/t/Makefile b/t/Makefile
index bd09390..ed8281a 100644
--- a/t/Makefile
+++ b/t/Makefile
@@ -17,6 +17,10 @@ T = $(wildcard t[0-9][0-9][0-9][0-9]-*.sh)
 TSVN = $(wildcard t91[0-9][0-9]-*.sh)

 all: pre-clean
+	@dups=`ls | sed -n 's/^\(t[0-9][0-9][0-9][0-9]\)-.*\.sh$$/\1/p' | \
+			uniq -d` && \
+		test -z "$$dups" || { \
+		echo "duplicate test numbers:" $$dups; exit 1; }
 	$(MAKE) aggregate-results-and-cleanup

 $(T):

-- 
1.6.4.204.g6aad7

Jeff King· Aug 21, 2009, 14:37 UTC · re: Johannes Sixt · lore

Re: [PATCH v2] test suite: add a check that all test numbers are unique

On Fri, Aug 21, 2009 at 03:13:23PM +0200, Johannes Sixt wrote:

> Because it's faster on Windows ;-) No, seriously, I didn't know about
> uniq -d.

:)

> Here is a replacement patch.

Looks good to me. Thanks.

-Peff