All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Ævar Arnfjörð Bjarmason" <avarab@gmail.com>
To: git@vger.kernel.org
Cc: "Junio C Hamano" <gitster@pobox.com>,
	"Jonathan Nieder" <jrnieder@gmail.com>,
	"Ævar Arnfjörð Bjarmason" <avarab@gmail.com>
Subject: [PATCH v2] test-lib: Multi-prereq support only checked the last prereq
Date: Wed, 11 Aug 2010 12:04:38 +0000	[thread overview]
Message-ID: <1281528278-15659-1-git-send-email-avarab@gmail.com> (raw)
In-Reply-To: <20100811014325.GA4169@burratino>

The support for multiple test prerequisites added by me in "test-lib:
Add support for multiple test prerequisites" was broken.

The for iterated over each prerequisite and returned true/false within
a case statement, but since it missed a return statement only the last
prerequisite in the list of prerequisites was ever considered, the
rest were ignored.

Fix that by changing the test_have_prereq code to something less
clever that keeps a count of the total prereqs and the ones we have
and compares the count at the end.

This comes with the added advantage that it's easy to list the missing
prerequisites in the test output, implement that while I'm at it.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---

This v2 patch has a new commit message.

On Wed, Aug 11, 2010 at 01:43, Jonathan Nieder <jrnieder@gmail.com> wrote:
> Ævar Arnfjörð Bjarmason wrote:
>
>> The clever for-loop (which I blindly copied from Junio)
>
> You did not copy his “return” statement, though. :)

Oops, yeah. I copied it from some mix-match of different
patches. Corrected the patch message for that.

> Wouldn’t
>
> -               ! : nope ;;
> +               return 1 ;;
>
> be simpler?

Yeah, that'd make it work, but I wanted to add something that spewed a
complete listing of missing prereqs for ecah test, bailing out with a
return wouldn't do that.

 t/t0000-basic.sh |    6 +++++-
 t/test-lib.sh    |   21 ++++++++++++++++++---
 2 files changed, 23 insertions(+), 4 deletions(-)

diff --git a/t/t0000-basic.sh b/t/t0000-basic.sh
index 2887677..9602085 100755
--- a/t/t0000-basic.sh
+++ b/t/t0000-basic.sh
@@ -84,7 +84,11 @@ donthaveit=yes
 test_expect_success HAVEIT,DONTHAVEIT 'unmet prerequisites causes test to be skipped' '
     donthaveit=no
 '
-if test $haveit$donthaveit != yesyes
+donthaveiteither=yes
+test_expect_success DONTHAVEIT,HAVEIT 'unmet prerequisites causes test to be skipped' '
+    donthaveiteither=no
+'
+if test $haveit$donthaveit$donthaveiteither != yesyesyes
 then
 	say "bug in test framework: multiple prerequisite tags do not work reliably"
 	exit 1
diff --git a/t/test-lib.sh b/t/test-lib.sh
index 4e73fff..8c8e129 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -337,15 +337,30 @@ test_have_prereq () {
 	IFS=,
 	set -- $*
 	IFS=$save_IFS
+
+	total_prereq=0
+	ok_prereq=0
+	missing_prereq=
+
 	for prerequisite
 	do
+		total_prereq=$(($total_prereq + 1))
 		case $satisfied in
 		*" $prerequisite "*)
-			: yes, have it ;;
+			ok_prereq=$(($ok_prereq + 1))
+			;;
 		*)
-			! : nope ;;
+			# Keep a list of missing prerequisites
+			if test -z "$missing_prereq"
+			then
+				missing_prereq=$prerequisite
+			else
+				missing_prereq="$prerequisite,$missing_prereq"
+			fi
 		esac
 	done
+
+	test $total_prereq = $ok_prereq
 }
 
 # You are not expected to call test_ok_ and test_failure_ directly, use
@@ -408,7 +423,7 @@ test_skip () {
 	case "$to_skip" in
 	t)
 		say_color skip >&3 "skipping test: $@"
-		say_color skip "ok $test_count # skip $1 (prereqs: $prereq)"
+		say_color skip "ok $test_count # skip $1 (missing $missing_prereq of $prereq)"
 		: true
 		;;
 	*)
-- 
1.7.2.1.295.gdf931

  reply	other threads:[~2010-08-11 12:04 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-08-10 23:21 [PATCH] test-lib: Multi-prereq support only checked the last prereq Ævar Arnfjörð Bjarmason
2010-08-11  1:43 ` Jonathan Nieder
2010-08-11 12:04   ` Ævar Arnfjörð Bjarmason [this message]
2010-08-13 20:28     ` [PATCH v2] " Johannes Sixt
2010-08-24  7:34     ` [PATCH] tests: simplify "missing PREREQ" message Jonathan Nieder
2010-08-24 10:16       ` Johannes Sixt
2010-08-24 19:01       ` Junio C Hamano

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1281528278-15659-1-git-send-email-avarab@gmail.com \
    --to=avarab@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=jrnieder@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.