git@vger.kernel.org list mirror (unofficial, one of many)
 help / color / mirror / Atom feed
From: Jeff King <peff@peff.net>
To: "SZEDER Gábor" <szeder.dev@gmail.com>
Cc: git@vger.kernel.org, Junio C Hamano <gitster@pobox.com>,
	Johannes Schindelin <johannes.schindelin@gmx.de>
Subject: Re: [PATCH 1/2] tests: make sure nested lazy prereqs work reliably
Date: Thu, 19 Nov 2020 12:56:08 -0500
Message-ID: <20201119175608.GA132922@coredump.intra.peff.net> (raw)
In-Reply-To: <20201119155824.GB25426@coredump.intra.peff.net>

On Thu, Nov 19, 2020 at 10:58:24AM -0500, Jeff King wrote:

> > +if test -z "$GIT_TEST_FAIL_PREREQS_INTERNAL" && test "$nestedworks" != yes
> > +then
> > +	say 'bug in test framework: nested lazy prerequisites do not work'
> > +	exit 1
> > +fi
> 
> I was surprised to see this bare exit, because I know we have some
> functions (run_sub_test_*) to help with testing the framework itself. It
> looks like the other prereq tests don't use it either, though. I wonder
> if there is a technical reason, or if they were simply added at a
> different time. (Either way, I am OK for your new test to match the
> surrounding ones like you have here).

I took a look at converting some of the existing tests. This seems to
work. It's a bit longer to read, perhaps, but I kind of like that the
expected outcome is all laid out. It also pollutes the test output less
(e.g., if you wanted to count up skipped tests in the whole suite, you'd
get a bunch of noise from t0000 for these uninteresting skips).

Thoughts? I think this is something I'd do on top of your patch.

diff --git a/t/t0000-basic.sh b/t/t0000-basic.sh
index f4ba2e8c85..f369af76be 100755
--- a/t/t0000-basic.sh
+++ b/t/t0000-basic.sh
@@ -759,43 +759,50 @@ test_expect_success '--run invalid range end' "
 	EOF_ERR
 "
 
-
-test_set_prereq HAVEIT
-haveit=no
-test_expect_success HAVEIT 'test runs if prerequisite is satisfied' '
-	test_have_prereq HAVEIT &&
-	haveit=yes
-'
-donthaveit=yes
-test_expect_success DONTHAVEIT 'unmet prerequisite causes test to be skipped' '
-	donthaveit=no
-'
-if test -z "$GIT_TEST_FAIL_PREREQS_INTERNAL" -a $haveit$donthaveit != yesyes
-then
-	say "bug in test framework: prerequisite tags do not work reliably"
-	exit 1
-fi
-
-test_set_prereq HAVETHIS
-haveit=no
-test_expect_success HAVETHIS,HAVEIT 'test runs if prerequisites are satisfied' '
-	test_have_prereq HAVEIT &&
-	test_have_prereq HAVETHIS &&
-	haveit=yes
-'
-donthaveit=yes
-test_expect_success HAVEIT,DONTHAVEIT 'unmet prerequisites causes test to be skipped' '
-	donthaveit=no
-'
-donthaveiteither=yes
-test_expect_success DONTHAVEIT,HAVEIT 'unmet prerequisites causes test to be skipped' '
-	donthaveiteither=no
+test_expect_success 'tests respect prerequisites' '
+	run_sub_test_lib_test prereqs "tests respect prereqs" <<-\EOF &&
+
+	test_set_prereq HAVEIT
+	haveit=no
+	test_expect_success HAVEIT "prereq is satisfied" "
+		test_have_prereq HAVEIT &&
+		haveit=yes
+	"
+
+	donthaveit=yes
+	test_expect_success DONTHAVEIT "prereq not satisfied" "
+		donthaveit=no
+	"
+
+	test_set_prereq HAVETHIS
+	haveit=no
+	test_expect_success HAVETHIS,HAVEIT "multiple prereqs" "
+		test_have_prereq HAVEIT &&
+		test_have_prereq HAVETHIS &&
+		haveit=yes
+	"
+
+	donthaveit=yes
+	test_expect_success HAVEIT,DONTHAVEIT "mixed prereqs (yes,no)" "
+		donthaveit=no
+	"
+
+	donthaveiteither=yes
+	test_expect_success DONTHAVEIT,HAVEIT "mixed prereqs (no,yes)" "
+		donthaveiteither=no
+	"
+	test_done
+	EOF
+	check_sub_test_lib_test prereqs <<-\EOF
+	ok 1 - prereq is satisfied
+	ok 2 # skip prereq not satisfied (missing DONTHAVEIT)
+	ok 3 - multiple prereqs
+	ok 4 # skip mixed prereqs (yes,no) (missing DONTHAVEIT of HAVEIT,DONTHAVEIT)
+	ok 5 # skip mixed prereqs (no,yes) (missing DONTHAVEIT of DONTHAVEIT,HAVEIT)
+	# passed all 5 test(s)
+	1..5
+	EOF
 '
