git@vger.kernel.org list mirror (unofficial, one of many)
 help / color / mirror / Atom feed
From: Jeff King <peff@peff.net>
To: Junio C Hamano <gitster@pobox.com>
Cc: "SZEDER Gábor" <szeder.dev@gmail.com>,
	git@vger.kernel.org,
	"Johannes Schindelin" <johannes.schindelin@gmx.de>
Subject: [PATCH 2/4] t0000: run prereq tests inside sub-test
Date: Thu, 19 Nov 2020 19:20:22 -0500
Message-ID: <20201120002022.GB307112@coredump.intra.peff.net> (raw)
In-Reply-To: <20201120001458.GA274082@coredump.intra.peff.net>

We test the behavior of prerequisites in t0000 by setting up fake ones
in the main test script, trying to run some tests, and then seeing if
those tests impacted the environment correctly. If they didn't, then we
write a message and manually call exit.

Instead, let's push these down into a sub-test, like many of the other
tests covering the framework itself. This has a few advantages:

  - it does not pollute the test output with mention of skipped tests
    (that we know are uninteresting -- the point of the test was to see
    that these are skipped).

  - when running in a TAP harness, we get a useful test failure message
    (whereas when the script exits early, a tool like "prove" simply
    says "Dubious, test returned 1").

  - we do not have to worry about different test environments, such as
    when GIT_TEST_FAIL_PREREQS_INTERNAL is set. Our sub-test helpers
    already give us a known environment.

  - the tests themselves are a bit easier to read, as we can just check
    the test-framework output to see what happened (and get the usual
    test_cmp diff if it failed)

A few notes on the implementation:

  - we could do one sub-test per each individual test_expect_success. I
    broke it up here into a few logical groups, as I think this makes it
    more readable

  - the original tests modified environment variables inside the test
    bodies. Instead, I've used "true" as the body of a test we expect to
    run and "false" otherwise. Technically this does not confirm that
    the body of the "true" test actually ran. We are trusting the
    framework output to believe that it truly ran, which is sufficient
    for these tests. And I think the end result is much simpler to
    follow.

  - the nested_prereq test uses a few bare "test -f" calls; I converted
    these to our usual test_path_is_* helpers while moving the code
    around.

Signed-off-by: Jeff King <peff@peff.net>
---
The diff is pretty gnarly. I recommend just reading the before/after
sections (I wish there was a way to convince diff _not_ to generate the
minimal diff; two big blocks would have been more readable).

 t/t0000-basic.sh | 149 ++++++++++++++++++++++-------------------------
 1 file changed, 69 insertions(+), 80 deletions(-)

diff --git a/t/t0000-basic.sh b/t/t0000-basic.sh
index d49b5dd4ac..3d36a87610 100755
--- a/t/t0000-basic.sh
+++ b/t/t0000-basic.sh
@@ -759,96 +759,85 @@ test_expect_success '--run invalid range end' "
 	EOF_ERR
 "
 
+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 '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 HAVEIT
+	test_expect_success HAVEIT "prereq is satisfied" "true"
+	test_expect_success "have_prereq works" "
+		test_have_prereq HAVEIT
+	"
+	test_expect_success DONTHAVEIT "prereq not satisfied" "false"
 
-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
-'
-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_set_prereq HAVETHIS
+	test_expect_success HAVETHIS,HAVEIT "multiple prereqs" "true"
+	test_expect_success HAVEIT,DONTHAVEIT "mixed prereqs (yes,no)" "false"
+	test_expect_success DONTHAVEIT,HAVEIT "mixed prereqs (no,yes)" "false"
 
-test_lazy_prereq LAZY_TRUE true
-havetrue=no
-test_expect_success LAZY_TRUE 'test runs if lazy prereq is satisfied' '
-	havetrue=yes
-'
-donthavetrue=yes
-test_expect_success !LAZY_TRUE 'missing lazy prereqs skip tests' '
-	donthavetrue=no
+	test_done
+	EOF
+
+	check_sub_test_lib_test prereqs <<-\EOF
+	ok 1 - prereq is satisfied
+	ok 2 - have_prereq works
+	ok 3 # skip prereq not satisfied (missing DONTHAVEIT)
+	ok 4 - multiple prereqs
+	ok 5 # skip mixed prereqs (yes,no) (missing DONTHAVEIT of HAVEIT,DONTHAVEIT)
+	ok 6 # skip mixed prereqs (no,yes) (missing DONTHAVEIT of DONTHAVEIT,HAVEIT)
+	# passed all 6 test(s)
+	1..6
+	EOF
 '
 
