[PATCH v2 0/8] Various fixes/improvements for tests

[PATCH v2 0/8] Various fixes/improvements for tests

[Glenn Washburn]

This is an update to the first version of this patch series to account for
changes in tests/ahci_test.in which cause a conflict in the previous patch
series. Nothing of substance has changed which would invalidate the previous
cover letter, to which I'll refer for comments on the patches.

Glenn

Glenn Washburn (8):
  tests: Make sure LANG is set properly for iso9660_test
  tests: Fix partmap_test for arm*-efi, disk numbering has changed
  tests: When checking squashfs fstime, use superblock last modified
    time
  tests: Fail immediately when grub-shell fails and do not occlude the
    error code
  tests: Make setup errors in grub-fs-tester hard errors
  tests: A failure of mktemp should cause the test script to exit with
    code 99
  tests: Exit with skipped exit code when test not performed
  tests: Use @BUILD_SHEBANG@ autoconf var instead of literal shell

 tests/ahci_test.in             | 18 +++++++++++-------
 tests/cdboot_test.in           | 11 ++++++-----
 tests/core_compress_test.in    |  8 +++++---
 tests/ehci_test.in             | 18 +++++++++++-------
 tests/f2fs_test.in             |  2 +-
 tests/fddboot_test.in          | 19 ++++++++++---------
 tests/gettext_strings_test.in  |  2 +-
 tests/grub_cmd_date.in         |  5 +++--
 tests/grub_cmd_set_date.in     |  6 +++---
 tests/grub_cmd_test.in         |  7 ++++---
 tests/grub_script_blockarg.in  |  4 ++--
 tests/grub_script_expansion.in |  3 ++-
 tests/gzcompress_test.in       |  3 ++-
 tests/hddboot_test.in          |  9 +++++----
 tests/iso9660_test.in          |  6 ++++++
 tests/lzocompress_test.in      |  3 ++-
 tests/netboot_test.in          | 15 ++++++++-------
 tests/ohci_test.in             | 18 +++++++++++-------
 tests/partmap_test.in          | 18 +++++++++---------
 tests/pata_test.in             | 13 +++++++------
 tests/pseries_test.in          |  2 +-
 tests/syslinux_test.in         |  2 +-
 tests/test_sha512sum.in        |  7 ++++---
 tests/uhci_test.in             | 18 +++++++++++-------
 tests/util/grub-fs-tester.in   | 17 ++++++++++++-----
 tests/xzcompress_test.in       |  3 ++-
 26 files changed, 140 insertions(+), 97 deletions(-)

Range-diff against v1:
1:  c1ff77eec = 1:  17ddccb2e tests: Make sure LANG is set properly for iso9660_test
2:  eb4bf8cb6 = 2:  62ed781c8 tests: Fix partmap_test for arm*-efi, disk numbering has changed
3:  b00c1df00 = 3:  066486e22 tests: When checking squashfs fstime, use superblock last modified time
4:  73f81a5b4 ! 4:  562d74a49 tests: Fail immediately when grub-shell fails and do not occlude the error code
    @@ tests/ahci_test.in: 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-drive,drive=disk,bus=ahci.0 " | tail -n 1)" != "Hello World" ]; then
    +-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-drive,drive=disk,bus=ahci.0" >$outfile
    ++				-device ide-hd,drive=disk,bus=ahci.0" >$outfile
     +if [ "$(cat "$outfile" | tail -n 1)" != "Hello World" ]; then
         rm "$imgfile"
         rm "$outfile"
5:  946730f71 = 5:  af85ae97f tests: Make setup errors in grub-fs-tester hard errors
6:  0e49be59e = 6:  fcb2b7a06 tests: A failure of mktemp should cause the test script to exit with code 99
7:  f08c84e58 = 7:  47afc3569 tests: Exit with skipped exit code when test not performed
8:  dca75ed56 = 8:  40f77d41b tests: Use @BUILD_SHEBANG@ autoconf var instead of literal shell
-- 
2.27.0

[PATCH v2 1/8] tests: Make sure LANG is set properly for iso9660_test

[Glenn Washburn]

LANG must be set to something that supports international characters,
otherwise xorriso will refuse to include the file with name having
international characters, causing the test to fail. So if LANG is not set,
set it to en_US.UTF-8, a very common UTF-8 locale. And if it is set, but
does not look like a UTF-8 locale, print a warning so the user will have a
clue as to why the iso9660_test might be failing.

Signed-off-by: Glenn Washburn <development@efficientek.com>
---
 tests/iso9660_test.in | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/tests/iso9660_test.in b/tests/iso9660_test.in
index 571b938d7..37b1d30af 100644
--- a/tests/iso9660_test.in
+++ b/tests/iso9660_test.in
@@ -7,6 +7,12 @@ if ! which xorriso >/dev/null 2>&1; then
    exit 77
 fi
 
+if [ -z "$LANG" ]; then
+   export LANG=en_US.UTF-8
+elif [ -n "${LANG##*UTF*}" ]; then
+   echo "WARNING: LANG=$LANG appears to not be unicode, international file test may fail."
+fi
+
 "@builddir@/grub-fs-tester" joliet
 "@builddir@/grub-fs-tester" rockridge
 "@builddir@/grub-fs-tester" rockridge_joliet
-- 
2.27.0

[PATCH v2 2/8] tests: Fix partmap_test for arm*-efi, disk numbering has changed

[Glenn Washburn]

Perhaps using a newer UEFI firmware is the reason for the created test disk
showing up as hd2 instead of hd3.

Signed-off-by: Glenn Washburn <development@efficientek.com>
---
 tests/partmap_test.in | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/partmap_test.in b/tests/partmap_test.in
index 6ef518b0a..7353dc70e 100644
--- a/tests/partmap_test.in
+++ b/tests/partmap_test.in
@@ -89,7 +89,7 @@ case "${grub_modinfo_target_cpu}-${grub_modinfo_platform}" in
 	disk=arc/scsi0/disk0/rdisk0
 	;;
     arm*-efi)
-	disk=hd3
+	disk=hd2
 	;;
     *)
 	disk=hd0
-- 
2.27.0

[PATCH v2 3/8] tests: When checking squashfs fstime, use superblock last modified time

[Glenn Washburn]

Currently, the filesystem timestamp check in grub-fs-tester uses the
squashfs image file's last modified timestamp and checks to see if that
time stamp is within 3 seconds of the superblock timestamp as determined by
grub. The image file's timestamp could be more than 3 seconds off if
mksquashfs takes more than 3 seconds to generate the image, as is the case
on a virtual machine. Instead use squashfs tools to get the filesystem
timestamp directly.

Signed-off-by: Glenn Washburn <development@efficientek.com>
---
 tests/util/grub-fs-tester.in | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/tests/util/grub-fs-tester.in b/tests/util/grub-fs-tester.in
index bfc425e1f..4213b7bfc 100644
--- a/tests/util/grub-fs-tester.in
+++ b/tests/util/grub-fs-tester.in
@@ -1351,6 +1351,12 @@ for LOGSECSIZE in $(range "$MINLOGSECSIZE" "$MAXLOGSECSIZE" 1); do
 			# With some abstractions like mdraid flushing to disk
 			# may be delayed for a long time.
 			FSTIME="$UMOUNT_TIME";;
+		    xsquash*)
+			# Creating the squash image may take more than a few
+			# seconds. Use the more accurate timestamp from the
+			# superblock.
+			FSTIME="$(unsquashfs -s "${FSIMAGEP}0.img" | grep ^Creation | awk '{print $6 " " $7 " " $8 " " $9 " " $10; }')"
+			FSTIME="$(date -d "$FSTIME" -u '+%Y-%m-%d %H:%M:%S')";;
 		    *)
 			FSTIME="$(TZ=UTC ls --time-style="+%Y-%m-%d_%H:%M:%S" -l -d "${FSIMAGEP}0.img"|awk '{print $6; }'|sed 's,_, ,g')";;
 		esac
-- 
2.27.0

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

[Glenn Washburn]

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
    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
    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
    exit 1
 fi
 
-if [ "$(echo hello | "${grubshell}" --grub-mkimage-extra=--compress=none)" != "Hello World" ]; then
+v=`echo hello | "${grubshell}" --grub-mkimage-extra=--compress=none`
+if [ "$v" != "Hello World" ]; then
    exit 1
 fi
diff --git a/tests/ehci_test.in b/tests/ehci_test.in
index b197f8cdc..64bd80070 100644
--- a/tests/ehci_test.in
+++ b/tests/ehci_test.in
@@ -41,7 +41,11 @@ echo "hello" > "$outfile"
 
 tar cf "$imgfile" "$outfile"
 
-if [ "$(echo "nativedisk; source '(usb0)/$outfile';" | "${grubshell}" --qemu-opts="-device ich9-usb-ehci1 -drive id=my_usb_disk,file=$imgfile,if=none -device usb-storage,drive=my_usb_disk" | tail -n 1)" != "Hello World" ]; then
+echo "nativedisk; source '(usb0)/$outfile';" |
+    "${grubshell}" --qemu-opts="-device ich9-usb-ehci1
+				-drive id=my_usb_disk,file=$imgfile,if=none
+				-device usb-storage,drive=my_usb_disk" >$outfile
+if [ "$(cat "$outfile" | tail -n 1)" != "Hello World" ]; then
    rm "$imgfile"
    rm "$outfile"
    exit 1
diff --git a/tests/fddboot_test.in b/tests/fddboot_test.in
index 2d7dfc889..1bbe60ee5 100644
--- a/tests/fddboot_test.in
+++ b/tests/fddboot_test.in
@@ -46,6 +46,7 @@ case "${grub_modinfo_target_cpu}-${grub_modinfo_platform}" in
 	exit 0;;
 esac
 
-if [ "$(echo hello | "${grubshell}" --boot=fd --mkrescue-arg="--compress=xz --fonts= --locales= --themes= -no-pad")" != "Hello World" ]; then
+v=`echo hello | "${grubshell}" --boot=fd --mkrescue-arg="--compress=xz --fonts= --locales= --themes= -no-pad"`
+if [ "$v" != "Hello World" ]; then
    exit 1
 fi
diff --git a/tests/grub_cmd_date.in b/tests/grub_cmd_date.in
index f7c9ca004..f9156691e 100644
--- a/tests/grub_cmd_date.in
+++ b/tests/grub_cmd_date.in
@@ -9,7 +9,8 @@ if [ "${grub_modinfo_target_cpu}-${grub_modinfo_platform}" = sparc64-ieee1275 ];
 fi
 
 pdt="$(date -u +%s)"
-dt=`echo date | @builddir@/grub-shell | sed 's, [A-Z][a-z]*$,,'`
+dt=`echo date | @builddir@/grub-shell`
+dt=`echo "$dt" | sed 's, [A-Z][a-z]*$,,'`
 dtg="$(date -u -d "$dt" +%s)"
 ndt="$(date -u +%s)"
 
