[PATCH v2 0/6] Various test fixes proposed by Thomas Schmitt

[PATCH v2 0/6] Various test fixes proposed by Thomas Schmitt

[Glenn Washburn]

Thomas Schmitt proposed variations on these fixes[1]. I've broken his
patch into several patches with improvements.

Glenn

[1] https://lore.kernel.org/all/9956308756800479343@scdbackup.webframe.org/

v2:
 * Add patch #2 to allow grub-shell-luks-tester to cleanup properly on expected
   failure.
 * Add patch #5 to remove empty directories left by the cryptomount tests.

Glenn Washburn (5):
  tests/util/grub-shell-luks-tester: Add missing line to create RET
    variable in cleanup
  tests: Cleanup generated files on expected failure in
    grub_cmd_cryptomount
  tests/grub_cmd_cryptomount: Cleanup the cryptsetup script unless debug
    is enabled
  tests/grub_cmd_cryptomount: Default TMPDIR to /tmp
  tests/util/grub-shell: Remove the work directory on successful run and
    debug is not on

Thomas Schmitt (1):
  tests/grub_cmd_cryptomount: Remove temporary directories if successful
    and debug is not on

 tests/grub_cmd_cryptomount.in        | 18 ++++++++++++++++--
 tests/util/grub-shell-luks-tester.in |  7 ++++++-
 tests/util/grub-shell.in             |  1 +
 3 files changed, 23 insertions(+), 3 deletions(-)

Range-diff against v1:
1:  50ea280aa138 = 1:  e686c1848ab1 tests/util/grub-shell-luks-tester: Add missing line to create RET variable in cleanup
-:  ------------ > 2:  f94ebf9f040e tests: Cleanup generated files on expected failure in grub_cmd_cryptomount
2:  4d8fca393bfd ! 3:  1d1faee928cf tests: Cleaup the cryptsetup script in grub_cmd_cryptomount unless debug is enabled
    @@ Metadata
     Author: Glenn Washburn <development@efficientek.com>
     
      ## Commit message ##
    -    tests: Cleaup the cryptsetup script in grub_cmd_cryptomount unless debug is enabled
    +    tests/grub_cmd_cryptomount: Cleanup the cryptsetup script unless debug is enabled
     
         This fixes an issue where the grub_cmd_cryptomount test leaves a file
         with an ambiguous name in the / directory when TMPDIR is not set.
