Re: [PATCH v2 4/8] tests: Fail immediately when grub-shell fails and do not occlude the error code

[Glenn Washburn]

On Wed, 6 Oct 2021 15:57:24 +0200
Daniel Kiper <dkiper@net-space.pl> wrote:

> On Wed, Aug 25, 2021 at 02:03:58AM -0500, Glenn Washburn wrote:
> > If grub-shell fails, that means that whatever was being tested was not
> > actually tested. So fail immediately. Sometimes grub-shell is not the last
> > command in a pipeline of several commands, and in this case the failed error
> > code can be hidden by a later failing command or hidden when 'set -e' is not
> > set and there are subsequent successful commands. When the test script fails
> > because of a failure in grub-shell, then test script should exit with the
> > failed exit code of grub-shell.
> >
> > Signed-off-by: Glenn Washburn <development@efficientek.com>
> > ---
> >  tests/ahci_test.in             | 6 +++++-
> >  tests/cdboot_test.in           | 3 ++-
> >  tests/core_compress_test.in    | 6 ++++--
> >  tests/ehci_test.in             | 6 +++++-
> >  tests/fddboot_test.in          | 3 ++-
> >  tests/grub_cmd_date.in         | 3 ++-
> >  tests/grub_cmd_test.in         | 1 +
> >  tests/grub_script_blockarg.in  | 2 +-
> >  tests/grub_script_expansion.in | 3 ++-
> >  tests/gzcompress_test.in       | 3 ++-
> >  tests/hddboot_test.in          | 3 ++-
> >  tests/lzocompress_test.in      | 3 ++-
> >  tests/netboot_test.in          | 3 ++-
> >  tests/ohci_test.in             | 6 +++++-
> >  tests/partmap_test.in          | 4 ++--
> >  tests/pata_test.in             | 3 ++-
> >  tests/test_sha512sum.in        | 1 +
> >  tests/uhci_test.in             | 6 +++++-
> >  tests/xzcompress_test.in       | 3 ++-
> >  19 files changed, 49 insertions(+), 19 deletions(-)
> >
> > diff --git a/tests/ahci_test.in b/tests/ahci_test.in
> > index d844fe680..30dc9d31a 100644
> > --- a/tests/ahci_test.in
> > +++ b/tests/ahci_test.in
> > @@ -41,7 +41,11 @@ echo "hello" > "$outfile"
> >
> >  tar cf "$imgfile" "$outfile"
> >
> > -if [ "$(echo "nativedisk; source '(ahci0)/$outfile';" | "${grubshell}" 
> > --qemu-opts="-drive id=disk,file=$imgfile,if=none -device ahci,id=ahci 
> > -device ide-hd,drive=disk,bus=ahci.0 " | tail -n 1)" != "Hello World" ]; 
> > then
> > +echo "nativedisk; source '(ahci0)/$outfile';" |
> > +    "${grubshell}" --qemu-opts="-drive id=disk,file=$imgfile,if=none
> > +                           -device ahci,id=ahci
> > +                           -device ide-hd,drive=disk,bus=ahci.0" >$outfile
> > +if [ "$(cat "$outfile" | tail -n 1)" != "Hello World" ]; then
> 
> This change is in line with the commit message...
> 
> >     rm "$imgfile"
> >     rm "$outfile"
> >     exit 1
> > diff --git a/tests/cdboot_test.in b/tests/cdboot_test.in
> > index 75acdfedb..7229f79fb 100644
> > --- a/tests/cdboot_test.in
> > +++ b/tests/cdboot_test.in
> > @@ -34,6 +34,7 @@ case 
> > "${grub_modinfo_target_cpu}-${grub_modinfo_platform}" in
> >     exit 0;;
> >  esac
> >
> > -if [ "$(echo hello | "${grubshell}" --boot=cd)" != "Hello World" ]; then
> > +v=`echo hello | "${grubshell}" --boot=cd`
> > +if [ "$v" != "Hello World" ]; then
> 
> ... but this one is not. The grub-shell call is last one here. Hmmm...
> Am I missing something?

This is the case where the error code is hidden, so 'set -e' doesn't
fail as it should.

Note how this will not cause the script to error:

$ bash -e -c 'if [ "$(echo XXX | ( cat; false ))" == "XXX" ]; then echo
`echo $$`; fi'

But this will, which is what we want.

$ bash -e -c 'v=`echo XXX | ( cat; false )`; if [ "$v" == "XXX" ]; then
echo `echo $$`; fi'

So yes, this case, where error code is occluded with 'set -e', is not
described in the commit message. Should I update the commit message?

> 
> >     exit 1
> >  fi
> > diff --git a/tests/core_compress_test.in b/tests/core_compress_test.in
> > index 9d216ebcf..90dd00607 100644
> > --- a/tests/core_compress_test.in
> > +++ b/tests/core_compress_test.in
> > @@ -27,10 +27,12 @@ case 
> > "${grub_modinfo_target_cpu}-${grub_modinfo_platform}" in
> >  esac
> >
> >
> > -if [ "$(echo hello | "${grubshell}" --grub-mkimage-extra=--compress=xz)" 
> > != "Hello World" ]; then
> > +v=`echo hello | "${grubshell}" --grub-mkimage-extra=--compress=xz`
> > +if [ "$v" != "Hello World" ]; then
> 
> Ditto... And a few similar things below...
> 
> Daniel

Glenn