diff --git a/tests/grub_cmd_test.in b/tests/grub_cmd_test.in
index 3399eb292..dac6f7d6a 100644
--- a/tests/grub_cmd_test.in
+++ b/tests/grub_cmd_test.in
@@ -1,4 +1,5 @@
 #! @BUILD_SHEBANG@
+set -e
 
 # create a randome file
 empty="`mktemp "${TMPDIR:-/tmp}/tmp.XXXXXXXXXX"`" || exit 1
diff --git a/tests/grub_script_blockarg.in b/tests/grub_script_blockarg.in
index 6ea9b8c3d..6152cc141 100644
--- a/tests/grub_script_blockarg.in
+++ b/tests/grub_script_blockarg.in
@@ -1,4 +1,4 @@
-#! @BUILD_SHEBANG@
+#! @BUILD_SHEBANG@ -e
 
 # Run GRUB script in a Qemu instance
 # Copyright (C) 2010  Free Software Foundation, Inc.
diff --git a/tests/grub_script_expansion.in b/tests/grub_script_expansion.in
index 9d0dcdd29..98d5a9068 100644
--- a/tests/grub_script_expansion.in
+++ b/tests/grub_script_expansion.in
@@ -17,7 +17,8 @@ set -e
 # You should have received a copy of the GNU General Public License
 # along with GRUB.  If not, see <http://www.gnu.org/licenses/>.
 
-disks=`echo ls | @builddir@/grub-shell| grep -av '^Network protocols:$'| grep -av '^tftp http $'`
+disks=`echo ls | @builddir@/grub-shell`
+disks=`echo "$disks"| grep -av '^Network protocols:$'| grep -av '^tftp http $'`
 other=`echo insmod regexp\; echo \* | @builddir@/grub-shell`
 for d in $disks; do
     if echo "$d" |grep ',' >/dev/null; then
diff --git a/tests/gzcompress_test.in b/tests/gzcompress_test.in
index 42c8fe7c4..572b2a1a5 100644
--- a/tests/gzcompress_test.in
+++ b/tests/gzcompress_test.in
@@ -24,6 +24,7 @@ if ! which gzip >/dev/null 2>&1; then
    exit 77
 fi
 
-if [ "$(echo hello | "${grubshell}" --mkrescue-arg=--compress=gz)" != "Hello World" ]; then
+v=`echo hello | "${grubshell}" --mkrescue-arg=--compress=gz`
+if [ "$v" != "Hello World" ]; then
    exit 1
 fi
diff --git a/tests/hddboot_test.in b/tests/hddboot_test.in
index 6d70847a5..abe3e0ae1 100644
--- a/tests/hddboot_test.in
+++ b/tests/hddboot_test.in
@@ -31,7 +31,8 @@ case "${grub_modinfo_target_cpu}-${grub_modinfo_platform}" in
 	exit 0;;
 esac
 
-if [ "$(echo hello | "${grubshell}" --boot=hd)" != "Hello World" ]; then
+v=`echo hello | "${grubshell}" --boot=hd`
+if [ "$v" != "Hello World" ]; then
    exit 1
 fi
 
diff --git a/tests/lzocompress_test.in b/tests/lzocompress_test.in
index 4e5f7e078..8f08a8046 100644
--- a/tests/lzocompress_test.in
+++ b/tests/lzocompress_test.in
@@ -24,6 +24,7 @@ if ! which lzop >/dev/null 2>&1; then
    exit 77
 fi
 
-if [ "$(echo hello | "${grubshell}" --mkrescue-arg=--compress=lzo)" != "Hello World" ]; then
+v=`echo hello | "${grubshell}" --mkrescue-arg=--compress=lzo`
+if [ "$v" != "Hello World" ]; then
    exit 1
 fi
diff --git a/tests/netboot_test.in b/tests/netboot_test.in
index 9f71e3d88..6a1d95a22 100644
--- a/tests/netboot_test.in
+++ b/tests/netboot_test.in
@@ -40,6 +40,7 @@ case "${grub_modinfo_target_cpu}-${grub_modinfo_platform}" in
 	exit 0;;
 esac
 
-if [ "$(echo hello | "${grubshell}" --boot=net)" != "Hello World" ]; then
+v=`echo hello | "${grubshell}" --boot=net`
+if [ "$v" != "Hello World" ]; then
    exit 1
 fi
diff --git a/tests/ohci_test.in b/tests/ohci_test.in
index 8693f8c47..cc35f67a9 100644
--- a/tests/ohci_test.in
+++ b/tests/ohci_test.in
@@ -41,7 +41,11 @@ echo "hello" > "$outfile"
 
 tar cf "$imgfile" "$outfile"
 
-if [ "$(echo "nativedisk; source '(usb0)/$outfile';" | "${grubshell}" --qemu-opts="-device pci-ohci -drive id=my_usb_disk,file=$imgfile,if=none -device usb-storage,drive=my_usb_disk" | tail -n 1)" != "Hello World" ]; then
+echo "nativedisk; source '(usb0)/$outfile';" |
+    "${grubshell}" --qemu-opts="-device pci-ohci
+				-drive id=my_usb_disk,file=$imgfile,if=none
+				-device usb-storage,drive=my_usb_disk" >$outfile
+if [ "$(cat "$outfile" | tail -n 1)" != "Hello World" ]; then
    rm "$imgfile"
    rm "$outfile"
    exit 1
diff --git a/tests/partmap_test.in b/tests/partmap_test.in
index 7353dc70e..5f18ab51c 100644
--- a/tests/partmap_test.in
+++ b/tests/partmap_test.in
@@ -58,8 +58,8 @@ list_parts () {
     shift
 
     echo ls | "${grubshell}" --disk="${imgfile}" \
-	--modules=$mod | tr -d "\n\r" > "${outfile}"
-    cat "${outfile}"
+	--modules=$mod > "${outfile}"
+    cat "${outfile}" | tr -d "\n\r"
     echo
 }
 
diff --git a/tests/pata_test.in b/tests/pata_test.in
index 4b18fdef3..3d19cecde 100644
--- a/tests/pata_test.in
+++ b/tests/pata_test.in
@@ -45,7 +45,8 @@ echo "hello" > "$outfile"
 
 tar cf "$imgfile" "$outfile"
 
-if [ "$(echo "nativedisk; source '($indisk)/$outfile';" | "${grubshell}" --qemu-opts="-$disk $imgfile")" != "Hello World" ]; then
+v=`echo "nativedisk; source '($indisk)/$outfile';" | "${grubshell}" --qemu-opts="-$disk $imgfile"`
+if [ "$v" != "Hello World" ]; then
    rm "$imgfile"
    rm "$outfile"
    exit 1
diff --git a/tests/test_sha512sum.in b/tests/test_sha512sum.in
index 027092a8b..d97b7ae2c 100644
--- a/tests/test_sha512sum.in
+++ b/tests/test_sha512sum.in
@@ -1,4 +1,5 @@
 #! @BUILD_SHEBANG@
+set -e
 
 # create a randome file
 file="`mktemp "${TMPDIR:-/tmp}/tmp.XXXXXXXXXX"`" || exit 1
diff --git a/tests/uhci_test.in b/tests/uhci_test.in
index f0eec5032..6aab10ba6 100644
--- a/tests/uhci_test.in
+++ b/tests/uhci_test.in
@@ -41,7 +41,11 @@ echo "hello" > "$outfile"
 
 tar cf "$imgfile" "$outfile"
 
-if [ "$(echo "nativedisk; source '(usb0)/$outfile';" | "${grubshell}" --qemu-opts="-device ich9-usb-uhci1 -drive id=my_usb_disk,file=$imgfile,if=none -device usb-storage,drive=my_usb_disk" | tail -n 1)" != "Hello World" ]; then
+echo "nativedisk; source '(usb0)/$outfile';" |
+    "${grubshell}" --qemu-opts="-device ich9-usb-uhci1
+				-drive id=my_usb_disk,file=$imgfile,if=none
+				-device usb-storage,drive=my_usb_disk" >$outfile
+if [ "$(cat "$outfile" | tail -n 1)" != "Hello World" ]; then
    rm "$imgfile"
    rm "$outfile"
    exit 1
diff --git a/tests/xzcompress_test.in b/tests/xzcompress_test.in
index 03bfb5e95..61acba33b 100644
--- a/tests/xzcompress_test.in
+++ b/tests/xzcompress_test.in
@@ -24,6 +24,7 @@ if ! which xz >/dev/null 2>&1; then
    exit 77
 fi
 
-if [ "$(echo hello | "${grubshell}" --mkrescue-arg=--compress=xz)" != "Hello World" ]; then
+v=`echo hello | "${grubshell}" --mkrescue-arg=--compress=xz`
+if [ "$v" != "Hello World" ]; then
    exit 1
 fi
-- 
2.27.0

[PATCH v2 5/8] tests: Make setup errors in grub-fs-tester hard errors

[Glenn Washburn]

When a test program fails because it failed to setup the test properly, this
does not indicate a failure in what is attempting to be tested because the
test is never run. So exit with a hard error exit status to note this
difference. This will allow easier detection of tests that are not actually
being run and those that are really failing.

Signed-off-by: Glenn Washburn <development@efficientek.com>
---
 tests/util/grub-fs-tester.in | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/tests/util/grub-fs-tester.in b/tests/util/grub-fs-tester.in
index 4213b7bfc..eacb1e0a7 100644
--- a/tests/util/grub-fs-tester.in
+++ b/tests/util/grub-fs-tester.in
@@ -6,7 +6,8 @@ fs="$1"
 
 GRUBFSTEST="@builddir@/grub-fstest"
 
-tempdir=`mktemp -d "${TMPDIR:-/tmp}/tmp.XXXXXXXXXX"` || exit 1
+tempdir=`mktemp -d "${TMPDIR:-/tmp}/tmp.XXXXXXXXXX"` ||
+{ echo "Failed to make temporary directory"; exit 99; }
 
 # This wrapper is to ease insertion of valgrind or time statistics
 run_it () {
@@ -273,7 +274,7 @@ for LOGSECSIZE in $(range "$MINLOGSECSIZE" "$MAXLOGSECSIZE" 1); do
 	    done
 	    if test "$CFILESRC" = "" ; then
 		echo "Couldn't find compressible file" >&2
-		exit 1
+		exit 99
 	    fi
 	    case x"$fs" in
 		    # FS LIMITATION: 8.3 names
@@ -616,7 +617,7 @@ for LOGSECSIZE in $(range "$MINLOGSECSIZE" "$MAXLOGSECSIZE" 1); do
 		    mkdir -p "$MNTPOINTRO"
 		    for i in $(range 0 $((NDEVICES-1)) 1); do
 			dd if=/dev/zero of="$FSIMAGEP${i}.img" count=1 bs=1 seek=$((DISKSIZE-1)) &> /dev/null
-			LODEVICE=$(losetup --find --show "$FSIMAGEP${i}.img")
+			LODEVICE=$(losetup --find --show "$FSIMAGEP${i}.img") || exit 99
 			LODEVICES="$LODEVICES $LODEVICE"
 			if test "$i" = 0; then
 			    MOUNTDEVICE="$LODEVICE"
@@ -820,7 +821,7 @@ for LOGSECSIZE in $(range "$MINLOGSECSIZE" "$MAXLOGSECSIZE" 1); do
 		    "mkfs.xfs" -m crc=1 -b size=$BLKSIZE -s size=$SECSIZE -L "$FSLABEL" -q "${MOUNTDEVICE}" ;;
 		*)
 		    echo "Add appropriate mkfs command here"