3:  e32175b06be6 ! 4:  5f1b424919b2 tests: Default TMPDIR to /tmp in grub_cmd_cryptomount tests
    @@ Metadata
     Author: Glenn Washburn <development@efficientek.com>
     
      ## Commit message ##
    -    tests: Default TMPDIR to /tmp in grub_cmd_cryptomount tests
    +    tests/grub_cmd_cryptomount: Default TMPDIR to /tmp
     
         This fixes behavior where grub_cmd_cryptomount temporary files, which are
         some times not cleaned up, are left in the / directory. Set TMPDIR if your
    @@ tests/grub_cmd_cryptomount.in: _testcase() {
     +    TMPDIR=${_TMPDIR}/`echo -n "$(date +%s).${LOGPREFIX}" | sed -e 's,[ /],_,g' -e 's,:$,,g'`
     +    export TMPDIR
          mkdir -p "$TMPDIR"
    +     set -- "$@" $([ "${EXPECTEDRES}" -eq 1 ] && echo "--xfail")
      
    -     output=`"$@" 2>&1` || res=$?
-:  ------------ > 5:  779233064126 tests/grub_cmd_cryptomount: Remove temporary directories if successful and debug is not on
4:  6ef75566e50b = 6:  e9fc44e07539 tests/util/grub-shell: Remove the work directory on successful run and debug is not on
-- 
2.34.1


_______________________________________________
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

[PATCH v2 1/6] tests/util/grub-shell-luks-tester: Add missing line to create RET variable in cleanup

[Glenn Washburn]

Set the RET variable to the exit status of the script, as was assumed in the
cleanup() function.

Reported-by: Thomas Schmitt <scdbackup@gmx.net>
Signed-off-by: Glenn Washburn <development@efficientek.com>
---
 tests/util/grub-shell-luks-tester.in | 1 +
 1 file changed, 1 insertion(+)

diff --git a/tests/util/grub-shell-luks-tester.in b/tests/util/grub-shell-luks-tester.in
index b2a8a91b41f2..855ad39c19c3 100644
--- a/tests/util/grub-shell-luks-tester.in
+++ b/tests/util/grub-shell-luks-tester.in
@@ -143,6 +143,7 @@ fi
 
 # Make sure that the dm-crypto device is shutdown
 cleanup() {
+    RET=$?
     if [ -e "$luksdev" ]; then
         cryptsetup close "$luksdev"
     fi
-- 
2.34.1


_______________________________________________
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

[PATCH v2 2/6] tests: Cleanup generated files on expected failure in grub_cmd_cryptomount

[Glenn Washburn]

grub-shell-luks-tester only cleans up generated files when the test it
runs returns success. Sometimes tests are run that should fail. Add a
--xfail argument to grub-shell-luks-tester and pass it from
grub_cmd_cryptomount when invoking a test that is expected to fail.

Reported-by: Thomas Schmitt <scdbackup@gmx.net>
Signed-off-by: Glenn Washburn <development@efficientek.com>
---
 tests/grub_cmd_cryptomount.in        | 1 +
 tests/util/grub-shell-luks-tester.in | 6 +++++-
 2 files changed, 6 insertions(+), 1 deletion(-)

diff --git a/tests/grub_cmd_cryptomount.in b/tests/grub_cmd_cryptomount.in
index f4d8f35473d0..63ed8e51bec3 100644
--- a/tests/grub_cmd_cryptomount.in
+++ b/tests/grub_cmd_cryptomount.in
@@ -48,6 +48,7 @@ _testcase() {
     _TMPDIR=$TMPDIR
     TMPDIR=$TMPDIR/`echo -n "$(date +%s).$LOGPREFIX" | sed -e 's,[ /],_,g' -e 's,:$,,g'`
     mkdir -p "$TMPDIR"
+    set -- "$@" $([ "${EXPECTEDRES}" -eq 1 ] && echo "--xfail")
 
     output=`"$@" 2>&1` || res=$?
     TMPDIR=$_TMPDIR
diff --git a/tests/util/grub-shell-luks-tester.in b/tests/util/grub-shell-luks-tester.in
index 855ad39c19c3..abb0013e4387 100644
--- a/tests/util/grub-shell-luks-tester.in
+++ b/tests/util/grub-shell-luks-tester.in
@@ -36,6 +36,7 @@ keyfile=
 keyfile_offset=
 keyfile_size=
 KEYFILE_SIZE_MAX=4096
+expected_res=0
 
 debug="${GRUB_SHELL_LUKS_DEFAULT_DEBUG:-$GRUB_TEST_DEFAULT_DEBUG}"
 GRUB_SHELL_LUKS_TIMEOUT=${GRUB_SHELL_LUKS_TIMEOUT:-${GRUB_SHELL_DEFAULT_TIMEOUT:-600s}}
@@ -58,6 +59,7 @@ running in a QEMU instance.
   --detached-header       Use a detached header
   --keyfile[=FILE]        Use a randomly generated key file of size $KEYFILE_SIZE_MAX if not
                           given a FILE to use as the key file.
+  --xfail                 Expected failure, so cleanup as if on success
 
 $0 creates a LUKS disk with cryptsetup, then verify that it is accessible by
 grub running in a QEMU instance.
@@ -118,6 +120,8 @@ for option in "$@"; do
     --disksize=*)
         qs=`echo "$option" | sed -e 's/--disksize=//'`
         disksize="$qs" ;;
+    --xfail)
+        expected_res=1 ;;
     -*)
 	echo "Unrecognized option \`$option'" 1>&2
 	usage
