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

[Thomas Schmitt via Grub-devel <grub-devel@gnu.org>]

Hi,

Glenn Washburn wrote:
> [...] grub-shell-luks-tester cleans up after
> itself, if it returns success. grub_cmd_cryptomount has a test that
> expects failure. But grub-shell-luks-tester doesn't know that this is
> an expected failure and should cleanup and grub_cmd_cryptomount doesn't
> ever cleanup after grub-shell-luks-tester. Perhaps
> grub-shell-luks-tester should be passed a parameter to indicate
> expected failure (eg. --xfail). Have any other ideas?

I agree to the idea of an option to invert the effect of
[ "$RET:-1" -eq 0 ] in cleanup() of tests/util/grub-shell-luks-tester.in .

Assuming variable "xfail" is be set to a non-empty string exactly if
argument "--xfail" is given, i'd replace:

    if [ -z "$debug" ] && [ "${RET:-1}" -eq 0 ]; then
        rm -rf "$lukstestdir" || :
    fi

by:

    if [ -z "$debug" ]; then
        if [ -n "$xfail" ]; then
            if [ "${RET:-0}" -ne 0 ]; then
                rm -rf "$lukstestdir" || :
            fi
        else
            if [ "${RET:-1}" -eq 0 ]; then
                rm -rf "$lukstestdir" || :
            fi
        fi
    fi

Equivalent, but heavily economizing on line count would be:

    if [ -z "$debug" ] && [ -n "$xfail" ] && [ "${RET:-0}" -ne 0 ]; then
        rm -rf "$lukstestdir" || :
    elif [ -z "$debug" ] && [ -z "$xfail" ] && [ "${RET:-1}" -eq 0 ]; then
        rm -rf "$lukstestdir" || :
    fi

I tested both code pieces in a dry-run script with the 12 variations of
debug={"", "1"} , xfail={"", "1"} , RET={undefined, "0", "1"} .
Decision for removal happened only with:
  (debug="", xfail="", RET="0")
  (debug="", xfail="1", RET="1")

The first code performs less []-expressions and seems clearer to me at
the price of doubled line count.


Have a nice day :)

Thomas


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