-		    exit 1
+		    exit 99
 		    ;;
 	    esac
 	    BASEFILE="1.img"
@@ -926,7 +927,7 @@ for LOGSECSIZE in $(range "$MINLOGSECSIZE" "$MAXLOGSECSIZE" 1); do
 			for i in $(range 0 $((NDEVICES-1)) 1); do
 			    rm "$FSIMAGEP${i}.img"
 			done
-			exit 1;
+			exit 99;
 		    fi
 		    ;;
 	    esac
-- 
2.27.0

[PATCH v2 6/8] tests: A failure of mktemp should cause the test script to exit with code 99

[Glenn Washburn]

A test exiting with code 99 means that there was an error in the test itself
and not a failure in the thing being tested (also known as a hard error).

Signed-off-by: Glenn Washburn <development@efficientek.com>
---
 tests/ahci_test.in            | 4 ++--
 tests/ehci_test.in            | 4 ++--
 tests/gettext_strings_test.in | 2 +-
 tests/grub_cmd_test.in        | 6 +++---
 tests/grub_script_blockarg.in | 2 +-
 tests/ohci_test.in            | 4 ++--
 tests/partmap_test.in         | 4 ++--
 tests/pata_test.in            | 4 ++--
 tests/syslinux_test.in        | 2 +-
 tests/test_sha512sum.in       | 6 +++---
 tests/uhci_test.in            | 4 ++--
 11 files changed, 21 insertions(+), 21 deletions(-)

diff --git a/tests/ahci_test.in b/tests/ahci_test.in
index 30dc9d31a..a2bcff6b9 100644
--- a/tests/ahci_test.in
+++ b/tests/ahci_test.in
@@ -34,8 +34,8 @@ case "${grub_modinfo_target_cpu}-${grub_modinfo_platform}" in
 	exit 0;;
 esac
 
-imgfile="`mktemp "${TMPDIR:-/tmp}/tmp.XXXXXXXXXX"`" || exit 1
-outfile="`mktemp "${TMPDIR:-/tmp}/tmp.XXXXXXXXXX"`" || exit 1
+imgfile="`mktemp "${TMPDIR:-/tmp}/tmp.XXXXXXXXXX"`" || exit 99
+outfile="`mktemp "${TMPDIR:-/tmp}/tmp.XXXXXXXXXX"`" || exit 99
 
 echo "hello" > "$outfile"
 
diff --git a/tests/ehci_test.in b/tests/ehci_test.in
index 64bd80070..da7652bd3 100644
--- a/tests/ehci_test.in
+++ b/tests/ehci_test.in
@@ -34,8 +34,8 @@ case "${grub_modinfo_target_cpu}-${grub_modinfo_platform}" in
 	exit 0;;
 esac
 
-imgfile="`mktemp "${TMPDIR:-/tmp}/tmp.XXXXXXXXXX"`" || exit 1
-outfile="`mktemp "${TMPDIR:-/tmp}/tmp.XXXXXXXXXX"`" || exit 1
+imgfile="`mktemp "${TMPDIR:-/tmp}/tmp.XXXXXXXXXX"`" || exit 99
+outfile="`mktemp "${TMPDIR:-/tmp}/tmp.XXXXXXXXXX"`" || exit 99
 
 echo "hello" > "$outfile"
 
diff --git a/tests/gettext_strings_test.in b/tests/gettext_strings_test.in
index 813999ebe..1c37fe41b 100644
--- a/tests/gettext_strings_test.in
+++ b/tests/gettext_strings_test.in
@@ -2,7 +2,7 @@
 
 cd '@srcdir@'
 
-tdir="$(mktemp -d "${TMPDIR:-/tmp}/tmp.XXXXXXXXXX")"
+tdir="$(mktemp -d "${TMPDIR:-/tmp}/tmp.XXXXXXXXXX")" || exit 99
 
 xgettext -f po/POTFILES.in -L C -o "$tdir/"skip.pot -x po/grub.pot --keyword=grub_util_info --keyword=grub_dprintf:1,2 --keyword=grub_register_command --keyword=grub_register_extcmd --keyword=grub_efiemu_resolve_symbol --keyword=grub_efiemu_register_symbol --keyword=grub_dl_load --keyword=grub_crypto_lookup_md_by_name --keyword=grub_crypto_lookup_cipher_by_name --keyword=grub_efiemu_resolve_symbol --keyword=alias --keyword=grub_ieee1275_get_property:2 --keyword=grub_ieee1275_find_device --keyword=grub_ieee1275_get_integer_property:2 --keyword=INIT_IEEE1275_COMMON:2  --keyword=grub_boot_time --keyword=grub_env_get --keyword=grub_env_set --keyword=grub_register_variable_hook --keyword=grub_fatal --keyword=__asm__ --keyword=volatile --keyword=__volatile__  --keyword=grub_error:2 --from-code=iso-8859-1
 xgettext -f po/POTFILES.in -L C -o "$tdir/"skip2.pot -x po/grub.pot --keyword=volatile:2 --keyword=__volatile__:2 --keyword=grub_dprintf:2 --from-code=iso-8859-1
diff --git a/tests/grub_cmd_test.in b/tests/grub_cmd_test.in
index dac6f7d6a..043c3a634 100644
--- a/tests/grub_cmd_test.in
+++ b/tests/grub_cmd_test.in
@@ -2,8 +2,8 @@
 set -e
 
 # create a randome file
-empty="`mktemp "${TMPDIR:-/tmp}/tmp.XXXXXXXXXX"`" || exit 1
-non_empty="`mktemp "${TMPDIR:-/tmp}/tmp.XXXXXXXXXX"`" || exit 1
+empty="`mktemp "${TMPDIR:-/tmp}/tmp.XXXXXXXXXX"`" || exit 99
+non_empty="`mktemp "${TMPDIR:-/tmp}/tmp.XXXXXXXXXX"`" || exit 99
 cat >$non_empty <<EOF
 hello world!
 EOF
@@ -21,7 +21,7 @@ else
 fi
 
 
-outfile="`mktemp "${TMPDIR:-/tmp}/tmp.XXXXXXXXXX"`" || exit 1
+outfile="`mktemp "${TMPDIR:-/tmp}/tmp.XXXXXXXXXX"`" || exit 99
 @builddir@/grub-shell --files=$grub_empty=$empty  --files=$grub_non_empty=$non_empty>$outfile <<EOF
 if ! test -f $grub_empty; then
   echo FAIL1
diff --git a/tests/grub_script_blockarg.in b/tests/grub_script_blockarg.in
index 6152cc141..6d63a345a 100644
--- a/tests/grub_script_blockarg.in
+++ b/tests/grub_script_blockarg.in
@@ -27,7 +27,7 @@ cmd='test_blockarg { true }'
 v=`echo "$cmd" | @builddir@/grub-shell`
 error_if_not "$v" '{ true }'
 
-tmp=`mktemp "${TMPDIR:-/tmp}/tmp.XXXXXXXXXX"` || exit 1
+tmp=`mktemp "${TMPDIR:-/tmp}/tmp.XXXXXXXXXX"` || exit 99
 cmd='test_blockarg { test_blockarg { true } }'
 echo "$cmd" | @builddir@/grub-shell >$tmp
 error_if_not "`head -n1 $tmp|tail -n1`" '{ test_blockarg { true } }'
diff --git a/tests/ohci_test.in b/tests/ohci_test.in
index cc35f67a9..c55aad4ad 100644
--- a/tests/ohci_test.in
+++ b/tests/ohci_test.in
@@ -34,8 +34,8 @@ case "${grub_modinfo_target_cpu}-${grub_modinfo_platform}" in
 	exit 0;;
 esac
 
-imgfile="`mktemp "${TMPDIR:-/tmp}/tmp.XXXXXXXXXX"`" || exit 1
-outfile="`mktemp "${TMPDIR:-/tmp}/tmp.XXXXXXXXXX"`" || exit 1
+imgfile="`mktemp "${TMPDIR:-/tmp}/tmp.XXXXXXXXXX"`" || exit 99
+outfile="`mktemp "${TMPDIR:-/tmp}/tmp.XXXXXXXXXX"`" || exit 99
 
 echo "hello" > "$outfile"
 
diff --git a/tests/partmap_test.in b/tests/partmap_test.in
index 5f18ab51c..7906db43d 100644
--- a/tests/partmap_test.in
+++ b/tests/partmap_test.in
@@ -101,8 +101,8 @@ if ! which parted >/dev/null 2>&1; then
    exit 77
 fi
 
-imgfile="`mktemp "${TMPDIR:-/tmp}/tmp.XXXXXXXXXX"`" || exit 1
-outfile="`mktemp "${TMPDIR:-/tmp}/tmp.XXXXXXXXXX"`" || exit 1
+imgfile="`mktemp "${TMPDIR:-/tmp}/tmp.XXXXXXXXXX"`" || exit 99
+outfile="`mktemp "${TMPDIR:-/tmp}/tmp.XXXXXXXXXX"`" || exit 99
 
 #
 # MSDOS partition types
diff --git a/tests/pata_test.in b/tests/pata_test.in
index 3d19cecde..0db4778d7 100644
--- a/tests/pata_test.in
+++ b/tests/pata_test.in
@@ -38,8 +38,8 @@ case "${grub_modinfo_target_cpu}-${grub_modinfo_platform}" in
 	;;
 esac
 
-imgfile="`mktemp "${TMPDIR:-/tmp}/tmp.XXXXXXXXXX"`" || exit 1
-outfile="`mktemp "${TMPDIR:-/tmp}/tmp.XXXXXXXXXX"`" || exit 1
+imgfile="`mktemp "${TMPDIR:-/tmp}/tmp.XXXXXXXXXX"`" || exit 99
+outfile="`mktemp "${TMPDIR:-/tmp}/tmp.XXXXXXXXXX"`" || exit 99
 
 echo "hello" > "$outfile"
 