@@ -147,7 +151,7 @@ cleanup() {
     if [ -e "$luksdev" ]; then
         cryptsetup close "$luksdev"
     fi
-    if [ -z "$debug" ] && [ "${RET:-1}" -eq 0 ]; then
+    if [ -z "$debug" ] && [ "$RET" -eq "$expected_res" ]; then
         rm -rf "$lukstestdir" || :
     fi
 }
-- 
2.34.1


_______________________________________________
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

[PATCH v2 3/6] tests/grub_cmd_cryptomount: Cleanup the cryptsetup script unless debug is enabled

[Glenn Washburn]

This fixes an issue where the grub_cmd_cryptomount test leaves a file
with an ambiguous name in the / directory when TMPDIR is not set.

Reported-by: Thomas Schmitt <scdbackup@gmx.net>
Signed-off-by: Glenn Washburn <development@efficientek.com>
---
 tests/grub_cmd_cryptomount.in | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/tests/grub_cmd_cryptomount.in b/tests/grub_cmd_cryptomount.in
index 63ed8e51bec3..2fba8a8e0e3a 100644
--- a/tests/grub_cmd_cryptomount.in
+++ b/tests/grub_cmd_cryptomount.in
@@ -37,6 +37,8 @@ fi
 
 COMMON_OPTS='${V:+--debug=$V} --cs-opts="--pbkdf-force-iterations 1000"'
 
