git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [PATCH 1/2] tests: make sure nested lazy prereqs work reliably

From
Jeff King <peff@peff.net>
Date
Nov 19, 2020, 17:56 UTC
Message-ID
<20201119175608.GA132922@coredump.intra.peff.net>
In-Reply-To
<20201119155824.GB25426@coredump.intra.peff.net>
On Thu, Nov 19, 2020 at 10:58:24AM -0500, Jeff King wrote:
Show 12 quoted lines
> > +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
Previous: Jeff KingNext: Junio C Hamano
Message 5 of 13 in “tests: make sure nested lazy prereqs work reliably”
  1. 1/2 tests: make sure nested lazy prereqs work reliablySZEDER Gábor, Nov 18, 2020
  2. 2/2 tests: fix description of 'test_set_prereq'SZEDER Gábor, Nov 18, 2020
  3. Junio C HamanoNov 18, 2020
  4. Jeff KingNov 19, 2020
  5. Jeff KingNov 19, 2020
  6. Junio C HamanoNov 19, 2020
  7. Jeff KingNov 20, 2020
  8. 1/4 t0000: keep clean-up tests togetherJeff King, Nov 20, 2020
  9. 2/4 t0000: run prereq tests inside sub-testJeff King, Nov 20, 2020
  10. 3/4 t0000: run cleaning test inside sub-testJeff King, Nov 20, 2020
  11. 4/4 t0000: consistently use single quotes for outer testsJeff King, Nov 20, 2020
  12. Junio C HamanoNov 20, 2020
  13. Junio C HamanoNov 20, 2020

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.