diff --git a/tests/syslinux_test.in b/tests/syslinux_test.in
index 4ea86390e..44d3cdf7a 100644
--- a/tests/syslinux_test.in
+++ b/tests/syslinux_test.in
@@ -2,7 +2,7 @@
 
 set -e
 
-outfile="`mktemp "${TMPDIR:-/tmp}/tmp.XXXXXXXXXX"`" || exit 1
+outfile="`mktemp "${TMPDIR:-/tmp}/tmp.XXXXXXXXXX"`" || exit 99
 
 "@builddir@/grub-syslinux2cfg" -r "@abs_top_srcdir@/tests/syslinux/ubuntu10.04" "@abs_top_srcdir@/tests/syslinux/ubuntu10.04/isolinux/isolinux.cfg" -o "$outfile"
 
diff --git a/tests/test_sha512sum.in b/tests/test_sha512sum.in
index d97b7ae2c..b2bd89609 100644
--- a/tests/test_sha512sum.in
+++ b/tests/test_sha512sum.in
@@ -2,7 +2,7 @@
 set -e
 
 # create a randome file
-file="`mktemp "${TMPDIR:-/tmp}/tmp.XXXXXXXXXX"`" || exit 1
+file="`mktemp "${TMPDIR:-/tmp}/tmp.XXXXXXXXXX"`" || exit 99
 cat >$file <<EOF
 hello world!
 EOF
@@ -16,12 +16,12 @@ else
 fi
 
 
-outfile1="`mktemp "${TMPDIR:-/tmp}/tmp.XXXXXXXXXX"`" || exit 1
+outfile1="`mktemp "${TMPDIR:-/tmp}/tmp.XXXXXXXXXX"`" || exit 99
 @builddir@/grub-shell --files=/boot/grub/file=$file >$outfile1 <<EOF
 sha512sum $grub_file
 EOF
 
-outfile2="`mktemp "${TMPDIR:-/tmp}/tmp.XXXXXXXXXX"`" || exit 1
+outfile2="`mktemp "${TMPDIR:-/tmp}/tmp.XXXXXXXXXX"`" || exit 99
 sha512sum $file >$outfile2
 
 SHA1=`cat $outfile1 | tr -d '\n' | cut -f1 -d\ `
diff --git a/tests/uhci_test.in b/tests/uhci_test.in
index 6aab10ba6..193129687 100644
--- a/tests/uhci_test.in
+++ b/tests/uhci_test.in
@@ -34,8 +34,8 @@ case "${grub_modinfo_target_cpu}-${grub_modinfo_platform}" in
 	exit 0;;
 esac
 
-imgfile="`mktemp "${TMPDIR:-/tmp}/tmp.XXXXXXXXXX"`" || exit 1
-outfile="`mktemp "${TMPDIR:-/tmp}/tmp.XXXXXXXXXX"`" || exit 1
+imgfile="`mktemp "${TMPDIR:-/tmp}/tmp.XXXXXXXXXX"`" || exit 99
+outfile="`mktemp "${TMPDIR:-/tmp}/tmp.XXXXXXXXXX"`" || exit 99
 
 echo "hello" > "$outfile"
 
-- 
2.27.0

[PATCH v2 7/8] tests: Exit with skipped exit code when test not performed

[Glenn Washburn]

These tests were not performed and therefore did not pass, nor fail. This
fixes misleading test exit code where, for instance, the pseries_test will
pass on i386-pc, which is not a pseries architecture.

Signed-off-by: Glenn Washburn <development@efficientek.com>
---
 tests/ahci_test.in          |  8 ++++----
 tests/cdboot_test.in        |  8 ++++----
 tests/core_compress_test.in |  2 +-
 tests/ehci_test.in          |  8 ++++----
 tests/fddboot_test.in       | 16 ++++++++--------
 tests/grub_cmd_date.in      |  2 +-
 tests/grub_cmd_set_date.in  |  6 +++---
 tests/hddboot_test.in       |  6 +++---
 tests/netboot_test.in       | 12 ++++++------
 tests/ohci_test.in          |  8 ++++----
 tests/partmap_test.in       |  8 ++++----
 tests/pata_test.in          |  6 +++---
 tests/pseries_test.in       |  2 +-
 tests/uhci_test.in          |  8 ++++----
 14 files changed, 50 insertions(+), 50 deletions(-)

diff --git a/tests/ahci_test.in b/tests/ahci_test.in
index a2bcff6b9..0e1c3679b 100644
--- a/tests/ahci_test.in
+++ b/tests/ahci_test.in
@@ -22,16 +22,16 @@ grubshell=@builddir@/grub-shell
 case "${grub_modinfo_target_cpu}-${grub_modinfo_platform}" in
     # PLATFORM: Don't mess with real devices when OS is active
     *-emu)
-	exit 0;;
+	exit 77;;
     # FIXME: qemu gets bonito DMA wrong
     mipsel-loongson)
-	exit 0;;
+	exit 77;;
     # PLATFORM: no AHCI on ARC and qemu-mips platforms
     mips*-arc | mips*-qemu_mips)
-	exit 0;;
+	exit 77;;
     # FIXME: No native drivers are available for those
     powerpc-ieee1275 | sparc64-ieee1275 | arm*-efi)
-	exit 0;;
+	exit 77;;
 esac
 
 imgfile="`mktemp "${TMPDIR:-/tmp}/tmp.XXXXXXXXXX"`" || exit 99
diff --git a/tests/cdboot_test.in b/tests/cdboot_test.in
index 7229f79fb..c0a5212ff 100644
--- a/tests/cdboot_test.in
+++ b/tests/cdboot_test.in
@@ -22,16 +22,16 @@ grubshell=@builddir@/grub-shell
 case "${grub_modinfo_target_cpu}-${grub_modinfo_platform}" in
     # PLATFORM: emu is different
     *-emu)
-	exit 0;;
+	exit 77;;
     # PLATFORM: Flash targets
     i386-qemu | i386-coreboot | mips-qemu_mips | mipsel-qemu_mips)
-	exit 0;;
+	exit 77;;
     # FIXME: currently grub-shell uses only -kernel for loongson
     mipsel-loongson)
-	exit 0;;
+	exit 77;;
     # FIXME: OFW fails to open CD-ROM
     i386-ieee1275)
-	exit 0;;
+	exit 77;;
 esac
 
 v=`echo hello | "${grubshell}" --boot=cd`
diff --git a/tests/core_compress_test.in b/tests/core_compress_test.in
index 90dd00607..72d2eca7d 100644
--- a/tests/core_compress_test.in
+++ b/tests/core_compress_test.in
@@ -22,7 +22,7 @@ grubshell=@builddir@/grub-shell
 case "${grub_modinfo_target_cpu}-${grub_modinfo_platform}" in
     # FIXME: Only mips currently supports configurable core compression
     *-emu | i386-* | x86_64-* | sparc64-* | ia64-*)
-	exit 0
+	exit 77
 	;;
 esac
 
diff --git a/tests/ehci_test.in b/tests/ehci_test.in
index da7652bd3..bd80f93d4 100644
--- a/tests/ehci_test.in
+++ b/tests/ehci_test.in
@@ -22,16 +22,16 @@ grubshell=@builddir@/grub-shell
 case "${grub_modinfo_target_cpu}-${grub_modinfo_platform}" in
     # PLATFORM: Don't mess with real devices when OS is active
     *-emu)
-	exit 0;;
+	exit 77;;
     # FIXME: qemu gets bonito DMA wrong
     mipsel-loongson)
-	exit 0;;
+	exit 77;;
     # PLATFORM: no USB on ARC and qemu-mips platforms
     mips*-arc | mips*-qemu_mips)
-	exit 0;;
+	exit 77;;
     # FIXME: No native drivers are available for those
     powerpc-ieee1275 | sparc64-ieee1275 | arm*-efi)
-	exit 0;;
+	exit 77;;
 esac
 
 imgfile="`mktemp "${TMPDIR:-/tmp}/tmp.XXXXXXXXXX"`" || exit 99
diff --git a/tests/fddboot_test.in b/tests/fddboot_test.in
index 1bbe60ee5..5348ac56b 100644
--- a/tests/fddboot_test.in
+++ b/tests/fddboot_test.in
@@ -22,28 +22,28 @@ grubshell=@builddir@/grub-shell
 case "${grub_modinfo_target_cpu}-${grub_modinfo_platform}" in
     # PLATFORM: emu is different
     *-emu)
-	exit 0;;
+	exit 77;;
     # PLATFORM: Flash targets
     i386-qemu | i386-coreboot | mips-qemu_mips | mipsel-qemu_mips)
-	exit 0;;
+	exit 77;;
     # FIXME: currently grub-shell uses only -kernel for loongson
     mipsel-loongson)
-	exit 0;;
+	exit 77;;
     # FIXME: We don't support EFI floppy boot in grub-mkrescue
     *-efi)
-	exit 0;;
+	exit 77;;
     # FIXME: no floppy support
     i386-multiboot)
-	exit 0;;
+	exit 77;;
     # FIXME: QEMU firmware crashes when trying to boot from floppy
     sparc64-ieee1275)
-	exit 0;;
+	exit 77;;
     # FIXME: QEMU doesn't emulate SCSI floppies
     mipsel-arc | mips-arc)
-	exit 0;;
+	exit 77;;
     # PLATFORM: powerpc doesn't boot from floppy except OldWorld Macs which we don't support anyway
     powerpc-ieee1275)
-	exit 0;;
+	exit 77;;
 esac
 
 v=`echo hello | "${grubshell}" --boot=fd --mkrescue-arg="--compress=xz --fonts= --locales= --themes= -no-pad"`
diff --git a/tests/grub_cmd_date.in b/tests/grub_cmd_date.in
index f9156691e..409cb684a 100644
--- a/tests/grub_cmd_date.in
+++ b/tests/grub_cmd_date.in
@@ -5,7 +5,7 @@ set -e
 
 # FIXME: OpenBIOS on sparc64 doesn't implement RTC
 if [ "${grub_modinfo_target_cpu}-${grub_modinfo_platform}" = sparc64-ieee1275 ]; then
-    exit 0
+    exit 77
 fi
 
 pdt="$(date -u +%s)"
diff --git a/tests/grub_cmd_set_date.in b/tests/grub_cmd_set_date.in
index aac120a6c..17673cd8a 100644
--- a/tests/grub_cmd_set_date.in
+++ b/tests/grub_cmd_set_date.in
@@ -6,15 +6,15 @@ set -e
 case "${grub_modinfo_target_cpu}-${grub_modinfo_platform}" in
     # FIXME: OpenBIOS on sparc64 doesn't implement RTC
     sparc64-ieee1275)
-	exit 0;;
+	exit 77;;
     # PLATFORM: ARC doesn't provide any way to set time
     *-arc)
