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 [thread overview]
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
next prev parent reply other threads:[~2020-11-20 0:22 UTC|newest]
Thread overview: 14+ 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
2021-01-28 6:32 [PATCH 0/4] t0000 cleanups Jeff King
2021-01-28 6:32 ` [PATCH 2/4] t0000: run prereq tests inside sub-test Jeff King
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
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
Code repositories for project(s) associated with this public inbox
https://80x24.org/mirrors/git.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).