On Wed, Oct 06, 2021 at 03:05:57PM -0500, Glenn Washburn wrote:
> On Wed, 6 Oct 2021 15:57:24 +0200
> Daniel Kiper <dki...@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 <developm...@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'
OK, AIUI error code is occluded by '['/test. Right?
> 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?
I think this patch should be split into two because these issues are two
different things.
Daniel
_______________________________________________
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