-	exit 0;;
+	exit 77;;
     # PLATFORM: EMU doesn't provide any way to set time
     # Even if it did we'd need some kind of sandbox to avoid
     # modifying real system time.
     *-emu)
-	exit 0;;
+	exit 77;;
 esac
 
 out=$(cat <<EOF | @builddir@/grub-shell
diff --git a/tests/hddboot_test.in b/tests/hddboot_test.in
index abe3e0ae1..110c70950 100644
--- a/tests/hddboot_test.in
+++ b/tests/hddboot_test.in
@@ -22,13 +22,13 @@ grubshell=@builddir@/grub-shell
 case "${grub_modinfo_target_cpu}-${grub_modinfo_platform}" in
     # PLATFORM: emu is different
     *-emu)
-	exit 0;;
+	exit 77;;
     # PLATFORM: Flash targets
     i386-qemu | i386-coreboot | mips-qemu_mips | mipsel-qemu_mips)
-	exit 0;;
+	exit 77;;
     # FIXME: currently grub-shell uses only -kernel for loongson
     mipsel-loongson)
-	exit 0;;
+	exit 77;;
 esac
 
 v=`echo hello | "${grubshell}" --boot=hd`
diff --git a/tests/netboot_test.in b/tests/netboot_test.in
index 6a1d95a22..e0b2c9d25 100644
--- a/tests/netboot_test.in
+++ b/tests/netboot_test.in
@@ -22,22 +22,22 @@ grubshell=@builddir@/grub-shell
 case "${grub_modinfo_target_cpu}-${grub_modinfo_platform}" in
     # PLATFORM: emu is different
     *-emu)
-	exit 0;;
+	exit 77;;
     # PLATFORM: Flash targets
     i386-qemu | i386-coreboot | mips-qemu_mips | mipsel-qemu_mips)
-	exit 0;;
+	exit 77;;
     # FIXME: currently grub-shell uses only -kernel for loongson
     mipsel-loongson)
-	exit 0;;
+	exit 77;;
     # FIXME: no rtl8139 support
     i386-multiboot)
-	exit 0;;
+	exit 77;;
     # FIXME: We don't fully support netboot on ARC
     *-arc)
-	exit 0;;
+	exit 77;;
     # FIXME: Many QEMU firmware have no netboot capability
     *-efi | i386-ieee1275 | powerpc-ieee1275 | sparc64-ieee1275)
-	exit 0;;
+	exit 77;;
 esac
 
 v=`echo hello | "${grubshell}" --boot=net`
diff --git a/tests/ohci_test.in b/tests/ohci_test.in
index c55aad4ad..105220856 100644
--- a/tests/ohci_test.in
+++ b/tests/ohci_test.in
@@ -22,16 +22,16 @@ grubshell=@builddir@/grub-shell
 case "${grub_modinfo_target_cpu}-${grub_modinfo_platform}" in
     # PLATFORM: Don't mess with real devices when OS is active
     *-emu)
-	exit 0;;
+	exit 77;;
     # FIXME: qemu gets bonito DMA wrong
     mipsel-loongson)
-	exit 0;;
+	exit 77;;
     # PLATFORM: no USB on ARC and qemu-mips platforms
     mips*-arc | mips*-qemu_mips)
-	exit 0;;
+	exit 77;;
     # FIXME: No native drivers are available for those
     powerpc-ieee1275 | sparc64-ieee1275 | arm*-efi)
-	exit 0;;
+	exit 77;;
 esac
 
 imgfile="`mktemp "${TMPDIR:-/tmp}/tmp.XXXXXXXXXX"`" || exit 99
diff --git a/tests/partmap_test.in b/tests/partmap_test.in
index 7906db43d..e72af9ce4 100644
--- a/tests/partmap_test.in
+++ b/tests/partmap_test.in
@@ -70,21 +70,21 @@ case "${grub_modinfo_target_cpu}-${grub_modinfo_platform}" in
     powerpc-ieee1275)
 	disk=ieee1275//pci@80000000/mac-io@4/ata-3@20000/disk@0
 	# FIXME: QEMU firmware has bugs which prevent it from accessing hard disk w/o recognised label.
-	exit 0
+	exit 77
 	;;
     sparc64-ieee1275)
 	disk=ieee1275//pci@1fe\,0/pci-ata@5/ide0@500/disk@0
 	# FIXME: QEMU firmware has bugs which prevent it from accessing hard disk w/o recognised label.
-	exit 0
+	exit 77
 	;;
     i386-ieee1275)
 	disk=ieee1275/d
 	# FIXME: QEMU firmware has bugs which prevent it from accessing hard disk w/o recognised label.
-	exit 0
+	exit 77
 	;;
     mips-arc)
 	# FIXME: ARC firmware has bugs which prevent it from accessing hard disk w/o dvh disklabel.
-	exit 0 ;;
+	exit 77 ;;
     mipsel-arc)
 	disk=arc/scsi0/disk0/rdisk0
 	;;
diff --git a/tests/pata_test.in b/tests/pata_test.in
index 0db4778d7..4a9c68ae4 100644
--- a/tests/pata_test.in
+++ b/tests/pata_test.in
@@ -25,13 +25,13 @@ indisk=ata0
 case "${grub_modinfo_target_cpu}-${grub_modinfo_platform}" in
     # PLATFORM: Don't mess with real devices when OS is active
     *-emu)
-	exit 0;;
+	exit 77;;
     # PLATFORM: no ATA on ARC platforms (they use SCSI)
     *-arc)
-	exit 0;;
+	exit 77;;
     # FIXME: No native drivers are available for those
     powerpc-ieee1275 | sparc64-ieee1275 | arm*-efi)
-	exit 0;;
+	exit 77;;
     i386-ieee1275)
 	disk=hdb
 	indisk=ata1
diff --git a/tests/pseries_test.in b/tests/pseries_test.in
index 655eb4f3a..9b4090cf5 100644
--- a/tests/pseries_test.in
+++ b/tests/pseries_test.in
@@ -20,7 +20,7 @@ grubshell=@builddir@/grub-shell
 . "@builddir@/grub-core/modinfo.sh"
 
 if [ "${grub_modinfo_target_cpu}-${grub_modinfo_platform}" != powerpc-ieee1275 ]; then
-   exit 0
+   exit 77
 fi
 
 if [ "$(echo hello | "${grubshell}" --pseries --timeout=180 --boot=cd)" != "Hello World" ]; then
diff --git a/tests/uhci_test.in b/tests/uhci_test.in
index 193129687..2984283c0 100644
--- a/tests/uhci_test.in
+++ b/tests/uhci_test.in
@@ -22,16 +22,16 @@ grubshell=@builddir@/grub-shell
 case "${grub_modinfo_target_cpu}-${grub_modinfo_platform}" in
     # PLATFORM: Don't mess with real devices when OS is active
     *-emu)
-	exit 0;;
+	exit 77;;
     # FIXME: qemu gets bonito DMA wrong
     mipsel-loongson)
-	exit 0;;
+	exit 77;;
     # PLATFORM: no USB on ARC and qemu-mips platforms
     mips*-arc | mips*-qemu_mips)
-	exit 0;;
+	exit 77;;
     # FIXME: No native drivers are available for those
     powerpc-ieee1275 | sparc64-ieee1275 | arm*-efi)
-	exit 0;;
+	exit 77;;
 esac
 
 imgfile="`mktemp "${TMPDIR:-/tmp}/tmp.XXXXXXXXXX"`" || exit 99
-- 
2.27.0

[PATCH v2 8/8] tests: Use @BUILD_SHEBANG@ autoconf var instead of literal shell

[Glenn Washburn]

This bring this test in line with the rest of the test scripts.

Signed-off-by: Glenn Washburn <development@efficientek.com>
---
 tests/f2fs_test.in | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/f2fs_test.in b/tests/f2fs_test.in
index 1ea77c826..8c415db61 100644
--- a/tests/f2fs_test.in
+++ b/tests/f2fs_test.in
@@ -1,4 +1,4 @@
-#!/bin/sh
+#!@BUILD_SHEBANG@
 
 set -e
 
-- 
2.27.0

Re: [PATCH v2 1/8] tests: Make sure LANG is set properly for iso9660_test

[Thomas Schmitt]

Hi,

Glenn Washburn wrote:
> LANG must be set to something that supports international characters,
> otherwise xorriso will refuse to include the file with name having
> international characters, causing the test to fail.

Can you tell me the exact error message from xorriso ?
I have some difficulties with reproducing the situation.

Your description is plausible, because libisofs takes its character set
default from nl_langinfo(3) with item CODESET and uses iconv(3) to
convert the file names to the output charset.

I would like to bring up for discussion an alternative remedy by these
xorrisofs options:

  -input-charset UTF-8 -output-charset UTF-8

-----------------------------------------------------------------------

I am quite sure that it will work, but have problems with creating the
situation where it shall be the remedy.
On a system with configured LANG=en_US.UTF-8 i fail to reproduce
the issue with LANG=C or LANG="" unless i add the explicit demand to
convert to UTF-8.

My disappointingly unproblematic xorriso test run is

  ( LANG=  xorrisofs -o test.iso 'ÄÖÜß' )

with various attempts to set the locale charset away from UTF-8:
LANG, LANGUAGE, LC_ALL set to empty text or to "C".
LANG= causes nl_langinfo(CODESET) to return "ANSI_X3.4-1968" but iconv(3)
throws no error when it shall convert "\303\204\303\226\303\234\303\237".

I get warning messages if i tell xorriso to aim for output charset UTF-8
while LANG is empty and the file name contains non-ASCII characters:

  (LANG= xorrisofs -o test.iso -output-charset UTF-8 'ÄÖÜß')

yields three times

  libisofs: WARNING : Charset conversion error. Cannot convert ÄÖÜß from ANSI_X3.4-1968 to UTF-8
  libisofs: NOTE :  > Caused by: Charset conversion error

Inspection of test.iso shows that the file name 'ÄÖÜß' was defaulted to
name '________'.

Adding the other charset option

  (LANG= xorrisofs -o test.iso \
            -input-charset UTF-8 -output-charset UTF-8 'ÄÖÜß')

yields a run without warning messages resulting in Rock Ridge file name
'ÄÖÜß'.

So i assume that my proposal is a valid alternative to setting LANG=UTF-8.
Whether it is preferrable over setting LANG to UTF-8 would have to be
discussed.
At least it would be nice for me to know why LANG= causes trouble in
the GRUB tests but not on my command line.
(I see nothing in the xorriso runs of tests/util/grub-fs-tester.in which
would be equivalent to the run which fails to convert 'ÄÖÜß' here.)


Have a nice day :)

Thomas

Re: [PATCH v2 1/8] tests: Make sure LANG is set properly for iso9660_test

[Glenn Washburn]

Hi Thomas,

On Wed, 25 Aug 2021 11:34:47 +0200
"Thomas Schmitt" <scdbackup@gmx.net> wrote:

