threads / patch / 24344

patchtest-lib: simplify GIT_SKIP_TESTS loop

Subject: [PATCH] test-lib: simplify GIT_SKIP_TESTS loop

## tl;dr

3 messages between Jul 9, 2010 and Jul 12, 2010. Diffs are folded; open one to read it.

replies: 2people: 2as markdown or json

Michael J Gruber· Jul 9, 2010, 10:01 UTC · lore

04ece59 (GIT_SKIP_TESTS: allow users to omit tests that are known to break, 2006-12-28) introduced GIT_SKIP_TESTS, and since then we have had two nested loops iterating over GIT_SKIP_TESTS with the same loop variable.

Reduce this to one loop.

Signed-off-by: Michael J Gruber <git@drmicha.warpmail.net>
---
I am probably making a complete shell fool out of myself by overlooking
something completely trivial.

But just in case I am not, this reduces the loop from O(N^2) to O(N), although I do admit that N is typically not that large... (Note that the inner $skp does not overwrite the outer one, at least not with bash.)

 t/test-lib.sh |   23 ++++++++++-------------
 1 files changed, 10 insertions(+), 13 deletions(-)

Show changes to t/test-lib.sh +10 −13

diff --git a/t/test-lib.sh b/t/test-lib.sh
index ac496aa..8e3de53 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -827,23 +827,20 @@ cd -P "$test" || exit 1
 
 this_test=${0##*/}
 this_test=${this_test%%-*}
+to_skip=
 for skp in $GIT_SKIP_TESTS
 do
-	to_skip=
-	for skp in $GIT_SKIP_TESTS
-	do
-		case "$this_test" in
-		$skp)
-			to_skip=t
-		esac
-	done
-	case "$to_skip" in
-	t)
-		say_color skip >&3 "skipping test $this_test altogether"
-		say_color skip "skip all tests in $this_test"
-		test_done
+	case "$this_test" in
+	$skp)
+		to_skip=t
 	esac
 done
+case "$to_skip" in
+t)
+	say_color skip >&3 "skipping test $this_test altogether"
+	say_color skip "skip all tests in $this_test"
+	test_done
+esac
 
 # Provide an implementation of the 'yes' utility
 yes () {

-- 
1.7.2.rc1.212.g850a

Junio C Hamano· Jul 9, 2010, 23:07 UTC · re: Michael J Gruber · lore

Re: [PATCH] test-lib: simplify GIT_SKIP_TESTS loop

Michael J Gruber <git@drmicha.warpmail.net> writes:

> I am probably making a complete shell fool out of myself by overlooking
> something completely trivial.

I don't think so. I don't know what the double-loop is trying to achieve myself.

We could even lose to_skip variable and the call to test_done inside the case statement, perhaps like this.

Thanks.

 t/test-lib.sh |   13 +++----------
 1 files changed, 3 insertions(+), 10 deletions(-)

Show changes to t/test-lib.sh +3 −10

diff --git a/t/test-lib.sh b/t/test-lib.sh
index ac496aa..bc06564 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -385,6 +385,7 @@ test_skip () {
 		case $this_test.$test_count in
 		$skp)
 			to_skip=t
+			break
 		esac
 	done
 	if test -z "$to_skip" && test -n "$prereq" &&
@@ -829,16 +830,8 @@ this_test=${0##*/}
 this_test=${this_test%%-*}
 for skp in $GIT_SKIP_TESTS
 do
-	to_skip=
-	for skp in $GIT_SKIP_TESTS
-	do
-		case "$this_test" in
-		$skp)
-			to_skip=t
-		esac
-	done
-	case "$to_skip" in
-	t)
+	case "$this_test" in
+	$skp)
 		say_color skip >&3 "skipping test $this_test altogether"
 		say_color skip "skip all tests in $this_test"
 		test_done

Michael J Gruber· Jul 12, 2010, 10:32 UTC · re: Junio C Hamano · lore

[PATCHv2] test-lib: simplify GIT_SKIP_TESTS loop

Reduce this to one loop.

Signed-off-by: Michael J Gruber <git@drmicha.warpmail.net>
---
v2 is simplified even more and is really Junio's patch.

 t/test-lib.sh |   13 +++----------
 1 files changed, 3 insertions(+), 10 deletions(-)

Show changes to t/test-lib.sh +3 −10

diff --git a/t/test-lib.sh b/t/test-lib.sh
index ac496aa..bc06564 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -385,6 +385,7 @@ test_skip () {
 		case $this_test.$test_count in
 		$skp)
 			to_skip=t
+			break
 		esac
 	done
 	if test -z "$to_skip" && test -n "$prereq" &&
@@ -829,16 +830,8 @@ this_test=${0##*/}
 this_test=${this_test%%-*}
 for skp in $GIT_SKIP_TESTS
 do
-	to_skip=
-	for skp in $GIT_SKIP_TESTS
-	do
-		case "$this_test" in
-		$skp)
-			to_skip=t
-		esac
-	done
-	case "$to_skip" in
-	t)
+	case "$this_test" in
+	$skp)
 		say_color skip >&3 "skipping test $this_test altogether"
 		say_color skip "skip all tests in $this_test"
 		test_done

-- 
1.7.2.rc1.212.g850a