-if test -z "$GIT_TEST_FAIL_PREREQS_INTERNAL" -a $haveit$donthaveit$donthaveiteither != yesyesyes
-then
-	say "bug in test framework: multiple prerequisite tags do not work reliably"
-	exit 1
-fi
 
 test_lazy_prereq LAZY_TRUE true
 havetrue=no

  reply	other threads:[~2020-11-19 18:01 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-11-18 19:04 SZEDER Gábor
2020-11-18 19:04 ` [PATCH 2/2] tests: fix description of 'test_set_prereq' SZEDER Gábor
2020-11-18 20:00 ` [PATCH 1/2] tests: make sure nested lazy prereqs work reliably Junio C Hamano
2020-11-19 15:58 ` Jeff King
2020-11-19 17:56   ` Jeff King [this message]
2020-11-19 19:50     ` Junio C Hamano
2020-11-20  0:14       ` Jeff King
2020-11-20  0:17         ` [PATCH 1/4] t0000: keep clean-up tests together Jeff King
2020-11-20  0:20         ` [PATCH 2/4] t0000: run prereq tests inside sub-test Jeff King
2020-11-20  0:22         ` [PATCH 3/4] t0000: run cleaning test " Jeff King
2020-11-20  0:27         ` [PATCH 4/4] t0000: consistently use single quotes for outer tests Jeff King
2020-11-20  1:32         ` [PATCH 1/2] tests: make sure nested lazy prereqs work reliably Junio C Hamano
2020-11-20  0:07   ` 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

  List information: http://vger.kernel.org/majordomo-info.html

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

  git send-email \
    --in-reply-to=20201119175608.GA132922@coredump.intra.peff.net \
    --to=peff@peff.net \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=johannes.schindelin@gmx.de \
    --cc=szeder.dev@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

git@vger.kernel.org list mirror (unofficial, one of many)

This inbox may be cloned and mirrored by anyone:

	git clone --mirror https://public-inbox.org/git
	git clone --mirror http://ou63pmih66umazou.onion/git
	git clone --mirror http://czquwvybam4bgbro.onion/git
	git clone --mirror http://hjrcffqmbrq6wope.onion/git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V1 git git/ https://public-inbox.org/git \
		git@vger.kernel.org
	public-inbox-index git

Example config snippet for mirrors.
Newsgroups are available over NNTP:
	nntp://news.public-inbox.org/inbox.comp.version-control.git
	nntp://ou63pmih66umazou.onion/inbox.comp.version-control.git
	nntp://czquwvybam4bgbro.onion/inbox.comp.version-control.git
	nntp://hjrcffqmbrq6wope.onion/inbox.comp.version-control.git
	nntp://news.gmane.io/gmane.comp.version-control.git
 note: .onion URLs require Tor: https://www.torproject.org/

code repositories for the project(s) associated with this inbox:

	https://80x24.org/mirrors/git.git

AGPL code for this site: git clone https://public-inbox.org/public-inbox.git