> Hi,
> 
> Glenn Washburn wrote:
> > LANG must be set to something that supports international
> > characters, otherwise xorriso will refuse to include the file with
> > name having international characters, causing the test to fail.
> 
> Can you tell me the exact error message from xorriso ?
> I have some difficulties with reproducing the situation.

There is no error message from xorriso, it will do as you noticed
below, which is to default to replacing non-ASCII characters with
underscores.

The problem is that that behavior causes the tests to fail (rightly so
because non-ASCII names are not being tested). The following causes the
test to fail, but removing the LANG= causes success (because my system
has LANG set to a UTF8 encoding).

  make SUBDIRS= LANG= TESTS=iso9660_test check

> Your description is plausible, because libisofs takes its character
> set default from nl_langinfo(3) with item CODESET and uses iconv(3) to
> convert the file names to the output charset.
> 
> I would like to bring up for discussion an alternative remedy by these
> xorrisofs options:
> 
>   -input-charset UTF-8 -output-charset UTF-8
> 
> -----------------------------------------------------------------------
> 
> I am quite sure that it will work, but have problems with creating the
> situation where it shall be the remedy.
> On a system with configured LANG=en_US.UTF-8 i fail to reproduce
> the issue with LANG=C or LANG="" unless i add the explicit demand to
> convert to UTF-8.
> 
> My disappointingly unproblematic xorriso test run is
> 
>   ( LANG=  xorrisofs -o test.iso 'ÄÖÜß' )
> 
> with various attempts to set the locale charset away from UTF-8:
> LANG, LANGUAGE, LC_ALL set to empty text or to "C".
> LANG= causes nl_langinfo(CODESET) to return "ANSI_X3.4-1968" but
> iconv(3) throws no error when it shall convert
> "\303\204\303\226\303\234\303\237".
> 
> I get warning messages if i tell xorriso to aim for output charset
> UTF-8 while LANG is empty and the file name contains non-ASCII
> characters:
> 
>   (LANG= xorrisofs -o test.iso -output-charset UTF-8 'ÄÖÜß')
> 
> yields three times
> 
>   libisofs: WARNING : Charset conversion error. Cannot convert ÄÖÜß
> from ANSI_X3.4-1968 to UTF-8 libisofs: NOTE :  > Caused by: Charset
> conversion error
> 
> Inspection of test.iso shows that the file name 'ÄÖÜß' was defaulted
> to name '________'.
> 
> Adding the other charset option
> 
>   (LANG= xorrisofs -o test.iso \
>             -input-charset UTF-8 -output-charset UTF-8 'ÄÖÜß')
> 
> yields a run without warning messages resulting in Rock Ridge file
> name 'ÄÖÜß'.
> 
> So i assume that my proposal is a valid alternative to setting
> LANG=UTF-8. Whether it is preferrable over setting LANG to UTF-8
> would have to be discussed.
> At least it would be nice for me to know why LANG= causes trouble in
> the GRUB tests but not on my command line.
> (I see nothing in the xorriso runs of tests/util/grub-fs-tester.in
> which would be equivalent to the run which fails to convert 'ÄÖÜß'
> here.)

The takeaway here is that 'ÄÖÜß' is being converted and its is
precisely that which is causing the test to fail. So unless I'm
mistaken, you are confirming the issue this patch resolves.

It looks like you have a viable alternative to my proposed patch,
though I've not tested it myself. I really don't care about the
implementation as long as the test succeeds when LANG is unset or
empty. Despite having an alternative approach, do you have concerns or
objections to my patch? Also would you like to create a patch that I
would be happy to test?

Glenn

Re: [PATCH v2 1/8] tests: Make sure LANG is set properly for iso9660_test

[Thomas Schmitt]

Hi,

Glenn Washburn wrote:
>  Despite having an alternative approach, do you have concerns or
> objections to my patch?

Not specifically. I initially only wanted to mention that there is
a xorriso alternative to setting LANG. Both ways have different
implications in detail. (LANG is global to the test code. The
xorriso charset commands would affect only the xorriso run.)

But my attempt to reproduce the problem failed and i do not yet know why
exactly your xorriso run had conversion problems although no explicit
character set conversion was demanded. So i hoped for an enlightening
error message.


> So unless I'm
> mistaken, you are confirming the issue this patch resolves.

It happens only if i tell xorriso explicitely that it shall convert to
UTF-8. In the next step it vanishes when i also tell xorriso to also
convert from UTF-8.

I will investigate further for the sake of a better xorriso.
If i find something of interest for GRUB then i will report.


Have a nice day :)

Thomas

Re: [PATCH v2 1/8] tests: Make sure LANG is set properly for iso9660_test

[Glenn Washburn]

[-- Attachment #1: Type: text/plain, Size: 2120 bytes --]

On Thu, 26 Aug 2021 08:53:31 +0200
"Thomas Schmitt" <scdbackup@gmx.net> wrote:

> Hi,
> 
> Glenn Washburn wrote:
> >  Despite having an alternative approach, do you have concerns or
> > objections to my patch?
> 
> Not specifically. I initially only wanted to mention that there is
> a xorriso alternative to setting LANG. Both ways have different
> implications in detail. (LANG is global to the test code. The
> xorriso charset commands would affect only the xorriso run.)
> 
> But my attempt to reproduce the problem failed and i do not yet know
> why exactly your xorriso run had conversion problems although no
> explicit character set conversion was demanded. So i hoped for an
> enlightening error message.

Nope, no error message, I believe xorriso is working as intended.

Did you verify that running the test with LANG=, as I suggested in my
prior email, did indeed fail? I have attached an output log of the
iso9660_test failure. You can see that the file
"éàèüöäëñкирилица䏌䐓䏕Ελληνικά䏌䐓䏕" was converted to
"__________________________________________________________________".


Here is the argv array according to strace:
["xorriso", "--rockridge", "off", "-compliance", "rec_mtime", "-as",
"mkisofs", "-iso-level", "3", "-graft-points", "-J", "-joliet-long",
"-V",
"g;/_\303\251\344\217\214\344\220\223\344\217\225\344\216\233\344\216\276\344\217\264\320\272\320\270\321\202
u", "--modification-date=2021082616575500", "-o",
"/media/tmpfs/bootloader/testtmp/tmp.gZvxe3D0aH/joliet_512_512_1_0.img",
"/=/media/tmpfs/bootloader/testtmp/tmp.gZvxe3D0aH/master"]

> > So unless I'm
> > mistaken, you are confirming the issue this patch resolves.
> 
> It happens only if i tell xorriso explicitely that it shall convert to
> UTF-8. In the next step it vanishes when i also tell xorriso to also
> convert from UTF-8.

I missed that in the previous email. The biggest difference I see with
the grub test is that its getting the non-ASCII filename from the file
system and not being passed in as a commandline arg explicitly.

Glenn

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: iso9660_test.log --]
[-- Type: text/x-log, Size: 2606 bytes --]

Testing joliet filesystem with ./grub-fs-tester
xorriso 1.5.2 : RockRidge filesystem manipulator, libburnia project.

Drive current: -outdev 'stdio:/media/tmpfs/bootloader/testtmp/tmp.XrGFpM2KlN/joliet_512_512_1_0.img'
Media current: stdio file, overwriteable
Media status : is blank
Media summary: 0 sessions, 0 data blocks, 0 data, 4009m free
xorriso : WARNING : -volid text problematic as automatic mount point name
xorriso : WARNING : -volid text is too long for Joliet (32 > 16)
xorriso : WARNING : -volid text does not comply to ISO 9660 / ECMA 119 rules
Added to ISO image: directory '/'='/media/tmpfs/bootloader/testtmp/tmp.XrGFpM2KlN/master'
xorriso : UPDATE :     221 files added in 1 seconds
xorriso : UPDATE :     221 files added in 1 seconds
ISO image produced: 21557 sectors
Written to medium : 21557 sectors at LBA 0
Writing to 'stdio:/media/tmpfs/bootloader/testtmp/tmp.XrGFpM2KlN/joliet_512_512_1_0.img' completed successfully.

Device proc: Filesystem type procfs - Sector size 512B - Total size 0KiB
Device loop0: Filesystem type iso9660 - Label `g;/_____________' - Last modification time 2021-08-26 16:39:55 Thursday, UUID 2021-08-26-16-39-55-00 - Sector size 512B - Total size 43114KiB
Device host: Filesystem type hostfs - Sector size 512B - Total size 0KiB

ILIST FAIL
DIR          20210826163955 ./
DIR          20210826163955 ../
5242879      20210826163955 .!"#%@$%&'()+,-.<=>^{_}[]`|~.
DIR          20210826163955 0/
5242879      20210826163955 1.img
5242879      20210826163955 CaSe
5242879      20210826163955 __________________________________________________________________
972398       20210826163955 american-english
5242879      20210826163955 cAsE
5242879      20210826163955 hard
5242879      20210826163955 qwertzuiopasdfghjklyxcvbnm1234567890qwertzuiopasdfghjklyxcvbnm1234567890oiewqfiewioqoiqoiurqruewqoiuwoi
DIR          20210826163955 sdir/
total 31672
drwxrwxr-x 3 crass crass      60 Aug 26 16:39 0
-rw-rw-r-- 2 crass crass 5242879 Aug 26 16:39 1.img
-rw-rw-r-- 1 crass crass 5242879 Aug 26 16:39 CaSe
-rw-r--r-- 1 crass crass  972398 Aug 26 16:39 american-english
-rw-rw-r-- 1 crass crass 5242879 Aug 26 16:39 cAsE
-rw-rw-r-- 2 crass crass 5242879 Aug 26 16:39 hard
-rw-rw-r-- 1 crass crass 5242879 Aug 26 16:39 qwertzuiopasdfghjklyxcvbnm1234567890qwertzuiopasdfghjklyxcvbnm1234567890oiewqfiewioqoiqoiurqruewqoiuwoi
drwxrwxr-x 2 crass crass      60 Aug 26 16:39 sdir
-rw-rw-r-- 1 crass crass 5242879 Aug 26 16:39 éàèüöäëñкирилица䏌䐓䏕Ελληνικά䏌䐓䏕
FAIL iso9660_test (exit status: 1)

Re: [PATCH v2 1/8] tests: Make sure LANG is set properly for iso9660_test

[Thomas Schmitt]

Hi,

now i know that i should have asked for the test-suite.log file.

The problem turned out to be in Joliet, not in ISO 9660.
Because Joliet gets its file names and other texts encoded as UCS-2
16-bit characters it is indeed inavoidable to define the meaning of the
bytes in a Unix file name.
The UTF-8 sequence \303\251 has no meaning as ANSI_X3.4-1968 characters
(which are what we learned to love as 7-bit ASCII). So iconv fails.

