From: Junio C Hamano <gitster@pobox.com>
To: Jeff King <peff@peff.net>
Cc: Johannes Schindelin <Johannes.Schindelin@gmx.de>,
Johannes Schindelin via GitGitGadget <gitgitgadget@gmail.com>,
git@vger.kernel.org
Subject: Re: [PATCH v2 3/5] tests: turn GPG, GPGSM and RFC1991 into lazy prereqs
Date: Fri, 27 Mar 2020 10:44:27 -0700 [thread overview]
Message-ID: <xmqqr1xdhev8.fsf@gitster.c.googlers.com> (raw)
In-Reply-To: <20200327091004.GA610157@coredump.intra.peff.net> (Jeff King's message of "Fri, 27 Mar 2020 05:10:04 -0400")
Jeff King <peff@peff.net> writes:
> So my perspective was the opposite of yours: "return" is the officially
> sanctioned way to exit early from a test snippet, and "exit" only
> happens to work because of the undocumented fact that lazy prereqs
> happen in a subshell. But it turns out neither was documented. :)
Good miniproject to document them, I presume. A rough draft
attached. I do not mind adding "exit 1 also works in this and that
case, but not in that other case" if the rule can be given succinct
enough, but I opted for simplicity (mostly because I couldn't come
up with such a clear rule for "exit 1").
As long as we are consciouly ensuring that "return 1" consistently
works everywhere, I didn't see much point offering multiple ways to
do the same thing.
> Yes. Part of the reason I kept mine as it was is because it matched the
> original behavior better (and I was really only using a lazy prereq
> because we didn't have a non-lazy version). But there's really no reason
> _not_ to be lazy, so as long as it isn't breaking anything I think
> that's a better way forward. (And if it is breaking something, that
> something should be fixed).
Yup, I too liked that part of Dscho's version better.
-- >8 --
Subject: [PATCH] t/README: suggest how to leave test early with failure
Over time, we added the support to our test framework to make it
easy to leave a test early with failure, but it was not clearly
documented in t/README to help developers writing new tests.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
* A tangent. 441ee35d (t/README: reformat Do, Don't, Keep in mind
lists, 2018-10-05) added these lines
Here are the "do's:"
And here are the "don'ts:"
Is the placement of the colons on these lines right? I am
tempted to chase them out of the dq pair and move them at the end
of their lines, but obviously that is outside of the scope of
this patch.
t/README | 55 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 55 insertions(+)
diff --git a/t/README b/t/README
index 369e3a9ded..08c8d00bb6 100644
--- a/t/README
+++ b/t/README
@@ -546,6 +546,61 @@ Here are the "do's:"
reports "ok" or "not ok" to the end user running the tests. Under
--verbose, they are shown to help debug the tests.
+ - Be careful when you loop
+
+ You may need to test multiple things in a loop, but the following
+ does not work correctly:
+
+ test_expect_success 'git frotz on various versions' '
+ for revision in one two three
+ do
+ echo "frotz $revision" >expect &&
+ git frotz "$revision" >actual &&
+ test_cmp expect actual
+ done &&
+ test something else
+ '
+
+ If the output from the "git frotz" command did not match what is
+ expected for 'one' and 'two', but it did for 'three', the loop
+ itself would not fail, and the test goes on happily. This is not
+ what you want.
+
+ You can work it around in two ways. You could use a separate
+ "flag" variable to carry the failed state out of the loop:
+
+ test_expect_success 'git frotz on various versions' '
+ failed=
+ for revision in one two three
+ do
+ echo "frotz $revision" >expect &&
+ git frotz "$revision" >actual &&
+ test_cmp expect actual ||
+ failed="$failed${failed:+ }$revision"
+ done &&
+ test -z "$failed" &&
+ test something else
+ '
+
+ Or you can fail the entire loop immediately when you see the
+ first failure by using "return 1" from inside the loop, like so:
+
+ test_expect_success 'git frotz on various versions' '
+ for revision in one two three
+ do
+ echo "frotz $revision" >expect &&
+ git frotz "$revision" >actual &&
+ test_cmp expect actual || return 1
+ done &&
+ test something else
+ '
+
+ Note that this is only possible in our test suite, where we
+ arrange to run your test <script> wrapped inside a helper
+ function to ensure that return values matter; in your own script
+ outside any function, this technique may not work.
+
+
And here are the "don'ts:"
- Don't exit() within a <script> part.
next prev parent reply other threads:[~2020-03-27 17:44 UTC|newest]
Thread overview: 55+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-03-23 13:09 [PATCH 0/2] Enable GPG in the Windows part of the CI/PR builds Johannes Schindelin via GitGitGadget
2020-03-23 13:09 ` [PATCH 1/2] tests(gpg): allow the gpg-agent to start on Windows Johannes Schindelin via GitGitGadget
2020-03-23 17:46 ` Junio C Hamano
2020-03-24 19:55 ` Johannes Schindelin
2020-03-24 20:59 ` Junio C Hamano
2020-03-24 22:26 ` Johannes Schindelin
2020-03-24 23:40 ` Junio C Hamano
2020-03-23 13:09 ` [PATCH 2/2] tests(gpg): increase verbosity to allow debugging Johannes Schindelin via GitGitGadget
2020-03-23 17:32 ` Jeff King
2020-03-23 18:04 ` Jeff King
2020-03-23 19:21 ` Junio C Hamano
2020-03-23 20:15 ` Jeff King
2020-03-23 21:28 ` Junio C Hamano
2020-03-23 21:31 ` Jeff King
2020-03-24 21:41 ` Johannes Schindelin
2020-03-24 22:05 ` Jeff King
2020-03-24 22:25 ` Johannes Schindelin
2020-03-24 22:33 ` Jeff King
2020-03-25 5:41 ` [PATCH v2 0/5] Enable GPG in the Windows part of the CI/PR builds Johannes Schindelin via GitGitGadget
2020-03-25 5:41 ` [PATCH v2 1/5] tests(gpg): allow the gpg-agent to start on Windows Johannes Schindelin via GitGitGadget
2020-03-25 5:41 ` [PATCH v2 2/5] t/lib-gpg.sh: stop pretending to be a stand-alone script Johannes Schindelin via GitGitGadget
2020-03-26 8:21 ` Jeff King
2020-03-26 13:48 ` Johannes Schindelin
2020-03-26 19:31 ` Junio C Hamano
2020-03-25 5:41 ` [PATCH v2 3/5] tests: turn GPG, GPGSM and RFC1991 into lazy prereqs Johannes Schindelin via GitGitGadget
2020-03-25 17:25 ` Junio C Hamano
2020-03-26 8:35 ` Jeff King
2020-03-26 14:27 ` Johannes Schindelin
2020-03-27 9:10 ` Jeff King
2020-03-27 17:44 ` Junio C Hamano [this message]
2020-03-27 20:24 ` Eric Sunshine
2020-03-27 21:37 ` Junio C Hamano
2020-03-28 10:58 ` Jeff King
2020-03-28 10:54 ` Jeff King
2020-03-28 23:49 ` [PATCH v2] t/README: suggest how to leave test early with failure Junio C Hamano
2020-03-29 7:23 ` Eric Sunshine
2020-03-29 14:33 ` Jeff King
2020-03-30 18:39 ` [PATCH v2 3/5] tests: turn GPG, GPGSM and RFC1991 into lazy prereqs Johannes Schindelin
2020-03-31 9:34 ` Jeff King
2020-03-25 5:41 ` [PATCH v2 4/5] tests: do not let lazy prereqs inside `test_expect_*` turn off tracing Johannes Schindelin via GitGitGadget
2020-03-25 17:23 ` Junio C Hamano
2020-03-26 13:45 ` Johannes Schindelin
2020-03-26 8:49 ` Jeff King
2020-03-26 14:34 ` Johannes Schindelin
2020-03-25 5:41 ` [PATCH v2 5/5] tests: increase the verbosity of the GPG-related prereqs Johannes Schindelin via GitGitGadget
2020-03-26 8:50 ` Jeff King
2020-03-26 14:36 ` Johannes Schindelin
2020-03-26 15:35 ` [PATCH v3 0/5] Enable GPG in the Windows part of the CI/PR builds Johannes Schindelin via GitGitGadget
2020-03-26 15:35 ` [PATCH v3 1/5] tests(gpg): allow the gpg-agent to start on Windows Johannes Schindelin via GitGitGadget
2020-03-26 15:35 ` [PATCH v3 2/5] t/lib-gpg.sh: stop pretending to be a stand-alone script Johannes Schindelin via GitGitGadget
2020-03-26 15:35 ` [PATCH v3 3/5] tests: do not let lazy prereqs inside `test_expect_*` turn off tracing Johannes Schindelin via GitGitGadget
2020-03-26 15:35 ` [PATCH v3 4/5] tests: turn GPG, GPGSM and RFC1991 into lazy prereqs Johannes Schindelin via GitGitGadget
2020-03-26 15:35 ` [PATCH v3 5/5] tests: increase the verbosity of the GPG-related prereqs Johannes Schindelin via GitGitGadget
2020-03-27 9:12 ` [PATCH v3 0/5] Enable GPG in the Windows part of the CI/PR builds Jeff King
2020-03-27 17:45 ` 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=xmqqr1xdhev8.fsf@gitster.c.googlers.com \
--to=gitster@pobox.com \
--cc=Johannes.Schindelin@gmx.de \
--cc=git@vger.kernel.org \
--cc=gitgitgadget@gmail.com \
--cc=peff@peff.net \
/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).