“! failing-command” pattern in shell tests is wrong

  • Done
  • quality assurance status badge
Details
3 participants
  • Eric Bavier
  • Martin Castillo
  • Ludovic Courtès
Owner
unassigned
Submitted by
Ludovic Courtès
Severity
important
L
L
Ludovic Courtès wrote on 23 Mar 2023 17:00
“! failing-command” pattern in shell tests is wrong
(address . bug-guix@gnu.org)
87y1nn790x.fsf@inria.fr
d8934360d2453a403b5433e71d09188e4ed23b57), we changed:

if command that should fail; then false; else true; fi

to:

! command that should fail

I had reservations back then, and now I know why: :-)

Toggle snippet (12 lines)
$ bash -xe -c '! true; true'
+ true
+ true
$ echo $?
0
$ bash -xe -c '! false; true'
+ false
+ true
$ echo $?
0

Whether or not the command following the exclamation mark succeeds, the
statement succeeds. Bummer.

The Bash manual (info "(bash) Pipelines") reads:

If the reserved word '!' precedes the pipeline, the exit status is the
logical negation of the exit status as described above. The shell
waits for all commands in the pipeline to terminate before returning a
value.

To me, that means it should work as we thought, but it’s a fact that it
doesn’t.

Thoughts?

Ludo’.
E
E
Eric Bavier wrote on 23 Mar 2023 19:53
Re: bug#62406: “! failing-command” pattern in shell tests is wrong
75622c7ab52087a4266b7b48374013d0c76d3c53.camel@posteo.net
On Thu, 2023-03-23 at 17:00 +0100, Ludovic Courtès wrote:
Toggle quote (27 lines)
> d8934360d2453a403b5433e71d09188e4ed23b57), we changed:
>
> if command that should fail; then false; else true; fi
>
> to:
>
> ! command that should fail
>
> I had reservations back then, and now I know why: :-)
>
> --8<---------------cut here---------------start------------->8---
> $ bash -xe -c '! true; true'
> + true
> + true
> $ echo $?
> 0
> $ bash -xe -c '! false; true'
> + false
> + true
> $ echo $?
> 0
> --8<---------------cut here---------------end--------------->8---
>
> Whether or not the command following the exclamation mark succeeds, the
> statement succeeds. Bummer.

I think it's maybe not that the statement succeeds regardless. But that 'set
-e' doesn't consider it a "failure". From "The Set Builtin":

'-e'
Exit immediately if a pipeline (*note Pipelines::)... returns a
non-zero status. The shell does not exit if the command that
fails is ..., or if the command's return status is being
inverted with '!'.

So in each of your examples, execution continues to the second 'true'
statement and the overall exit status is 0. This is not the behavior we want
in our tests.

The purpose of d89343 was to ease visual parsing of the tests. I mentioned
having used the '!' syntax in my own shell tests, but I realize now that I
was not relying on `set -e` like guix is.

I'll consider a few options. Do we have a known issue where this is causing
a test to not to catch a failure?