My remedy proposal

  xorriso ... -as mkisofs -input-charset UTF-8 -output-charset UTF-8 ...

turned out to be a valid alternative to

  LANG=en_US.UTF-8

Using above charset options unconditionally would make obsolete the
warning in Glenn Washburn's patch at least for the xorriso runs:

  +elif [ -n "${LANG##*UTF*}" ]; then
  +   echo "WARNING: LANG=$LANG appears to not be unicode, international file test may fail."

It would still be beneficial to default LANG="" to LANG=en_US.UTF-8
because 7-bit ASCII is really an odd character interpretation for Unix
filenames. (HP-UX from 1986 maybe ...)

--------------------------------------------------------------------------

Noob question:
How can i force "make check" to do something at its second run ?

--------------------------------------------------------------------------
Long story:

On a Debian 10 with a freshly made GRUB git clone i see indeed a failure
of iso9660_test with mutilated volume id and a mutilated exotic filename
after

  make SUBDIRS= LANG= TESTS=iso9660_test check

On the same machine with the same xorriso, the same file name from the
log and the volume id taken from the script, i get with

  LANG= xorriso -as mkisofs -o test.iso -V "...exotic.characters..." test_tree

a somewhat confused xterm (or libreadline) but

  xorriso -indev test.iso -find / --

reports the exotic volume id and the exotic filename.

The "make ... check" run says on the second try
  make[3]: 'iso9660_test' is up to date.
but i can reproduce the problem by

  LANG= ./grub-fs-tester joliet

The trigger for the file name mutilation seems to be the mkisofs
emulation option for creating a Joliet tree
  -J
With default settings of mount(8) or xorriso's file name display the
problem becomes visible only if no Rock Ridge info was added to the
ISO 9660 tree. This is caused by xorriso command
  --rockridge off

Nevertheless file(1) and xorriso still see the exotic volume id.
Binary inspection shows that the ISO 9660 volume id (at offset 32768 + 40)
is originally exotic, but the Joliet volume id (at offset 34816 + 40) was
mutilated.

So obviously the problem sits in the Joliet name conversion to UCS-2.
It is inavoidable if the file names do not match the input character set
(defaulted from locale) because the Unix file names cannot be just copied
to Joliet directory records.

Now that i can reproduce the problem i can also confirm that my remedy
proposal would work as expected. It preservies the exotic names in the
Joliet tree and the exotic volume id in the Joliet supplementary volume
descriptor.


Have a nice day :)

Thomas

Re: [PATCH v2 1/8] tests: Make sure LANG is set properly for iso9660_test

[Glenn Washburn]

On Thu, 26 Aug 2021 19:49:37 +0200
"Thomas Schmitt" <scdbackup@gmx.net> wrote:

> Hi,
> 
> now i know that i should have asked for the test-suite.log file.
> 
> The problem turned out to be in Joliet, not in ISO 9660.
> Because Joliet gets its file names and other texts encoded as UCS-2
> 16-bit characters it is indeed inavoidable to define the meaning of
> the bytes in a Unix file name.
> The UTF-8 sequence \303\251 has no meaning as ANSI_X3.4-1968
> characters (which are what we learned to love as 7-bit ASCII). So
> iconv fails.
> 
> My remedy proposal
> 
>   xorriso ... -as mkisofs -input-charset UTF-8 -output-charset UTF-8
> ...

Do there need to be any UTF-8 locales installed (or any locales for
that matter) for this to work? My guess is no.

> turned out to be a valid alternative to
> 
>   LANG=en_US.UTF-8
> 
> Using above charset options unconditionally would make obsolete the
> warning in Glenn Washburn's patch at least for the xorriso runs:
> 
>   +elif [ -n "${LANG##*UTF*}" ]; then
>   +   echo "WARNING: LANG=$LANG appears to not be unicode,
> international file test may fail."
> 
> It would still be beneficial to default LANG="" to LANG=en_US.UTF-8
> because 7-bit ASCII is really an odd character interpretation for Unix
> filenames. (HP-UX from 1986 maybe ...)

Correct me if I'm wrong, but this logic about defaulting LANG is more
general than the iso9660_test. Would you agree that this should be
applied to the tests a a whole?

If yes, then I should probably remove this patch altogether and put
this code elsewhere.

--------------------------------------------------------------------------
> 
> Noob question:
> How can i force "make check" to do something at its second run ?

I might be answering this question below.

--------------------------------------------------------------------------
> Long story:
> 
> On a Debian 10 with a freshly made GRUB git clone i see indeed a
> failure of iso9660_test with mutilated volume id and a mutilated
> exotic filename after
> 
>   make SUBDIRS= LANG= TESTS=iso9660_test check
> 
> On the same machine with the same xorriso, the same file name from the
> log and the volume id taken from the script, i get with
> 
>   LANG= xorriso -as mkisofs -o test.iso -V "...exotic.characters..."
> test_tree
> 
> a somewhat confused xterm (or libreadline) but
> 
>   xorriso -indev test.iso -find / --
> 
> reports the exotic volume id and the exotic filename.
> 
> The "make ... check" run says on the second try
>   make[3]: 'iso9660_test' is up to date.
> but i can reproduce the problem by
> 
>   LANG= ./grub-fs-tester joliet

Yes the iso9660_test is up to date in the sense that it does not need
to be generated from iso9660_test.in. However, "make check" should
still be running the test. IOW, the tests are always rerun, there is no
make rule that does not run tests based on them having been perviously
run (that I know of).

> The trigger for the file name mutilation seems to be the mkisofs
> emulation option for creating a Joliet tree
>   -J
> With default settings of mount(8) or xorriso's file name display the
> problem becomes visible only if no Rock Ridge info was added to the
> ISO 9660 tree. This is caused by xorriso command
>   --rockridge off
> 
> Nevertheless file(1) and xorriso still see the exotic volume id.
> Binary inspection shows that the ISO 9660 volume id (at offset 32768
> + 40) is originally exotic, but the Joliet volume id (at offset 34816
> + 40) was mutilated.
> 
> So obviously the problem sits in the Joliet name conversion to UCS-2.
> It is inavoidable if the file names do not match the input character
> set (defaulted from locale) because the Unix file names cannot be
> just copied to Joliet directory records.
>
> Now that i can reproduce the problem i can also confirm that my remedy
> proposal would work as expected. It preservies the exotic names in the
> Joliet tree and the exotic volume id in the Joliet supplementary
> volume descriptor.

As i understand it, your proposed change is entirely with in
grub-fs-tester. Would you submit a patch with the changes you've
outlined that would fix this issue for when LANG is empty? Based on the
work you've done, I think it makes most sense for you to author the
patch. And I'll test/review it.

Glenn

Re: [PATCH v2 1/8] tests: Make sure LANG is set properly for iso9660_test

[Thomas Schmitt]

Hi,

i wrote:
> >   xorriso ... -as mkisofs -input-charset UTF-8 -output-charset UTF-8

Glenn Washburn wrote:
> Do there need to be any UTF-8 locales installed (or any locales for
> that matter) for this to work? My guess is no.

I expect the same. It's not easy to test, though.

In theory it is essentially a matter of iconv_open(3) and iconv(3) whether
conversion works.
The installed locale only gives the default values for parameters of
  iconv_t iconv_open(const char *tocode, const char *fromcode);
The options -input-charset and -output-charset override the locale
setting, whatever it is.
(Actually xorriso stumbled over the hardcoded output character set of
Joliet, which is UCS-2.)


> Correct me if I'm wrong, but this logic about defaulting LANG is more
> general than the iso9660_test. Would you agree that this should be
> applied to the tests a a whole?

The comments in grub-fs-tester talk of UTF-8 as expected character set
and the text constants look like UTF-8, too. So it would not be wrong
to enforce a UTF-8 locale.

It might be peculiar to xorriso to look at the locale when interpreting
the meaning of bytes which represent characters.
But this interpretation is inevitable if the target filesystem specs
prescribe a particular character set, as Joliet does, and if we aim for
dealing with UTF-8 names.


> If yes, then I should probably remove this patch altogether and put
> this code elsewhere.

I think it can be justified to hardcode UTF-8 in grub-fs-tester regardless
whether higher levels of the test empire or the user have own interests
in locale settings.
The code in grub-fs-tester is a good example for the situation that the
file names on disk might belong to users with differing locales. We expect
UTF-8 to be the common base of all those nowadays.

So i'd vote for something like

  if [ -n "${LANG##*UTF*}" ]; then
     echo "NOTE: LANG=$LANG appears to not be UTF-8."
     echo "NOTE: Setting LANG=en_US.UTF-8 to match the test file names."
     export LANG=en_US.UTF-8
  fi

at the start of grub-fs-tester.in.


> > How can i force "make check" to do something at its second run ?

> However, "make check" should still be running the test.

I obviously got misled by the many make messages and the silence of the
iso9660_test run.


> Would you submit a patch with the changes you've
> outlined that would fix this issue for when LANG is empty?

I'd go farer and smack down everything that does not claim to be UTF-8.

Is a git diff enough or has it to be git format-patch from a local commit ?
Is git send-email necessary ?
(My relation to git based interaction with projects is best illustrated by
  https://upload.wikimedia.org/wikipedia/commons/thumb/1/17/Laocoon_Pio-Clementino_Inv1059-1064-1067.jpg/564px-Laocoon_Pio-Clementino_Inv1059-1064-1067.jpg
)


Have a nice day :)

Thomas

Re: [PATCH v2 1/8] tests: Make sure LANG is set properly for iso9660_test

[Glenn Washburn]

On Thu, 26 Aug 2021 23:28:04 +0200
"Thomas Schmitt" <scdbackup@gmx.net> wrote:

> > If yes, then I should probably remove this patch altogether and put
> > this code elsewhere.
> 
> I think it can be justified to hardcode UTF-8 in grub-fs-tester
> regardless whether higher levels of the test empire or the user have
> own interests in locale settings.
> The code in grub-fs-tester is a good example for the situation that
> the file names on disk might belong to users with differing locales.
> We expect UTF-8 to be the common base of all those nowadays.
> 
> So i'd vote for something like
> 
>   if [ -n "${LANG##*UTF*}" ]; then
>      echo "NOTE: LANG=$LANG appears to not be UTF-8."
>      echo "NOTE: Setting LANG=en_US.UTF-8 to match the test file
> names." export LANG=en_US.UTF-8
>   fi
> 
> at the start of grub-fs-tester.in.

Sounds good to me, please add this to the patch.

> > Would you submit a patch with the changes you've
> > outlined that would fix this issue for when LANG is empty?
> 
> I'd go farer and smack down everything that does not claim to be
> UTF-8.
> 
> Is a git diff enough or has it to be git format-patch from a local
> commit ? Is git send-email necessary ?

I'm not sure that it has to be git format-patch then git send-email,
but that's the desired method. The following commands may help you get
to speed quickly (since its only one patch things are less complicated).

