Re: [PATCH v2 3/5] tests: turn GPG, GPGSM and RFC1991 into lazy prereqs

From

Junio C Hamano <gitster@pobox.com>

Date

Mar 27, 2020, 17:44 UTC

Message-ID

<xmqqr1xdhev8.fsf@gitster.c.googlers.com>

In-Reply-To

<20200327091004.GA610157@coredump.intra.peff.net>

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. :)

> 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

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.