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 3/4] t0000: run cleaning test inside sub-test
Date: Thu, 19 Nov 2020 19:22:47 -0500
Message-ID: <20201120002247.GC307112@coredump.intra.peff.net> (raw)
In-Reply-To: <20201120001458.GA274082@coredump.intra.peff.net>

Our check of test_when_finished is done directly in the main script, and
if we failed to clean, we complain and exit immediately. It's nicer to
signal a test failure here, for a few reasons:

  - this gives better output to the user when run under a TAP harness
    like "prove"

  - constency; it's the only test left in the file that behaves this way

  - half of its "if" conditional is nonsense anyway; it picked up a
    reference to GIT_TEST_FAIL_PREREQS internal in dfe1a17df9 (tests:
    add a special setup where prerequisites fail, 2019-05-13) along with
    its neighbors, even though it has nothing to do with that flag

We could actually do this without a sub-test at all, and just put our
two tests (one to do cleanup, and one to check that it happened) in the
main script. But doing it in a subtest is conceptually cleaner (from the
perspective of the main test script, we are checking only one thing),
and it remains consistent with the "cleanup when failing" test directly
after it, which has to happen in a sub-test (to avoid the main script
complaining of the failed test).

Signed-off-by: Jeff King <peff@peff.net>
---
 t/t0000-basic.sh | 25 +++++++++++++++++--------
 1 file changed, 17 insertions(+), 8 deletions(-)

diff --git a/t/t0000-basic.sh b/t/t0000-basic.sh
index 3d36a87610..502375bdf6 100755
--- a/t/t0000-basic.sh
+++ b/t/t0000-basic.sh
@@ -852,16 +852,25 @@ test_expect_success 'lazy prereqs do not turn off tracing' "
 	grep 'echo trace' lazy-prereq-and-tracing/err
 "
 
-clean=no
 test_expect_success 'tests clean up after themselves' '
-	test_when_finished clean=yes
-'
+	run_sub_test_lib_test cleanup "test with cleanup" <<-\EOF &&
+	clean=no
+	test_expect_success "do cleanup" "
+		test_when_finished clean=yes
+	"
+	test_expect_success "cleanup happened" "
+		test $clean = yes
+	"
+	test_done
+	EOF
 
-if test -z "$GIT_TEST_FAIL_PREREQS_INTERNAL" -a $clean != yes
-then
-	say "bug in test framework: basic cleanup command does not work reliably"
-	exit 1
-fi
+	check_sub_test_lib_test cleanup <<-\EOF
+	ok 1 - do cleanup
+	ok 2 - cleanup happened
+	# passed all 2 test(s)
+	1..2
+	EOF
+'
 
 test_expect_success 'tests clean up even on failures' "
 	run_sub_test_lib_test_err \
-- 
2.29.2.730.g3e418f96ba


  parent reply	other threads:[~2020-11-20  0:24 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         ` [PATCH 2/4] t0000: run prereq tests inside sub-test Jeff King
2020-11-20  0:22         ` Jeff King [this message]
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=20201120002247.GC307112@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