git format-patch -s <git commit hash>

Now you should have a patch file ready for sending. Setting up git
send-email isn't that hard, but I'd refer you to the man page since it
depends on your setup. Daniel might find it acceptable to send that
patch as an attachment since its just a single patch.

Glenn

Re: [PATCH v2 1/8] tests: Make sure LANG is set properly for iso9660_test

[Thomas Schmitt]

Hi,

it turns out that it might be contraproductive to set LANG=en_US.UTF-8.

If the LANG locale is not listed by
  locale -a
then nl_langinfo(3) returns "ANSI_X3.4-1968".
I stumbled over this when testing LANG=de_DE.UTF8, which despite of
my location is not an available locale on my machine.

So i think that setting any locale by LANG makes daring assumptions
about its availability, unless brain is added to inspect the installed
locals and to choose one that is based on UTF-8.
This reaches quite far beyond the problem which the patch shall solve.

What route to go ?

- Choose the first UTF-8 locale offered by locale -a ?
  (What to do if none is offered ?)

- Abandon the idea of setting LANG and rather use xorrisofs charset
  options to enforce UTF-8 as origin of the conversion ?

I will for now strive for the second alternative but am ready to change
to number one.


Have a nice day :)

Thomas

Re: [PATCH v2 1/8] tests: Make sure LANG is set properly for iso9660_test

[Glenn Washburn]

On Fri, 27 Aug 2021 21:13:17 +0200
"Thomas Schmitt" <scdbackup@gmx.net> wrote:

> Hi,
> 
> it turns out that it might be contraproductive to set
> LANG=en_US.UTF-8.
> 
> If the LANG locale is not listed by
>   locale -a
> then nl_langinfo(3) returns "ANSI_X3.4-1968".
> I stumbled over this when testing LANG=de_DE.UTF8, which despite of
> my location is not an available locale on my machine.

My patch only sets LANG if empty, so in this case I don't think its
counterproductive, just not productive.

> So i think that setting any locale by LANG makes daring assumptions
> about its availability, unless brain is added to inspect the installed
> locals and to choose one that is based on UTF-8.
> This reaches quite far beyond the problem which the patch shall solve.
> 
> What route to go ?
> 
> - Choose the first UTF-8 locale offered by locale -a ?
>   (What to do if none is offered ?)
> 
> - Abandon the idea of setting LANG and rather use xorrisofs charset
>   options to enforce UTF-8 as origin of the conversion ?
> 
> I will for now strive for the second alternative but am ready to
> change to number one.

I think that regardless we should be using the xorrisofs charset. The
issue of LANG may or may not be an issue, I'm inclined to believe not.
I was only using this method to get the test working. The configuration
of locales and LANG should be a user problem, but ideally should not
affect how the tests are run. So in my judgement option 2 is the right
way forward. If there are other LANG related issues we may have to mess
with it in the future, but we can cross that bridge if we need to. I'm
looking forward to your patch and will make sure this one gets dropped.

Glenn

Re: [PATCH v2 1/8] tests: Make sure LANG is set properly for iso9660_test

[Thomas Schmitt]

Hi,

i wrote:
> > it might be contraproductive to set LANG=en_US.UTF-8.

Glenn Washburn wrote:
> My patch only sets LANG if empty, so in this case I don't think its
> counterproductive, just not productive.

Yes. It cautiously avoids to make the situation any worse.
I meant my own announced plan which was more agressively aiming for
enforcing UTF-8.


Well, i hope that my cheat sheet for (unsuccessful) kernel contribution
attempts enabled me to create a convincing imitation of an acceptable
patch for GRUB.
To help reading the oversized line salad:
I defined the variable XORRISOFS_CHARSET and added $XORRISOFS_CHARSET
after each "-as mkisofs" in the xorriso command lines.

Tests were made without any LANG setting (effectively en_US.utf-8),
with LANG="", "C", "en_US.ISO-8859-1" (not installed), "en_US.UTF-8",
"de_DE.UTF-8" (not installed). All succeeded.


Have a nice day :)

Thomas

Re: [PATCH v2 7/8] tests: Exit with skipped exit code when test not performed

[Glenn Washburn]

On Wed, 25 Aug 2021 02:04:01 -0500
Glenn Washburn <development@efficientek.com> wrote:

> These tests were not performed and therefore did not pass, nor fail.
> This fixes misleading test exit code where, for instance, the
> pseries_test will pass on i386-pc, which is not a pseries
> architecture.
> 
> Signed-off-by: Glenn Washburn <development@efficientek.com>
> ---
>  tests/ahci_test.in          |  8 ++++----
>  tests/cdboot_test.in        |  8 ++++----
>  tests/core_compress_test.in |  2 +-
>  tests/ehci_test.in          |  8 ++++----
>  tests/fddboot_test.in       | 16 ++++++++--------
>  tests/grub_cmd_date.in      |  2 +-
>  tests/grub_cmd_set_date.in  |  6 +++---
>  tests/hddboot_test.in       |  6 +++---
>  tests/netboot_test.in       | 12 ++++++------
>  tests/ohci_test.in          |  8 ++++----
>  tests/partmap_test.in       |  8 ++++----
>  tests/pata_test.in          |  6 +++---
>  tests/pseries_test.in       |  2 +-
>  tests/uhci_test.in          |  8 ++++----
>  14 files changed, 50 insertions(+), 50 deletions(-)

I missed that the test for grub_cmd_sleep.in on sparc64-ieee1275 should
be skipped as well. Its a trivial change and will add it on the next
revision of this series after review.

Glenn

Re: [PATCH v2 2/8] tests: Fix partmap_test for arm*-efi, disk numbering has changed

[Daniel Kiper]

On Wed, Aug 25, 2021 at 02:03:56AM -0500, Glenn Washburn wrote:
> Perhaps using a newer UEFI firmware is the reason for the created test disk
> showing up as hd2 instead of hd3.
>
> Signed-off-by: Glenn Washburn <development@efficientek.com>

Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>

Daniel

Re: [PATCH v2 3/8] tests: When checking squashfs fstime, use superblock last modified time

[Daniel Kiper]

On Wed, Aug 25, 2021 at 02:03:57AM -0500, Glenn Washburn wrote:
> Currently, the filesystem timestamp check in grub-fs-tester uses the
> squashfs image file's last modified timestamp and checks to see if that
> time stamp is within 3 seconds of the superblock timestamp as determined by
> grub. The image file's timestamp could be more than 3 seconds off if
> mksquashfs takes more than 3 seconds to generate the image, as is the case
> on a virtual machine. Instead use squashfs tools to get the filesystem
> timestamp directly.
>
> Signed-off-by: Glenn Washburn <development@efficientek.com>

Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>

Daniel

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

[Daniel Kiper]

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?

>     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

Re: [PATCH v2 5/8] tests: Make setup errors in grub-fs-tester hard errors

[Daniel Kiper]

On Wed, Aug 25, 2021 at 02:03:59AM -0500, Glenn Washburn wrote:
> When a test program fails because it failed to setup the test properly, this
> does not indicate a failure in what is attempting to be tested because the
> test is never run. So exit with a hard error exit status to note this
> difference. This will allow easier detection of tests that are not actually
> being run and those that are really failing.
>
> Signed-off-by: Glenn Washburn <development@efficientek.com>

Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>

Daniel

Re: [PATCH v2 6/8] tests: A failure of mktemp should cause the test script to exit with code 99

[Daniel Kiper]

On Wed, Aug 25, 2021 at 02:04:00AM -0500, Glenn Washburn wrote:
> A test exiting with code 99 means that there was an error in the test itself
> and not a failure in the thing being tested (also known as a hard error).
>
> Signed-off-by: Glenn Washburn <development@efficientek.com>

Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>

Daniel

Re: [PATCH v2 7/8] tests: Exit with skipped exit code when test not performed

[Daniel Kiper]

On Fri, Sep 17, 2021 at 09:42:49PM +0000, Glenn Washburn wrote:
> On Wed, 25 Aug 2021 02:04:01 -0500
> Glenn Washburn <development@efficientek.com> wrote:
>
> > These tests were not performed and therefore did not pass, nor fail.
> > This fixes misleading test exit code where, for instance, the
> > pseries_test will pass on i386-pc, which is not a pseries
> > architecture.
> >
> > Signed-off-by: Glenn Washburn <development@efficientek.com>
> > ---
> >  tests/ahci_test.in          |  8 ++++----
> >  tests/cdboot_test.in        |  8 ++++----
> >  tests/core_compress_test.in |  2 +-
> >  tests/ehci_test.in          |  8 ++++----
> >  tests/fddboot_test.in       | 16 ++++++++--------
> >  tests/grub_cmd_date.in      |  2 +-
> >  tests/grub_cmd_set_date.in  |  6 +++---
> >  tests/hddboot_test.in       |  6 +++---
> >  tests/netboot_test.in       | 12 ++++++------
> >  tests/ohci_test.in          |  8 ++++----
> >  tests/partmap_test.in       |  8 ++++----
> >  tests/pata_test.in          |  6 +++---
> >  tests/pseries_test.in       |  2 +-
> >  tests/uhci_test.in          |  8 ++++----
> >  14 files changed, 50 insertions(+), 50 deletions(-)
>
> I missed that the test for grub_cmd_sleep.in on sparc64-ieee1275 should
> be skipped as well. Its a trivial change and will add it on the next
> revision of this series after review.

You can add "Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>" to
this patch too...

Daniel

Re: [PATCH v2 8/8] tests: Use @BUILD_SHEBANG@ autoconf var instead of literal shell

[Daniel Kiper]

On Wed, Aug 25, 2021 at 02:04:02AM -0500, Glenn Washburn wrote:
> This bring this test in line with the rest of the test scripts.
>
> Signed-off-by: Glenn Washburn <development@efficientek.com>

Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>

Daniel

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

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

[Daniel Kiper]

On Wed, Oct 06, 2021 at 03:05:57PM -0500, Glenn Washburn wrote:
> 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'

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

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

[Glenn Washburn]

On Thu, 7 Oct 2021 14:37:16 +0200
Daniel Kiper <dkiper@net-space.pl> wrote:

> On Wed, Oct 06, 2021 at 03:05:57PM -0500, Glenn Washburn wrote:
> > 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'
> 
> OK, AIUI error code is occluded by '['/test. Right?

Yes, although its not the fault of test. I believe it would be the case
for any "simple command" which uses subshells and one of the subshells
is returning an error exit code. For simple commands, only the exit
code of the primary command is checked for an error code.

For example:

  bash -e -c 'echo "$(echo XXX | ( cat; false ))"; echo ret=$?'

and

  bash -e -c 'cat <<<"$(echo XXX | ( cat; false ))"; echo ret=$?'

> 
> > 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

Ok, I'll split it up.

Glenn