[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v3 2/4] tests: Do not occlude subshell error codes
From: |
Daniel Kiper |
Subject: |
Re: [PATCH v3 2/4] tests: Do not occlude subshell error codes |
Date: |
Mon, 11 Oct 2021 16:20:46 +0200 |
User-agent: |
NeoMutt/20170113 (1.7.2) |
On Thu, Oct 07, 2021 at 03:33:25PM -0500, Glenn Washburn wrote:
> When a subshell's output is used as input to a "simple command", its return
> code is not checked. These subshells contain an execution of the grub-shell
> script which does the work of the actual test. If grub-shell returns an
> error code, the test should fail. So refactor to not have the subshell which
> contains grub-shell be direct input into a simple command (usually the test
> command). Mostly this is accomplished by having the output first go to a
> shell variable, and then using the shell variable in the simple command.
>
> Signed-off-by: Glenn Washburn <development@efficientek.com>
> ---
> tests/ahci_test.in | 7 ++++++-
> tests/cdboot_test.in | 3 ++-
> tests/core_compress_test.in | 6 ++++--
> tests/ehci_test.in | 7 ++++++-
> tests/fddboot_test.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 | 7 ++++++-
> tests/pata_test.in | 3 ++-
> tests/uhci_test.in | 7 ++++++-
> tests/xzcompress_test.in | 3 ++-
> 13 files changed, 44 insertions(+), 14 deletions(-)
>
> diff --git a/tests/ahci_test.in b/tests/ahci_test.in
> index 9b1d85df4..1e4e3e443 100644
> --- a/tests/ahci_test.in
> +++ b/tests/ahci_test.in
> @@ -41,7 +41,12 @@ 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
> +v=$(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)
The first change partially contradicts what you said in the commit
message. I had to take a look at the patch #3 to understand why you did
not drop "tail -n 1". I think the commit message should be clarified in
a such way which does not require referring to another patch to
understand this one.
> +if [ "$v" != "Hello World" ]; then
> 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`
I would prefer if you are more consistent and use "$(...)" instead of "`...`".
The former looks more common in these scripts.
Daniel
[PATCH v3 3/4] tests: Do not occlude grub-shell return code, Glenn Washburn, 2021/10/07
[PATCH v3 4/4] tests: Exit with skipped exit code when test not performed, Glenn Washburn, 2021/10/07