On Fri, Mar 20, 2015 at 10:04:43AM -0700, Junio C Hamano wrote:
> One case where this might misdetect a good test would be this one:
>
> test_expect_success 'either succeed or fail with status 1' '
> git subcmd || case "$?" in 1) : happy ;; *) false failure ;; esac
> '
Yes. Any use of "||" is going to cause problems. See the conversions in
the series I posted a few hours ago. The solution is to adapt the test
style to encase any "||" inside a block. I don't like having to modify
the tests to match our lint check, but it is a fairly rare case, and the
modification is not complex. And in fact I think it often highlights to
the reader that something funny is going on; a snippet like the one
above is OK to run at the start of a test, but:
foo &&
bar ||
case "$?"
...
is potentially dangerous. You do not know if you are checking the $? of
"foo" or "bar" in that case statement.
Your case above is actually better spelled as test_expect_code, but
there are more complex one-off cases that I solved using a {} block.
> I wonder if another valid way to make it harder for us to commit
> "broken && chain" errors in our test may be to make it not an error
> in the first place. Do we know how buggy various implementations of
> shells are with respect to their handling of "set -e"?
Certainly that is a thought that occurred to me while writing the
series. At GitHub, we have a shell-based test harness for internal
projects that is somewhat like Git's t/test-lib.sh, but uses "-e" to
catch errors.
It's mostly OK, but there are some surprising bits. For instance, try
this sequence of snippets:
set -e
false
echo should not reach
That should print nothing. So far so good. How about in a subshell:
set -e
(
false
echo 1
)
echo 2
That also prints nothing. Good. But "set -e" is suppressed in a
conditional or in the presence of logic operators like "||". So you'd
expect this:
set -e
(
false
echo 1
) || {
echo outcome=$?
false
}
echo 2
to break out of the subshell, print the failed outcome, and then exit.
But it doesn't. It prints "1" and "2". The suppression of "set -e"
continues inside the subshell, even though the inner commands are not
part of their own conditionals.
OK, so when we open a subshell we have to re-iterate our desire for "set
-e":
set -e
(
set -e
false
echo 1
) || {
echo outcome=$?
false
}
echo 2
Nope, does nothing, at least in bash and dash. The ||-operator
suppression is clearly not "turn off set -e", but "globally make -e
useless inside here".
So you end up having to use manual exits to jump out of the subshell,
like:
set -e
(
false || exit 1
echo 1
) || {
echo outcome=$?
false
}
echo 2
That snippet produces "outcome=1", which is what I expect.
Of course most tests are not so complicated to have subshells with
conditional operators on the side. But I did not just type out that
example right now from scratch. I cut-and-pasted it from a real-world
commit message.
So I dunno. I think "set -e" is kind of a dangerous lure. It works so
well _most_ of the time that you start to rely on it, but it really does
have some funny corner cases (even on modern shells, and for all I know,
the behavior above is mandated by POSIX).
-Peff
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html