-if test -z "$GIT_TEST_FAIL_PREREQS_INTERNAL" -a "$havetrue$donthavetrue" != yesyes
-then
-	say 'bug in test framework: lazy prerequisites do not work'
-	exit 1
-fi
+test_expect_success 'tests respect lazy prerequisites' '
+	run_sub_test_lib_test lazy-prereqs "respect lazy prereqs" <<-\EOF &&
 
-test_lazy_prereq LAZY_FALSE false
-nothavefalse=no
-test_expect_success !LAZY_FALSE 'negative lazy prereqs checked' '
-	nothavefalse=yes
-'
-havefalse=yes
-test_expect_success LAZY_FALSE 'missing negative lazy prereqs will skip' '
-	havefalse=no
-'
+	test_lazy_prereq LAZY_TRUE true
+	test_expect_success LAZY_TRUE "lazy prereq is satisifed" "true"
+	test_expect_success !LAZY_TRUE "negative lazy prereq" "false"
 
-if test -z "$GIT_TEST_FAIL_PREREQS_INTERNAL" -a "$nothavefalse$havefalse" != yesyes
-then
-	say 'bug in test framework: negative lazy prerequisites do not work'
-	exit 1
-fi
+	test_lazy_prereq LAZY_FALSE false
+	test_expect_success LAZY_FALSE "lazy prereq not satisfied" "false"
+	test_expect_success !LAZY_FALSE "negative false prereq" "true"
 
-test_lazy_prereq NESTED_INNER '
-	>inner &&
-	rm -f outer
-'
-test_lazy_prereq NESTED_PREREQ '
-	>outer &&
-	test_have_prereq NESTED_INNER &&
-	echo "can create new file in cwd" >file &&
-	test -f outer &&
-	test ! -f inner
-'
-test_expect_success NESTED_PREREQ 'evaluating nested lazy prereqs dont interfere with each other' '
-	nestedworks=yes
+	test_done
+	EOF
+
+	check_sub_test_lib_test lazy-prereqs <<-\EOF
+	ok 1 - lazy prereq is satisifed
+	ok 2 # skip negative lazy prereq (missing !LAZY_TRUE)
+	ok 3 # skip lazy prereq not satisfied (missing LAZY_FALSE)
+	ok 4 - negative false prereq
+	# passed all 4 test(s)
+	1..4
+	EOF
 '
 
-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
+test_expect_success 'nested lazy prerequisites' '
+	run_sub_test_lib_test nested-lazy "nested lazy prereqs" <<-\EOF &&
+
+	test_lazy_prereq NESTED_INNER "
+		>inner &&
+		rm -f outer
+	"
+	test_lazy_prereq NESTED_PREREQ "
+		>outer &&
+		test_have_prereq NESTED_INNER &&
+		echo can create new file in cwd >file &&
+		test_path_is_file outer &&
+		test_path_is_missing inner
+	"
+	test_expect_success NESTED_PREREQ "evaluate nested prereq" "true"
+
+	test_done
+	EOF
+
+	check_sub_test_lib_test nested-lazy <<-\EOF
+	ok 1 - evaluate nested prereq
+	# passed all 1 test(s)
+	1..1
+	EOF
+'
 
 test_expect_success 'lazy prereqs do not turn off tracing' "
 	run_sub_test_lib_test lazy-prereq-and-tracing \
-- 
2.29.2.730.g3e418f96ba


  parent reply	other threads:[~2020-11-20  0:22 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-11-18 19:04 [PATCH 1/2] tests: make sure nested lazy prereqs work reliably 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
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         ` Jeff King [this message]
2020-11-20  0:22         ` [PATCH 3/4] t0000: run cleaning test inside sub-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=20201120002022.GB307112@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