Re: [Devel] [PATCH hci-8.1 4/6] selftests/zram: fail when a step fails

[no-reply@sashiko.virtuozzo.com]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] selftests/zram: zram_load() and zram_cleanup() failures still yield PASS
--

commit 0339f0cb6294df6e9df86072b01edefd848cf680
Author: Eva Kurchatova <eva.kurchatova@virtuozzo.com>

selftests/zram: fail when a step fails

This patch makes the zram selftest library set ERR_CODE when a step fails.
It makes zram01 and zram02 exit 1 on failure, and makes zram.sh exit with
the combined result of both tests.

Link: https://virtuozzo.atlassian.net/browse/VSTOR-134205

> diff --git a/tools/testing/selftests/zram/zram_lib.sh b/tools/testing/selftests/zram/zram_lib.sh
> --- a/tools/testing/selftests/zram/zram_lib.sh
> +++ b/tools/testing/selftests/zram/zram_lib.sh

[ ... ]

> @@ -131,12 +131,14 @@ zram_max_streams()
>  	for max_s in $zram_max_streams; do
>  		local sys_path="/sys/block/zram${i}/max_comp_streams"
>  		echo $max_s > $sys_path || \
> -			echo "FAIL failed to set '$max_s' to $sys_path"
> +			{ echo "FAIL failed to set '$max_s' to $sys_path"; ERR_CODE=-1; }
>  		sleep 1
>  		local max_streams=$(cat $sys_path)
>  
> -		[ "$max_s" -ne "$max_streams" ] && \
> -			echo "FAIL can't set max_streams '$max_s', get $max_stream"
> +		if [ "$max_s" -ne "$max_streams" ]; then
> +			echo "FAIL can't set max_streams '$max_s', get $max_streams"
> +			ERR_CODE=-1
> +		fi

[Severity: Low]
This isn't a regression in the new code, but is the max_streams check
complete?

If cat returns an empty or non-numeric value, the test
[ "$max_s" -ne "$max_streams" ] fails with a shell error (status 2).  The
if statement treats that as false, so the FAIL branch is skipped and
ERR_CODE stays unset in zram_max_streams().

This is caught only if the preceding echo to max_comp_streams also failed.

[ ... ]

> @@ -261,7 +267,7 @@ zram_mount()
>  		echo "mount /dev/zram$i"
>  		mkdir zram$i
>  		mount /dev/zram$i zram$i > /dev/null || \
> -			echo "FAIL mount /dev/zram$i failed"
> +			{ echo "FAIL mount /dev/zram$i failed"; ERR_CODE=-1; }
>  		dev_mounted=$i
>  	done

[Severity: Low]
The commit message says "A failed step now fails its test", but some library
steps still do not record failures in ERR_CODE.  Can a zram test still print
PASS and exit 0 after a library step failed?

In zram_load(), the result of these commands is not checked:

    cat /sys/class/zram-control/hot_add > /dev/null
    ...
    modprobe zram num_devices=$dev_num

A failure there is caught only indirectly, if a later sysfs write fails.

In zram_cleanup(), the result of these commands is not checked:

    swapoff /dev/zram$i
    ...
    umount /dev/zram$i
    ...
    echo 1 > /sys/block/zram${i}/reset

zram_cleanup() runs before the verdict in zram01 and zram02.  A teardown
failure that leaves swap or a mount behind therefore still ends in PASS.

Should these steps set ERR_CODE=-1 on failure as well, or should the commit
message say that load and cleanup are excluded?

-- 
Sashiko AI review · https://sashiko.vzint.dev/#/patchset/20261026124139.606106-1-eva.kurchatova@virtuozzo.com?part=4

_______________________________________________
Devel mailing list
Devel@openvz.org
https://lists.openvz.org/mailman/listinfo/devel