Re: [PATCH] tests: Let grub_cmd_cryptomount by default operate in /tmp rather than in /

[Thomas Schmitt]

Hi,

i post this as base of discussion, not yet as sincere patch proposal.

It seems desirable to bundle the temporary data of each particular test
in tests/grub_cmd_cryptomount.in in a single directory, as it seems
intended by the code but currently is not achieved.
So i propose to default TMPDIR to "/tmp" and to export it after it
was augmented by a test specific subdirectory component.

In order to let tests/grub_cmd_cryptomount.in finally remove this work
directory, we can either do "rm -rf" or let the subordinate workers clean
up their work directories which they create as subdirectories.
I am in favor of the latter.

This implies a possibly intrusive change in
  tests/util/grub-shell-luks-tester.in
where i propose to drop the evaluation of a variable named RET, which i
cannot find to be set or used anywhere in the git repo tree.
Another change would be in
  tests/util/grub-shell.in
where i propose to finally remove the work directory.
In order to avoid large colateral damage, i propose rmdir rather than
"rm -rf". (If debris remains in the work directory than this should be
investigated by the user.)

Finally i propose to remove a regular file which the last test of
grub_cmd_cryptomount.in currently leaves in /tmp.

I add a diff to the code which now works for me and leaves no debris
in /tmp after
  make check TESTS=grub_cmd_cryptomount
In case somebody finds parts of it useful:
  Signed-off-by: Thomas Schmitt <scdbackup@gmx.net>


Have a nice day :)

Thomas

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

diff --git a/tests/grub_cmd_cryptomount.in b/tests/grub_cmd_cryptomount.in
index f4d8f3547..5ce97fa06 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
@@ -46,10 +48,22 @@ _testcase() {

     # 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}"/`echo -n "$(date +%s).$LOGPREFIX" | sed -e 's,[ 
/],_,g' -e 's,:$,,g'`
+    export TMPDIR
     mkdir -p "$TMPDIR"

     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
+    unset TMPDIR
     TMPDIR=$_TMPDIR

     if [ "$res" -eq "$EXPECTEDRES" ]; then
@@ -182,4 +196,6 @@ 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
diff --git a/tests/util/grub-shell-luks-tester.in 
b/tests/util/grub-shell-luks-tester.in
index b2a8a91b4..20ebecacf 100644
--- a/tests/util/grub-shell-luks-tester.in
+++ b/tests/util/grub-shell-luks-tester.in
@@ -146,7 +146,7 @@ cleanup() {
     if [ -e "$luksdev" ]; then
         cryptsetup close "$luksdev"
     fi
-    if [ -z "$debug" ] && [ "${RET:-1}" -eq 0 ]; then
+    if [ -z "$debug" ] ; then
         rm -rf "$lukstestdir" || :
     fi
 }
diff --git a/tests/util/grub-shell.in b/tests/util/grub-shell.in
index ae5f711fe..98f7e7394 100644
--- a/tests/util/grub-shell.in
+++ b/tests/util/grub-shell.in
@@ -711,6 +711,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