Thanks for bringing this up,
`~Eric
-----BEGIN PGP SIGNATURE-----

iQJGBAABCgAwFiEEo6S0GQB0CHyn3laYvEXKZ+L40AcFAmQcoDsSHGJhdmllckBw
b3N0ZW8ubmV0AAoJELxFymfi+NAH2CMP/iEDMDJBkmTHzQ7IZuMvUHTuY3bSRrKY
ux0xPoeCN+jsRr/11qhsIJnvcTAjXzfM8n+7/a5cRHVJZqrB5yN7PNvfs5jXu53l
wzT4fx7LrfzttEbdubT/7oARGsCA528DC1tcP2HZRTFpichMwxRlQlUJuRZd74E8
3I3XTXApfh+FQBShkNRXU22BkMv2csv3qaXkkJIifnYM9BtvjB7Z7ATuNh8JLJXW
qg/Yu0xZ2us9ya8/0SEPJ9PvWJ3bMCVpEVPSxFPdeupAqTXjKI5F6BSoJsXaRKC0
I3YCRXS+ab9l8Un31tdJz3Vu6NbsLQ31pHqHe3/ipUb6crTxxfV+myLI0hHcNKQB
F1YCMSoBuz+hmSHxIYDLGEZHF2xENbz9a434pCriJoV3GS+FxxyqYLHKPgvhe94w
SgLupMfKpZlj6P9+dHs0YdS1nAjZ7K5/rGYa1ecTh5ghw/hMJO7sLCVq442OjSTn
+5xm74fwsDqDRigi7psyjvdKM3GuhPcHZ9oTzs5ShiwzHFhps55JV4DS1xMx2n9F
iDG57EtwtGOHDGo6D/mrJ21RMTPe8Elyd7z4bWb633e0KjElQSOFTSSnJMpZbs3N
j5KKSnw8tXk2bSk8oNjmCf6Hcf7sksfOzP/vbkJxbQpXlSdNOo5KZOfYdXOfFkTr
mZsPoPM29Hv8
=g1CF
-----END PGP SIGNATURE-----


L
L
Ludovic Courtès wrote on 26 Mar 2023 15:36
control message for bug #62406
(address . control@debbugs.gnu.org)
87ilend48l.fsf@gnu.org
severity 62406 important
quit
M
M
Martin Castillo wrote on 26 Mar 2023 20:16
Re: bug#62406: “! failing-command” pat tern in shell tests is wrong
(name . Eric Bavier)(address . bavier@posteo.net)
7d5754f3-86e8-4be5-ccb6-689d4d29a8d7@uni-bremen.de
Hi,

Am 23.03.23 um 17:00 schrieb Ludovic Courtès:> In
> d8934360d2453a403b5433e71d09188e4ed23b57), we changed:
>
> if command that should fail; then false; else true; fi
>
> to:
>
> ! command that should fail
>
> I had reservations back then, and now I know why: :-)
>
> --8<---------------cut here---------------start------------->8---
> $ bash -xe -c '! true; true'
> + true
> + true
> $ echo $?
> 0
> $ bash -xe -c '! false; true'
> + false
> + true
> $ echo $?
> 0
> --8<---------------cut here---------------end--------------->8---
>
> Whether or not the command following the exclamation mark succeeds, the
> statement succeeds. Bummer.
>
> The Bash manual (info "(bash) Pipelines") reads:
>
> If the reserved word '!' precedes the pipeline, the exit status is the
> logical negation of the exit status as described above. The shell
> waits for all commands in the pipeline to terminate before returning a
> value.
>
> To me, that means it should work as we thought, but it’s a fact that it
> doesn’t.
the documentation on `-e` says:

-e Exit immediately if a pipeline (which may consist of a
single simple command), a list, or a compound command
(see SHELL GRAMMAR above), exits with a non-zero status.
The shell does not exit [...] if the
command's return value is being inverted with !.

I have no idea what might be the rationale for that.
M
M
Martin Castillo wrote on 26 Mar 2023 20:21
(name . Eric Bavier)(address . bavier@posteo.net)
10f5409d-051b-a74b-760a-9c195edf5ca0@uni-bremen.de
Sorry for my answer. I overlooked that Eric already answered.
L
L
Ludovic Courtès wrote on 28 Mar 2023 18:21
Re: bug#62406: “! failing-command” pattern in shell tests is wrong
(name . Eric Bavier)(address . bavier@posteo.net)(address . 62406@debbugs.gnu.org)
87a5zwg83r.fsf@gnu.org
Hi Eric,

Eric Bavier <bavier@posteo.net> skribis:

Toggle quote (6 lines)
> The purpose of d89343 was to ease visual parsing of the tests. I mentioned
> having used the '!' syntax in my own shell tests, but I realize now that I
> was not relying on `set -e` like guix is.
>
> I'll consider a few options.

Neat. I guess we could have a ‘lib.sh’ with an ‘expect_fail’ function
or something.

Toggle quote (3 lines)
> Do we have a known issue where this is causing a test to not to catch
> a failure?

No; I noticed it while writing a new test that I expected to fail.

Thanks for your feedback! Shell semantics are definitely weird. :-)

Ludo’.
E
E
Eric Bavier wrote on 20 Apr 2023 07:48
Re: bug#62406: “! failing-command” pattern in shell tests is wrong
(name . Ludovic Courtès)(address . ludo@gnu.org)(address . 62406@debbugs.gnu.org)
2fe1f69bc1db71278aebc6dce34d3c1c01f87a39.camel@posteo.net
On Tue, 2023-03-28 at 18:21 +0200, Ludovic Courtès wrote:
Toggle quote (14 lines)
> Hi Eric,
>
> Eric Bavier <bavier@posteo.net> skribis:
>
> > The purpose of d89343 was to ease visual parsing of the tests. I mentioned
> > having used the '!' syntax in my own shell tests, but I realize now that I
> > was not relying on `set -e` like guix is.
> >
> > I'll consider a few options.
>
> Neat. I guess we could have a ‘lib.sh’ with an ‘expect_fail’ function
> or something.
>

Instead of a shared 'lib.sh', the attached patch uses 'cmd && false', which
has the desired semantics under 'set -e' and is no more verbose than a
wrapping function call.

If 'cmd' fails, the return status is ignored by 'set -e', which considers
only the return status of a command following the final '&&' or '||'. And
because 'cmd' failed the statement short-circuits without executing the
'false. Otherwise, if 'cmd' succeeds, the 'false' is executed and the shell
exits immediately.

In other places the '! test ...' pattern is replaced with 'test ! ...'.

There was some small amount of fall-out. I fixed a couple issues where I
could:

- tests/guix-archive.sh: added '--export' to command

- tests/guix-style.sh: added an escape to a sed pattern

But a couple others have failures I'm not as confident in fixing myself:

- tests/guix-refresh.sh: 'guix refresh' seems to not exit with a failure
status if a warning is issued, but the tests seem to think it should.

- tests/guix-git-authenticate.sh: A general failure to authenticate a
particular commit. This could be an issue with my test environment.

This patch should probably not be applied until those tests are fixed. I
would appreciate any help with that.

`~Eric
Toggle quote (1 lines)
>
L
L
Ludovic Courtès wrote on 15 Jun 2023 22:54
control message for bug #62406
(address . control@debbugs.gnu.org)
87mt10v444.fsf@gnu.org
close 62406
quit
?