Jeff King <peff@xxxxxxxx> 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@xxxxxxxxx>
---
* 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.