+debug=${GRUB_SHELL_DEFAULT_DEBUG:-$GRUB_TEST_DEFAULT_DEBUG}
+
 _testcase() {
     local EXPECTEDRES=$1
     local LOGPREFIX=$2
@@ -183,4 +185,5 @@ eval testcase "'LUKS2 test with second key slot and first slot using different p
     @builddir@/grub-shell-luks-tester $LUKS2_COMMON_OPTS $COMMON_OPTS \
         "--cs-script='$csscript'"
 
+test -n "$debug" || rm "$csscript"
 exit 0
-- 
2.34.1


_______________________________________________
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

[PATCH v2 4/6] tests/grub_cmd_cryptomount: Default TMPDIR to /tmp

[Glenn Washburn]

This fixes behavior where grub_cmd_cryptomount temporary files, which are
some times not cleaned up, are left in the / directory. Set TMPDIR if your
system does not have /tmp or it can not be used for some reason.

Reported-by: Thomas Schmitt <scdbackup@gmx.net>
Signed-off-by: Glenn Washburn <development@efficientek.com>
---
 tests/grub_cmd_cryptomount.in | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/tests/grub_cmd_cryptomount.in b/tests/grub_cmd_cryptomount.in
index 2fba8a8e0e3a..8dc99127d39e 100644
--- a/tests/grub_cmd_cryptomount.in
+++ b/tests/grub_cmd_cryptomount.in
@@ -47,8 +47,9 @@ _testcase() {
     shift 2
 
     # Create a subdir in TMPDIR for each testcase
-    _TMPDIR=$TMPDIR
-    TMPDIR=$TMPDIR/`echo -n "$(date +%s).$LOGPREFIX" | sed -e 's,[ /],_,g' -e 's,:$,,g'`
+    _TMPDIR=${TMPDIR:-/tmp}
+    TMPDIR=${_TMPDIR}/`echo -n "$(date +%s).${LOGPREFIX}" | sed -e 's,[ /],_,g' -e 's,:$,,g'`
+    export TMPDIR
     mkdir -p "$TMPDIR"
     set -- "$@" $([ "${EXPECTEDRES}" -eq 1 ] && echo "--xfail")
 
-- 
2.34.1


_______________________________________________
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

[PATCH v2 5/6] tests/grub_cmd_cryptomount: Remove temporary directories if successful and debug is not on

[Glenn Washburn]

From: Thomas Schmitt <scdbackup@gmx.net>

grub_cmd_cryptomount creates a directory per subtest. If a subtest is
successful and debugging is not on, the directory should be empty. So it
can be deleted.

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

diff --git a/tests/grub_cmd_cryptomount.in b/tests/grub_cmd_cryptomount.in
index 8dc99127d39e..eaa187efad0a 100644
--- a/tests/grub_cmd_cryptomount.in
+++ b/tests/grub_cmd_cryptomount.in
@@ -54,6 +54,15 @@ _testcase() {
     set -- "$@" $([ "${EXPECTEDRES}" -eq 1 ] && echo "--xfail")
 
     output=`"$@" 2>&1` || res=$?
+    if [ -z "$debug" ]; then
+        if ! rmdir "$TMPDIR" >/dev/null 2>&1; then
+            echo
+            echo "Note: Temporary directory cannot be removed:"
+            echo "        $TMPDIR"
+            echo "      Please inspect and remove manually."
+            echo
+        fi
+    fi
     TMPDIR=$_TMPDIR
 
     if [ "$res" -eq "$EXPECTEDRES" ]; then
-- 
2.34.1


_______________________________________________
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

[PATCH v2 6/6] tests/util/grub-shell: Remove the work directory on successful run and debug is not on

[Glenn Washburn]

This removes alot of empty grub-shell working directories in the TMPDIR
directory.

Signed-off-by: Thomas Schmitt <scdbackup@gmx.net>
Signed-off-by: Glenn Washburn <development@efficientek.com>
---
 tests/util/grub-shell.in | 1 +
 1 file changed, 1 insertion(+)

diff --git a/tests/util/grub-shell.in b/tests/util/grub-shell.in
index 15c5f45a5c30..af3d31ef4609 100644
--- a/tests/util/grub-shell.in
+++ b/tests/util/grub-shell.in
@@ -715,6 +715,7 @@ test -n "$debug" || rm -f "${isofile}"
 test -n "$debug" || rm -rf "${rom_directory}"
 test -n "$debug" || rm -f "${tmpfile}" "${cfgfile}" "${goutfile}"
 test -n "$debug" || rm -f "$work_directory/run.sh"
+test -n "$debug" || rmdir "$work_directory"
 
 exit $ret
 
-- 
2.34.1


_______________________________________________
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

Re: [PATCH v2 0/6] Various test fixes proposed by Thomas Schmitt

[Thomas Schmitt via Grub-devel]

Hi,

Tested-by: Thomas Schmitt <scdbackup@gmx.net>

with remaining unrelated problems.

-----------------------------------------------------------------------
Test details:

The patch series applied by "git am" without problems to a branch cloned
from freshly pulled "master".

  make check TESTS=grub_cmd_cryptomount

produces

  # PASS:  1

No remaining files from that run are to see in / and /tmp.
During the "make check" i could see files appear and vanish in /tmp.


Then i ran the full "make check" (which lasts really long on a 4 GHz
Xeon with "top" reporting the system being mostly idle).

These failures did not happen in june 2024, when i ran "make check":

  FAIL: ntfs_test
  FAIL: zfs_test

Other failures which i already had in 2024 are:

  FAIL: hfs_test
  FAIL: luks1_test
  FAIL: luks2_test
  FAIL: grub_func_test

Some tests which failed in 2024 but don't any more:
  PASS: erofs_test
  PASS: exfat_test

(My mail in june 2024 with the list of test failures was
 Date: Thu, 20 Jun 2024 22:05:02 +0200
 Message-Id: <20240620200504.466683-1-scdbackup@gmx.net>
Regrettably http://lists.gnu.org/mailman/listinfo/grub-devel does not
work for me today. So i cannot give a link into the archive.)

Files which remained in /tmp:

  grub-fs-tester.20250304100244880329310.ntfs.JrW
  grub-fs-tester.20250304100732808627445.vfat12.AKi
  grub-fs-tester.20250304102111753946043.zfs_zstd.HeI
  grub-fs-tester.20250304102132560362937.luks1.szH
  grub-fs-tester.20250304102140126776818.luks2.eTV
  tmp.3t4AGcksU8
  tmp.5ch8QxZT6p
  tmp.6msLyaqLuq
  tmp.CeFRqmCNlf
  tmp.m60l7N0wuj

tmp.CeFRqmCNlf looks like a GRUB configuration.
tmp.m60l7N0wuj looks like the result of GRUB's "ls" command, possibly
with a grub-mkrescue ISO in (cd0).
The contents of the others give me no clue.


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

I also reviewed the patches.
(Give me a note if i shall add the tags by single mails to the patch
mails.)


> [PATCH v2 1/6] tests/util/grub-shell-luks-tester: Add missing line
>    to create RET variable in cleanup

Reviewed-by: Thomas Schmitt <scdbackup@gmx.net>


> [PATCH v2 2/6] tests: Cleanup generated files on expected failure
>    in grub_cmd_cryptomount

Reviewed-by: Thomas Schmitt <scdbackup@gmx.net>
(Deeply impressed by the "set --" gesture)


> [PATCH v2 3/6] tests/grub_cmd_cryptomount: Cleanup the cryptsetup
>    script unless debug is enabled

Reviewed-by: Thomas Schmitt <scdbackup@gmx.net>


> [PATCH v2 4/6] tests/grub_cmd_cryptomount: Default TMPDIR to /tmp

Reviewed-by: Thomas Schmitt <scdbackup@gmx.net>


> [PATCH v2 5/6] tests/grub_cmd_cryptomount: Remove temporary
>    directories if successful and debug is not on

Signed-off-by: Thomas Schmitt <scdbackup@gmx.net>


> [PATCH v2 6/6] tests/util/grub-shell: Remove the work directory on
>    successful run and debug is not on

(Already contains Signed-off-by: Thomas Schmitt <scdbackup@gmx.net> )

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

Have a nice day :)

Thomas


_______________________________________________
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

Re: [PATCH v2 0/6] Various test fixes proposed by Thomas Schmitt

[Glenn Washburn]

On Tue, 04 Mar 2025 10:56:15 +0100
"Thomas Schmitt" <scdbackup@gmx.net> wrote:

> Hi,
> 
> Tested-by: Thomas Schmitt <scdbackup@gmx.net>
> 
> with remaining unrelated problems.
> 
> -----------------------------------------------------------------------
> Test details:
> 
> The patch series applied by "git am" without problems to a branch cloned
> from freshly pulled "master".
> 
>   make check TESTS=grub_cmd_cryptomount
> 
> produces
> 
>   # PASS:  1
> 
> No remaining files from that run are to see in / and /tmp.
> During the "make check" i could see files appear and vanish in /tmp.
> 
> 
> Then i ran the full "make check" (which lasts really long on a 4 GHz
> Xeon with "top" reporting the system being mostly idle).
> 
> These failures did not happen in june 2024, when i ran "make check":
> 
>   FAIL: ntfs_test

This was introduced by the security fixes and there's a patch on the
list to fix this.

>   FAIL: zfs_test

I've not looked into this, but I'm seeing it fail too.

> 
> Other failures which i already had in 2024 are:
> 
>   FAIL: hfs_test

This should fail on anything but now ancient debian systems. This is
due to the package having a custom patch that added functionality and
now doesn't, IIRC.

>   FAIL: luks1_test
>   FAIL: luks2_test

These should not be failing.

>   FAIL: grub_func_test

This is expected, and will be for the foreseeable future, unfortunately.

Glenn

> 
> Some tests which failed in 2024 but don't any more:
>   PASS: erofs_test
>   PASS: exfat_test
> 
> (My mail in june 2024 with the list of test failures was
>  Date: Thu, 20 Jun 2024 22:05:02 +0200
>  Message-Id: <20240620200504.466683-1-scdbackup@gmx.net>
> Regrettably http://lists.gnu.org/mailman/listinfo/grub-devel does not
> work for me today. So i cannot give a link into the archive.)
> 
> Files which remained in /tmp:
> 
>   grub-fs-tester.20250304100244880329310.ntfs.JrW
>   grub-fs-tester.20250304100732808627445.vfat12.AKi
>   grub-fs-tester.20250304102111753946043.zfs_zstd.HeI
>   grub-fs-tester.20250304102132560362937.luks1.szH
>   grub-fs-tester.20250304102140126776818.luks2.eTV
>   tmp.3t4AGcksU8
>   tmp.5ch8QxZT6p
>   tmp.6msLyaqLuq
>   tmp.CeFRqmCNlf
>   tmp.m60l7N0wuj
> 
> tmp.CeFRqmCNlf looks like a GRUB configuration.
> tmp.m60l7N0wuj looks like the result of GRUB's "ls" command, possibly
> with a grub-mkrescue ISO in (cd0).
> The contents of the others give me no clue.
> 
> 
> ----------------------------------------------------------------------
> 
> I also reviewed the patches.
> (Give me a note if i shall add the tags by single mails to the patch
> mails.)
> 
> 
> > [PATCH v2 1/6] tests/util/grub-shell-luks-tester: Add missing line
> >    to create RET variable in cleanup
> 
> Reviewed-by: Thomas Schmitt <scdbackup@gmx.net>
> 
> 
> > [PATCH v2 2/6] tests: Cleanup generated files on expected failure
> >    in grub_cmd_cryptomount
> 
> Reviewed-by: Thomas Schmitt <scdbackup@gmx.net>
> (Deeply impressed by the "set --" gesture)
> 
> 
> > [PATCH v2 3/6] tests/grub_cmd_cryptomount: Cleanup the cryptsetup
> >    script unless debug is enabled
> 
> Reviewed-by: Thomas Schmitt <scdbackup@gmx.net>
> 
> 
> > [PATCH v2 4/6] tests/grub_cmd_cryptomount: Default TMPDIR to /tmp
> 
> Reviewed-by: Thomas Schmitt <scdbackup@gmx.net>
> 
> 
> > [PATCH v2 5/6] tests/grub_cmd_cryptomount: Remove temporary
> >    directories if successful and debug is not on
> 
> Signed-off-by: Thomas Schmitt <scdbackup@gmx.net>
> 
> 
> > [PATCH v2 6/6] tests/util/grub-shell: Remove the work directory on
> >    successful run and debug is not on
> 
> (Already contains Signed-off-by: Thomas Schmitt <scdbackup@gmx.net> )
> 
> ----------------------------------------------------------------------
> 
> Have a nice day :)
> 
> Thomas
> 

_______________________________________________
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

Re: [PATCH v2 0/6] Various test fixes proposed by Thomas Schmitt

[Daniel Kiper]

On Mon, Mar 03, 2025 at 02:12:00AM -0600, Glenn Washburn wrote:
> Thomas Schmitt proposed variations on these fixes[1]. I've broken his
> patch into several patches with improvements.
>
> Glenn
>
> [1] https://lore.kernel.org/all/9956308756800479343@scdbackup.webframe.org/
>
> v2:
>  * Add patch #2 to allow grub-shell-luks-tester to cleanup properly on expected
>    failure.
>  * Add patch #5 to remove empty directories left by the cryptomount tests.
>
> Glenn Washburn (5):
>   tests/util/grub-shell-luks-tester: Add missing line to create RET
>     variable in cleanup
>   tests: Cleanup generated files on expected failure in
>     grub_cmd_cryptomount
>   tests/grub_cmd_cryptomount: Cleanup the cryptsetup script unless debug
>     is enabled
>   tests/grub_cmd_cryptomount: Default TMPDIR to /tmp
>   tests/util/grub-shell: Remove the work directory on successful run and
>     debug is not on
>
> Thomas Schmitt (1):
>   tests/grub_cmd_cryptomount: Remove temporary directories if successful
>     and debug is not on

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

Daniel

_______________________________________________
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel