All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH blktests v2] common, tests: Support printing multiple skip reasons
@ 2022-07-12  8:21 lizhijian
  2022-07-12 21:26 ` Bart Van Assche
  2022-07-13 10:41 ` Shinichiro Kawasaki
  0 siblings, 2 replies; 4+ messages in thread
From: lizhijian @ 2022-07-12  8:21 UTC (permalink / raw)
  To: linux-block, shinichiro.kawasaki; +Cc: bvanassche, lizhijian

Some test cases or test groups have rather large number of test
run requirements and then they may have multiple skip reasons. However,
blktests can report only single skip reason. To know all of the skip
reasons, we need to repeat skip reason resolution and blktests run.
This is a troublesome work.

In this patch, we add skip reasons to SKIP_REASONS array, then all of
the skip reasons will be printed by iterating SKIP_REASONS at one shot
run.

Most of the works are done by following script:

sed -i 's/SKIP_REASON/SKIP_REASONS/' $(git ls-files)
git grep -h 'SKIP_REASONS=' | awk -F'SKIP_REASONS=' '{print $2}' | sort | uniq |
while read -r r
do
	s="SKIP_REASONS=$r"
	f=$(git grep -l "$s")
	sed -i "s@$s@SKIP_REASONS+=($r)@" $f
done

Signed-off-by: Li Zhijian <lizhijian@fujitsu.com>
---
V3: update description in new
---
 check                      | 27 ++++++++++++--------
 common/cgroup              |  6 ++---
 common/cpuhotplug          |  2 +-
 common/fio                 |  4 +--
 common/iopoll              | 10 ++++----
 common/multipath-over-rdma |  4 +--
 common/null_blk            |  2 +-
 common/rc                  | 50 +++++++++++++++++++-------------------
 common/shellcheck          |  2 +-
 new                        | 50 ++++++++++++++++++++------------------
 tests/block/030            |  2 +-
 tests/loop/rc              |  2 +-
 tests/meta/007             |  2 +-
 tests/meta/008             |  2 +-
 tests/meta/013             |  2 +-
 tests/meta/014             |  2 +-
 tests/meta/015             |  2 +-
 tests/meta/rc              |  4 +--
 tests/nbd/rc               |  4 +--
 tests/nvme/rc              | 10 ++++----
 tests/nvmeof-mp/rc         |  7 +++---
 tests/scsi/rc              |  4 +--
 tests/srp/012              |  2 +-
 tests/srp/rc               |  6 ++---
 tests/zbd/004              |  2 +-
 tests/zbd/rc               |  4 +--
 26 files changed, 111 insertions(+), 103 deletions(-)

diff --git a/check b/check
index a0c27d92c8fe..e6c321c388cf 100755
--- a/check
+++ b/check
@@ -225,9 +225,16 @@ _output_status() {
 	echo "]"
 }
 
+_output_skip_reasons()
+{
+	for reason in "${SKIP_REASONS[@]}"; do
+		echo "    $reason"
+	done
+}
+
 _output_notrun() {
 	_output_status "$1" "not run"
-	echo "    $SKIP_REASON"
+	_output_skip_reasons
 }
 
 _output_last_test_run() {
@@ -288,7 +295,7 @@ _output_test_run() {
 	} | column -t -s $'\t'
 
 	if [[ $status = "not run" ]]; then
-		echo "    $SKIP_REASON"
+		_output_skip_reasons
 	fi
 }
 
@@ -343,7 +350,7 @@ _call_test() {
 	# Remove leftovers from last time.
 	rm -f "${seqres}" "${seqres}."*
 
-	if [[ -v SKIP_REASON ]]; then
+	if [[ -v SKIP_REASONS ]]; then
 		TEST_RUN["status"]="not run"
 	else
 		if [[ -w /dev/kmsg ]]; then
@@ -371,7 +378,7 @@ _call_test() {
 
 		_cleanup
 
-		if [[ -v SKIP_REASON ]]; then
+		if [[ -v SKIP_REASONS ]]; then
 			TEST_RUN["status"]="not run"
 		elif ! diff "tests/${TEST_NAME}.out" "${seqres}.out" >/dev/null; then
 			mv "${seqres}.out" "${seqres}.out.bad"
@@ -493,10 +500,10 @@ _run_test() {
 			TEST_DEV_PART_SYSFS="${TEST_DEV_PART_SYSFS_DIRS["$TEST_DEV"]}"
 
 			local unset_skip_reason=0
-			if [[ ! -v SKIP_REASON ]]; then
+			if [[ ! -v SKIP_REASONS ]]; then
 				unset_skip_reason=1
 				if (( !CAN_BE_ZONED )) && _test_dev_is_zoned; then
-					SKIP_REASON="${TEST_DEV} is a zoned block device"
+					SKIP_REASONS+=("${TEST_DEV} is a zoned block device")
 				elif declare -fF device_requires >/dev/null; then
 					device_requires
 				fi
@@ -506,7 +513,7 @@ _run_test() {
 				ret=1
 			fi
 			if (( unset_skip_reason )); then
-				unset SKIP_REASON
+				unset SKIP_REASONS
 			fi
 		done
 
@@ -530,7 +537,7 @@ _run_group() {
 
 	if declare -fF group_requires >/dev/null; then
 		group_requires
-		if [[ -v SKIP_REASON ]]; then
+		if [[ -v SKIP_REASONS ]]; then
 			_output_notrun "${group}/***"
 			return 0
 		fi
@@ -544,10 +551,10 @@ _run_group() {
 			# shellcheck disable=SC2034
 			TEST_DEV_PART_SYSFS="${TEST_DEV_PART_SYSFS_DIRS["$TEST_DEV"]}"
 			group_device_requires
-			if [[ -v SKIP_REASON ]]; then
+			if [[ -v SKIP_REASONS ]]; then
 				_output_notrun "${group}/*** => $(basename "$TEST_DEV")"
 				unset "TEST_DEVS[$i]"
-				unset SKIP_REASON
+				unset SKIP_REASONS
 			fi
 		done
 		# Fix the array indices.
diff --git a/common/cgroup b/common/cgroup
index c34bffd5d299..38511f0319d5 100644
--- a/common/cgroup
+++ b/common/cgroup
@@ -31,7 +31,7 @@ _exit_cgroup2()
 _have_cgroup2()
 {
 	if [[ -z $(_cgroup2_base_dir) ]]; then
-		SKIP_REASON="cgroup2 is not mounted"
+		SKIP_REASONS+=("cgroup2 is not mounted")
 		return 1
 	fi
 	return 0
@@ -46,7 +46,7 @@ _have_cgroup2_controller()
 	dir="$(_cgroup2_base_dir)"
 
 	if ! grep -q "$controller" "$dir/cgroup.controllers"; then
-		SKIP_REASON="no support for $controller cgroup controller; if it is enabled, you may need to boot with cgroup_no_v1=$controller"
+		SKIP_REASONS+=("no support for $controller cgroup controller; if it is enabled, you may need to boot with cgroup_no_v1=$controller")
 		return 1
 	fi
 }
@@ -63,7 +63,7 @@ _have_cgroup2_controller_file()
 	echo "+$controller" > "$dir/cgroup.subtree_control"
 	if [[ ! -f $dir/blktests/$file ]]; then
 		rmdir "$dir/blktests"
-		SKIP_REASON="cgroup file $file doesn't exist"
+		SKIP_REASONS+=("cgroup file $file doesn't exist")
 		return 1
 	fi
 	rmdir "$dir/blktests"
diff --git a/common/cpuhotplug b/common/cpuhotplug
index d3aabfa75831..7facd0de955e 100644
--- a/common/cpuhotplug
+++ b/common/cpuhotplug
@@ -24,7 +24,7 @@ _have_cpu_hotplug() {
 	done
 
 	if [[ ${#ALL_CPUS[@]} -eq 1 || ${#HOTPLUGGABLE_CPUS[@]} -eq 0 ]]; then
-		SKIP_REASON="CPU hotplugging is not supported"
+		SKIP_REASONS+=("CPU hotplugging is not supported")
 		return 1
 	fi
 	return 0
diff --git a/common/fio b/common/fio
index 94c65c107a14..bed76d555b2a 100644
--- a/common/fio
+++ b/common/fio
@@ -11,7 +11,7 @@ _have_fio() {
 		return 1
 	fi
 	if ! fio --parse-only --terse-version=4 >/dev/null 2>&1; then
-		SKIP_REASON="Fio version too old (does not support --terse-version=4)"
+		SKIP_REASONS+=("Fio version too old (does not support --terse-version=4)")
 		return 1
 	fi
 	return 0
@@ -20,7 +20,7 @@ _have_fio() {
 _have_fio_zbd_zonemode() {
 	_have_fio || return $?
 	if ! fio --cmdhelp=zonemode 2>&1 | grep -q zbd; then
-		SKIP_REASON="Fio version too old (does not support --zonemode=zbd)"
+		SKIP_REASONS+=("Fio version too old (does not support --zonemode=zbd)")
 		return 1
 	fi
 	return 0
diff --git a/common/iopoll b/common/iopoll
index dfdd2cf6f08f..34c021161bf4 100644
--- a/common/iopoll
+++ b/common/iopoll
@@ -11,7 +11,7 @@ _have_fio_with_poll() {
 		return 1
 	fi
 	if ! fio --parse-only --name=test --ioengine=pvsync2 --hipri=1 1>/dev/null 2>&1; then
-		SKIP_REASON="Fio does not support polling"
+		SKIP_REASONS+=("Fio does not support polling")
 		return 1
 	fi
 	return 0
@@ -20,11 +20,11 @@ _have_fio_with_poll() {
 _require_test_dev_supports_io_poll() {
 	local old_io_poll
 	if ! old_io_poll="$(cat "${TEST_DEV_SYSFS}/queue/io_poll" 2>/dev/null)"; then
-		SKIP_REASON="kernel does not support polling"
+		SKIP_REASONS+=("kernel does not support polling")
 		return 1
 	fi
 	if ! echo "$old_io_poll" >"${TEST_DEV_SYSFS}/queue/io_poll" 2>/dev/null; then
-		SKIP_REASON="$TEST_DEV does not support polling"
+		SKIP_REASONS+=("$TEST_DEV does not support polling")
 		return 1
 	fi
 	return 0
@@ -33,11 +33,11 @@ _require_test_dev_supports_io_poll() {
 _require_test_dev_supports_io_poll_delay() {
 	local old_io_poll_delay
 	if ! old_io_poll_delay="$(cat "${TEST_DEV_SYSFS}/queue/io_poll_delay" 2>/dev/null)"; then
-		SKIP_REASON="kernel does not support hybrid polling"
+		SKIP_REASONS+=("kernel does not support hybrid polling")
 		return 1
 	fi
 	if ! echo "$old_io_poll_delay" >"${TEST_DEV_SYSFS}/queue/io_poll_delay" 2>/dev/null; then
-		SKIP_REASON="$TEST_DEV does not support hybrid polling"
+		SKIP_REASONS+=("$TEST_DEV does not support hybrid polling")
 		return 1
 	fi
 	return 0
diff --git a/common/multipath-over-rdma b/common/multipath-over-rdma
index 8edcf3921a7f..b8214ad52ac6 100644
--- a/common/multipath-over-rdma
+++ b/common/multipath-over-rdma
@@ -21,7 +21,7 @@ fi
 _have_legacy_dm() {
 	_have_kernel_config_file || return
 	if ! _check_kernel_option DM_MQ_DEFAULT; then
-		SKIP_REASON="legacy device mapper support is missing"
+		SKIP_REASONS+=("legacy device mapper support is missing")
 		return 1
 	fi
 }
@@ -53,7 +53,7 @@ _multipathd_version_ge() {
 	if version_le "$min_ver" "$mp_ver"; then
 		return 0
 	fi
-	SKIP_REASON="Need multipathd version $min_ver; found multipathd version $mp_ver."
+	SKIP_REASONS+=("Need multipathd version $min_ver; found multipathd version $mp_ver.")
 	return 1
 }
 
diff --git a/common/null_blk b/common/null_blk
index 918ba443b983..52eb48659d8d 100644
--- a/common/null_blk
+++ b/common/null_blk
@@ -24,7 +24,7 @@ _init_null_blk() {
 	if (( RUN_FOR_ZONED )); then zoned="zoned=1"; fi
 
 	if ! modprobe -r null_blk || ! modprobe null_blk "$@" "${zoned}" ; then
-		SKIP_REASON="requires modular null_blk"
+		SKIP_REASONS+=("requires modular null_blk")
 		return 1
 	fi
 
diff --git a/common/rc b/common/rc
index bf6cd10cfb7e..01df6fa53641 100644
--- a/common/rc
+++ b/common/rc
@@ -22,7 +22,7 @@ _divide_timeout() {
 
 _have_root() {
 	if [[ $EUID -ne 0 ]]; then
-		SKIP_REASON="not running as root"
+		SKIP_REASONS+=("not running as root")
 		return 1
 	fi
 	return 0
@@ -33,7 +33,7 @@ _have_driver()
 	local modname="${1/-/_}"
 
 	if [ ! -d "/sys/module/${modname}" ] && ! modprobe -q "${modname}"; then
-		SKIP_REASON="driver ${modname} is not available"
+		SKIP_REASONS+=("driver ${modname} is not available")
 		return 1
 	fi
 
@@ -50,10 +50,10 @@ _have_modules() {
 		fi
 	done
 	if [[ ${#missing} -gt 1 ]]; then
-		SKIP_REASON="the following modules are not available: ${missing[*]}"
+		SKIP_REASONS+=("the following modules are not available: ${missing[*]}")
 		return 1
 	elif [[ ${#missing} -eq 1 ]]; then
-		SKIP_REASON="${missing[0]} module is not available"
+		SKIP_REASONS+=("${missing[0]} module is not available")
 		return 1
 	fi
 	return 0
@@ -67,7 +67,7 @@ _have_module_param() {
 	fi
 
 	if ! modinfo -F parm -0 "$1" | grep -q -z "^$2:"; then
-		SKIP_REASON="$1 module does not have parameter $2"
+		SKIP_REASONS+=("$1 module does not have parameter $2")
 		return 1
 	fi
 	return 0
@@ -89,7 +89,7 @@ _have_module_param_value() {
 
 	value=$(cat "/sys/module/$modname/parameters/$param")
 	if [[ "${value}" != "$expected_value" ]]; then
-		SKIP_REASON="$modname module parameter $param must be set to $expected_value"
+		SKIP_REASONS+=("$modname module parameter $param must be set to $expected_value")
 		return 1
 	fi
 
@@ -100,7 +100,7 @@ _have_program() {
 	if command -v "$1" >/dev/null 2>&1; then
 		return 0
 	fi
-	SKIP_REASON="$1 is not available"
+	SKIP_REASONS+=("$1 is not available")
 	return 1
 }
 
@@ -109,11 +109,11 @@ _have_iproute2() {
 	local snapshot
 
 	if ! snapshot=$(ip -V | sed 's/ip utility, iproute2-ss//'); then
-		SKIP_REASON="ip utility not found"
+		SKIP_REASONS+=("ip utility not found")
 		return 1
 	fi
 	if [[ "$snapshot" =~ ^[0-9]+$ && "$snapshot" -lt "$1" ]]; then
-		SKIP_REASON="ip utility too old"
+		SKIP_REASONS+=("ip utility too old")
 		return 1
 	fi
 	return 0
@@ -121,7 +121,7 @@ _have_iproute2() {
 
 _have_src_program() {
 	if [[ ! -x "$SRCDIR/$1" ]]; then
-		SKIP_REASON="$1 was not built; run \`make\`"
+		SKIP_REASONS+=("$1 was not built; run \`make\`")
 		return 1
 	fi
 	return 0
@@ -135,7 +135,7 @@ _have_blktrace() {
 	# CONFIG_BLK_DEV_IO_TRACE might still be disabled, but this is easier
 	# to check. We can fix it if someone complains.
 	if [[ ! -d /sys/kernel/debug/block ]]; then
-		SKIP_REASON="CONFIG_DEBUG_FS is not enabled"
+		SKIP_REASONS+=("CONFIG_DEBUG_FS is not enabled")
 		return 1
 	fi
 	_have_program blktrace
@@ -143,7 +143,7 @@ _have_blktrace() {
 
 _have_configfs() {
 	if ! findmnt -t configfs /sys/kernel/config >/dev/null; then
-		SKIP_REASON="configfs is not mounted at /sys/kernel/config"
+		SKIP_REASONS+=("configfs is not mounted at /sys/kernel/config")
 		return 1
 	fi
 	return 0
@@ -152,7 +152,7 @@ _have_configfs() {
 # Check if the specified kernel config files are available.
 _have_kernel_config_file() {
 	if [[ ! -f /proc/config.gz && ! -f /boot/config-$(uname -r) ]]; then
-		SKIP_REASON="kernel $(uname -r) config not found"
+		SKIP_REASONS+=("kernel $(uname -r) config not found")
 		return 1
 	fi
 
@@ -180,7 +180,7 @@ _have_kernel_option() {
 
 	_have_kernel_config_file || return
 	if ! _check_kernel_option "$opt"; then
-		SKIP_REASON="kernel option $opt has not been enabled"
+		SKIP_REASONS+=("kernel option $opt has not been enabled")
 		return 1
 	fi
 
@@ -195,7 +195,7 @@ _have_kver() {
 	IFS='.' read -r a b c < <(uname -r | sed 's/-.*//')
 	if [ $((a * 65536 + b * 256 + c)) -lt $((d * 65536 + e * 256 + f)) ];
 	then
-		SKIP_REASON="Kernel version too old"
+		SKIP_REASONS+=("Kernel version too old")
 		return 1
 	fi
 }
@@ -203,7 +203,7 @@ _have_kver() {
 _have_tracefs() {
 	stat /sys/kernel/debug/tracing/trace > /dev/null 2>&1
 	if ! findmnt -t tracefs /sys/kernel/debug/tracing >/dev/null; then
-		SKIP_REASON="tracefs is not mounted at /sys/kernel/debug/tracing"
+		SKIP_REASONS+=("tracefs is not mounted at /sys/kernel/debug/tracing")
 		return 1
 	fi
 	return 0
@@ -213,7 +213,7 @@ _have_tracepoint() {
 	local event=$1
 
 	if [[ ! -d /sys/kernel/debug/tracing/events/${event} ]]; then
-		SKIP_REASON="tracepoint ${event} does not exist"
+		SKIP_REASONS+=("tracepoint ${event} does not exist")
 		return 1
 	fi
 	return 0
@@ -222,7 +222,7 @@ _have_tracepoint() {
 _have_fs() {
 	modprobe "$1" >/dev/null 2>&1
 	if [[ ! -d "/sys/fs/$1" ]]; then
-		SKIP_REASON="kernel does not support filesystem $1"
+		SKIP_REASONS+=("kernel does not support filesystem $1")
 		return 1
 	fi
 }
@@ -231,7 +231,7 @@ _require_min_cpus() {
 	if [[ $(nproc) -ge $1 ]]; then
 		return 0
 	fi
-	SKIP_REASON="minimum $1 cpus required"
+	SKIP_REASONS+=("minimum $1 cpus required")
 	return 1
 }
 
@@ -241,7 +241,7 @@ _test_dev_is_rotational() {
 
 _require_test_dev_is_rotational() {
 	if ! _test_dev_is_rotational; then
-		SKIP_REASON="$TEST_DEV is not rotational"
+		SKIP_REASONS+=("$TEST_DEV is not rotational")
 		return 1
 	fi
 	return 0
@@ -253,7 +253,7 @@ _test_dev_can_discard() {
 
 _require_test_dev_can_discard() {
 	if ! _test_dev_can_discard; then
-		SKIP_REASON="$TEST_DEV does not support discard"
+		SKIP_REASONS+=("$TEST_DEV does not support discard")
 		return 1
 	fi
 	return 0
@@ -291,7 +291,7 @@ _require_test_dev_is_pci() {
 			fi
 		fi
 
-		SKIP_REASON="$TEST_DEV is not a PCI device"
+		SKIP_REASONS+=("$TEST_DEV is not a PCI device")
 		return 1
 	fi
 	return 0
@@ -316,7 +316,7 @@ _require_test_dev_in_hotplug_slot() {
 	local slt_cap
 	slt_cap="$(setpci -s "${parent}" CAP_EXP+14.w)"
 	if [[ $((0x${slt_cap} & 0x60)) -ne $((0x60)) ]]; then
-		SKIP_REASON="$TEST_DEV is not in a hot pluggable slot"
+		SKIP_REASONS+=("$TEST_DEV is not in a hot pluggable slot")
 		return 1
 	fi
 	return 0
@@ -349,7 +349,7 @@ _test_dev_max_open_active_zones() {
 
 _require_test_dev_is_partition() {
 	if ! _test_dev_is_partition; then
-		SKIP_REASON="${TEST_DEV} is not a partition device"
+		SKIP_REASONS+=("${TEST_DEV} is not a partition device")
 		return 1
 	fi
 	return 0
@@ -391,7 +391,7 @@ unload_module() {
 
 _have_writeable_kmsg() {
 	if [[ ! -w /dev/kmsg ]]; then
-		SKIP_REASON="cannot write to /dev/kmsg"
+		SKIP_REASONS+=("cannot write to /dev/kmsg")
 		return 1
 	fi
 	return 0
diff --git a/common/shellcheck b/common/shellcheck
index 72900b030121..8c324bd3113a 100644
--- a/common/shellcheck
+++ b/common/shellcheck
@@ -6,5 +6,5 @@
 
 # Suppress unused global variable warnings.
 _silence_sc2034() {
-	echo "$CAN_BE_ZONED $CGROUP2_DIR $CHECK_DMESG $DESCRIPTION $DMESG_FILTER $FIO_PERF_FIELDS $FIO_PERF_PREFIX $QUICK $SKIP_REASON ${TEST_RUN[*]} $TIMED" > /dev/null
+	echo "$CAN_BE_ZONED $CGROUP2_DIR $CHECK_DMESG $DESCRIPTION $DMESG_FILTER $FIO_PERF_FIELDS $FIO_PERF_PREFIX $QUICK $SKIP_REASONS ${TEST_RUN[*]} $TIMED" > /dev/null
 }
diff --git a/new b/new
index 41f8b8d5468b..817656fac97b 100755
--- a/new
+++ b/new
@@ -64,29 +64,31 @@ if [[ ! -e tests/${group} ]]; then
 
 # TODO: if this test group has any extra requirements, it should define a
 # group_requires() function. If tests in this group cannot be run,
-# group_requires() should set the \$SKIP_REASON variable.
+# group_requires() should add strings to the \$SKIP_REASONS array which
+# describe why the group cannot be run.
 #
 # Usually, group_requires() just needs to check that any necessary programs and
 # kernel features are available using the _have_foo helpers. If
-# group_requires() sets \$SKIP_REASON, all tests in this group will be skipped.
+# group_requires() adds strings to \$SKIP_REASONS, all tests in this group will
+# be skipped.
 group_requires() {
 	_have_root
 }
 
 # TODO: if this test group has extra requirements for what devices it can be
 # run on, it should define a group_device_requires() function. If tests in this
-# group cannot be run on the test device, it should set the \$SKIP_REASON
-# variable. \$TEST_DEV is the full path of the block device (e.g., /dev/nvme0n1
-# or /dev/sda1), and \$TEST_DEV_SYSFS is the sysfs path of the disk (not the
-# partition, e.g., /sys/block/nvme0n1 or /sys/block/sda). If the target device
-# is a partition device, \$TEST_DEV_PART_SYSFS is the sysfs path of the
-# partition device (e.g., /sys/block/nvme0n1/nvme0n1p1 or /sys/block/sda/sda1).
-# Otherwise, \$TEST_DEV_PART_SYSFS is an empty string.
+# group cannot be run on the test device, it should adds strings to
+# \$SKIP_REASONS. \$TEST_DEV is the full path of the block device (e.g.,
+# /dev/nvme0n1 # or /dev/sda1), and \$TEST_DEV_SYSFS is the sysfs path of the
+# disk (not the # partition, e.g., /sys/block/nvme0n1 or /sys/block/sda). If
+# the target device is a partition device, \$TEST_DEV_PART_SYSFS is the sysfs
+# path of the partition device (e.g., /sys/block/nvme0n1/nvme0n1p1 or
+# /sys/block/sda/sda1). Otherwise, \$TEST_DEV_PART_SYSFS is an empty string.
 #
 # Usually, group_device_requires() just needs to check that the test device is
 # the right type of hardware or supports any necessary features using the
-# _require_test_dev_foo helpers. If group_device_requires() sets \$SKIP_REASON,
-# all tests in this group will be skipped on that device.
+# _require_test_dev_foo helpers. If group_device_requires() adds strings to
+# \$SKIP_REASONS, all tests in this group will be skipped on that device.
 # group_device_requires() {
 # 	_require_test_dev_is_foo && _require_test_dev_supports_bar
 # }
@@ -149,28 +151,28 @@ DESCRIPTION=""
 # CAN_BE_ZONED=1
 
 # TODO: if this test has any extra requirements, it should define a requires()
-# function. If the test cannot be run, requires() should set the \$SKIP_REASON
-# variable. Usually, requires() just needs to check that any necessary programs
-# and kernel features are available using the _have_foo helpers. If requires()
-# sets \$SKIP_REASON, the test is skipped.
+# function. If the test cannot be run, requires() should add strings to
+# \$SKIP_REASONS. Usually, requires() just needs to check that any necessary
+# programs and kernel features are available using the  _have_foo helpers.
+# If requires() adds strings to \$SKIP_REASONS, the test is skipped.
 # requires() {
 # 	_have_foo
 # }
 
 # TODO: if this test has extra requirements for what devices it can be run on,
 # it should define a device_requires() function. If this test cannot be run on
-# the test device, it should set \$SKIP_REASON. \$TEST_DEV is the full path of
-# the block device (e.g., /dev/nvme0n1 or /dev/sda1), and \$TEST_DEV_SYSFS is
-# the sysfs path of the disk (not the partition, e.g., /sys/block/nvme0n1 or
-# /sys/block/sda). If the target device is a partition device,
-# \$TEST_DEV_PART_SYSFS is the sysfs path of the partition device (e.g.,
-# /sys/block/nvme0n1/nvme0n1p1 or /sys/block/sda/sda1). Otherwise,
+# the test device, it should add strings to \$SKIP_REASONS. \$TEST_DEV is the
+# full path of the block device (e.g., /dev/nvme0n1 or /dev/sda1), and
+# \$TEST_DEV_SYSFS is the sysfs path of the disk (not the partition, e.g.,
+# /sys/block/nvme0n1 or /sys/block/sda). If the target device is a partition
+# device, \$TEST_DEV_PART_SYSFS is the sysfs path of the partition device
+# (e.g., /sys/block/nvme0n1/nvme0n1p1 or /sys/block/sda/sda1). Otherwise,
 # \$TEST_DEV_PART_SYSFS is an empty string.
 #
 # Usually, device_requires() just needs to check that the test device is the
 # right type of hardware or supports any necessary features using the
-# _require_test_dev_foo helpers. If device_requires() sets \$SKIP_REASON, the
-# test will be skipped on that device.
+# _require_test_dev_foo helpers. If device_requires() adds strings to
+# \$SKIP_REASONS, the test will be skipped on that device.
 # device_requires() {
 # 	_require_test_dev_is_foo && _require_test_dev_supports_bar
 # }
@@ -185,7 +187,7 @@ DESCRIPTION=""
 # failure. You should prefer letting the test fail because of broken output
 # over, say, checking the exit status of every command you run.
 #
-# If the test cannot be run, this function may set the \$SKIP_REASON variable
+# If the test cannot be run, this function may add strings to \$SKIP_REASONS
 # and return. In that case, the return value and output are ignored, and the
 # test is considered skipped. This should only be done when the requirements
 # can only be detected with a non-trivial amount of setup; use
diff --git a/tests/block/030 b/tests/block/030
index f08f7720bc46..7a0712b5b800 100755
--- a/tests/block/030
+++ b/tests/block/030
@@ -45,7 +45,7 @@ test() {
 			{ nproc > $sq; } >>"$FULL" 2>&1
 		done
 	else
-		SKIP_REASON="Skipping test because $sq cannot be modified"
+		SKIP_REASONS+=("Skipping test because $sq cannot be modified")
 	fi
 	rmdir /sys/kernel/config/nullb/nullb0
 	_exit_null_blk
diff --git a/tests/loop/rc b/tests/loop/rc
index 001292af93ee..c9496563048f 100644
--- a/tests/loop/rc
+++ b/tests/loop/rc
@@ -13,7 +13,7 @@ group_requires() {
 _have_loop_set_block_size() {
 	src/loblksize "$(losetup -f)" 512 &>/dev/null
 	if [[ $? -eq 2 ]]; then
-		SKIP_REASON="kernel does not support LOOP_SET_BLOCK_SIZE"
+		SKIP_REASONS+=("kernel does not support LOOP_SET_BLOCK_SIZE")
 		return 1
 	fi
 	return 0
diff --git a/tests/meta/007 b/tests/meta/007
index ae57a67e9631..30b67588e05c 100755
--- a/tests/meta/007
+++ b/tests/meta/007
@@ -9,7 +9,7 @@
 DESCRIPTION="skip in requires()"
 
 requires() {
-	SKIP_REASON="(╯°□°)╯︵ ┻━┻"
+	SKIP_REASONS+=("(╯°□°)╯︵ ┻━┻")
 }
 
 test() {
diff --git a/tests/meta/008 b/tests/meta/008
index 6d7b032ad784..ec3d7891b762 100755
--- a/tests/meta/008
+++ b/tests/meta/008
@@ -9,7 +9,7 @@
 DESCRIPTION="skip in device_requires()"
 
 device_requires() {
-	SKIP_REASON="(╯°□°)╯︵ $TEST_DEV ┻━┻"
+	SKIP_REASONS+=("(╯°□°)╯︵ $TEST_DEV ┻━┻")
 }
 
 test_device() {
diff --git a/tests/meta/013 b/tests/meta/013
index fd9f8e92bfd3..31fbbe0e3264 100755
--- a/tests/meta/013
+++ b/tests/meta/013
@@ -9,7 +9,7 @@
 DESCRIPTION="skip test_device() in requires()"
 
 requires() {
-	SKIP_REASON="(╯°□°)╯︵ ┻━┻"
+	SKIP_REASONS+=("(╯°□°)╯︵ ┻━┻")
 }
 
 test_device() {
diff --git a/tests/meta/014 b/tests/meta/014
index 0db1fb8005a2..26b6a4ee5f26 100755
--- a/tests/meta/014
+++ b/tests/meta/014
@@ -9,5 +9,5 @@
 DESCRIPTION="skip in test()"
 
 test() {
-	SKIP_REASON="(╯°□°)╯︵ ┻━┻"
+	SKIP_REASONS+=("(╯°□°)╯︵ ┻━┻")
 }
diff --git a/tests/meta/015 b/tests/meta/015
index 3dc588b42542..9592e32c97c9 100755
--- a/tests/meta/015
+++ b/tests/meta/015
@@ -9,5 +9,5 @@
 DESCRIPTION="skip in test_device()"
 
 test_device() {
-	SKIP_REASON="(╯°□°)╯︵ ┻━┻"
+	SKIP_REASONS+=("(╯°□°)╯︵ ┻━┻")
 }
diff --git a/tests/meta/rc b/tests/meta/rc
index 833e87250efd..3a472be02ddc 100644
--- a/tests/meta/rc
+++ b/tests/meta/rc
@@ -8,13 +8,13 @@
 
 group_requires() {
 	if [[ "${META_REQUIRES_SKIP:-}" ]]; then
-		SKIP_REASON="META_REQUIRES_SKIP was set"
+		SKIP_REASONS+=("META_REQUIRES_SKIP was set")
 	fi
 }
 
 group_device_requires() {
 	if [[ "${META_DEVICE_REQUIRES_SKIP:-}" ]]; then
-		SKIP_REASON="META_DEVICE_REQUIRES_SKIP was set"
+		SKIP_REASONS+=("META_DEVICE_REQUIRES_SKIP was set")
 	fi
 }
 
diff --git a/tests/nbd/rc b/tests/nbd/rc
index 118553c1d192..5cfb2cc6f64c 100644
--- a/tests/nbd/rc
+++ b/tests/nbd/rc
@@ -21,7 +21,7 @@ _have_nbd() {
 		return 1
 	fi
 	if ! nbd-client --help 2>&1 | grep -q -- -L; then
-		SKIP_REASON="nbd-client does not have -nonetlink/-L option"
+		SKIP_REASONS+=("nbd-client does not have -nonetlink/-L option")
 		return 1
 	fi
 	return 0
@@ -35,7 +35,7 @@ _have_nbd_netlink() {
 		return 1
 	fi
 	if ! genl-ctrl-list | grep -q nbd; then
-		SKIP_REASON="nbd does not support netlink"
+		SKIP_REASONS+=("nbd does not support netlink")
 		return 1
 	fi
 	return 0
diff --git a/tests/nvme/rc b/tests/nvme/rc
index 4bebbc762cbb..c5725ed55eb3 100644
--- a/tests/nvme/rc
+++ b/tests/nvme/rc
@@ -35,7 +35,7 @@ _nvme_requires() {
 		_have_driver rdma_rxe || _have_driver siw
 		;;
 	*)
-		SKIP_REASON="unsupported nvme_trtype=${nvme_trtype}"
+		SKIP_REASONS+=("unsupported nvme_trtype=${nvme_trtype}")
 		return 1
 	esac
 
@@ -51,7 +51,7 @@ _nvme_requires() {
 			# ignore for non ip transports
 			if [[ "${nvme_trtype}" == "tcp" ||
 			      "${nvme_trtype}" == "rdma" ]]; then
-				SKIP_REASON="unsupported nvme_adrfam=${nvme_adrfam}"
+				SKIP_REASONS+=("unsupported nvme_adrfam=${nvme_adrfam}")
 				return 1
 			fi
 		esac
@@ -72,7 +72,7 @@ NVMET_CFS="/sys/kernel/config/nvmet/"
 
 _require_test_dev_is_nvme() {
 	if ! readlink -f "$TEST_DEV_SYSFS/device" | grep -q nvme; then
-		SKIP_REASON="$TEST_DEV is not a NVMe device"
+		SKIP_REASONS+=("$TEST_DEV is not a NVMe device")
 		return 1
 	fi
 	return 0
@@ -80,7 +80,7 @@ _require_test_dev_is_nvme() {
 
 _require_nvme_trtype_is_loop() {
 	if [[ "${nvme_trtype}" != "loop" ]]; then
-		SKIP_REASON="nvme_trtype=${nvme_trtype} is not supported in this test"
+		SKIP_REASONS+=("nvme_trtype=${nvme_trtype} is not supported in this test")
 		return 1
 	fi
 	return 0
@@ -88,7 +88,7 @@ _require_nvme_trtype_is_loop() {
 
 _require_nvme_trtype_is_fabrics() {
 	if [[ "${nvme_trtype}" == "pci" ]]; then
-		SKIP_REASON="nvme_trtype=${nvme_trtype} is not supported in this test"
+		SKIP_REASONS+=("nvme_trtype=${nvme_trtype} is not supported in this test")
 		return 1
 	fi
 	return 0
diff --git a/tests/nvmeof-mp/rc b/tests/nvmeof-mp/rc
index 2ae80c3d47e8..b7ca611f3ff3 100755
--- a/tests/nvmeof-mp/rc
+++ b/tests/nvmeof-mp/rc
@@ -20,8 +20,7 @@ group_requires() {
 	# option with multipathing enabled in the nvme_core kernel module.
 	if _check_kernel_option NVME_MULTIPATH && \
 		_have_module_param_value nvme_core multipath Y; then
-		SKIP_REASON="CONFIG_NVME_MULTIPATH has been set in .config \
-and multipathing has been enabled in the nvme_core kernel module"
+		SKIP_REASONS+=("CONFIG_NVME_MULTIPATH has been set in .config and multipathing has been enabled in the nvme_core kernel module")
 		return
 	fi
 
@@ -56,14 +55,14 @@ and multipathing has been enabled in the nvme_core kernel module"
 	# shellcheck disable=SC2043
 	for name in multipathd; do
 		if pidof "$name" >/dev/null; then
-			SKIP_REASON="$name must be stopped before the nvmeof-mp tests are run"
+			SKIP_REASONS+=("$name must be stopped before the nvmeof-mp tests are run")
 			return
 		fi
 	done
 	if [ -e /etc/multipath.conf ] &&
 	    ! diff -q /etc/multipath.conf tests/nvmeof-mp/multipath.conf >&/dev/null
 	then
-		SKIP_REASON="/etc/multipath.conf already exists"
+		SKIP_REASONS+=("/etc/multipath.conf already exists")
 		return
 	fi
 }
diff --git a/tests/scsi/rc b/tests/scsi/rc
index 0751e77c7635..3b4a33c4b203 100644
--- a/tests/scsi/rc
+++ b/tests/scsi/rc
@@ -20,7 +20,7 @@ _have_scsi_generic() {
 
 _require_test_dev_is_scsi() {
 	if [[ ! -d ${TEST_DEV_SYSFS}/device/scsi_device ]]; then
-		SKIP_REASON="$TEST_DEV is not a SCSI device"
+		SKIP_REASONS+=("$TEST_DEV is not a SCSI device")
 		return 1
 	fi
 	return 0
@@ -28,7 +28,7 @@ _require_test_dev_is_scsi() {
 
 _require_test_dev_is_scsi_disk() {
 	if [[ ! -d ${TEST_DEV_SYSFS}/device/scsi_disk ]]; then
-		SKIP_REASON="$TEST_DEV is not a SCSI disk"
+		SKIP_REASONS+=("$TEST_DEV is not a SCSI disk")
 		return 1
 	fi
 	return 0
diff --git a/tests/srp/012 b/tests/srp/012
index b862ac46c28c..1b34393600ce 100755
--- a/tests/srp/012
+++ b/tests/srp/012
@@ -16,7 +16,7 @@ test_io_schedulers() {
 	done
 	for mq in y n; do
 		if [ $mq = n ] && ! _have_legacy_dm; then
-			unset SKIP_REASON
+			unset SKIP_REASONS
 			continue
 		fi
 		use_blk_mq ${mq} ${mq} || return $?
diff --git a/tests/srp/rc b/tests/srp/rc
index d44082af39de..94ee97cd6d97 100755
--- a/tests/srp/rc
+++ b/tests/srp/rc
@@ -32,7 +32,7 @@ group_requires() {
 
 	_have_configfs || return
 	if is_lio_configured; then
-		SKIP_REASON="LIO must be unloaded before the SRP tests are run"
+		SKIP_REASONS+=("LIO must be unloaded before the SRP tests are run")
 		return
 	fi
 	required_modules=(
@@ -74,14 +74,14 @@ group_requires() {
 
 	for name in srp_daemon multipathd; do
 		if pidof "$name" >/dev/null; then
-			SKIP_REASON="$name must be stopped before the SRP tests are run"
+			SKIP_REASONS+=("$name must be stopped before the SRP tests are run")
 			return
 		fi
 	done
 	if [ -e /etc/multipath.conf ] &&
 	    ! diff -q /etc/multipath.conf tests/srp/multipath.conf >&/dev/null
 	then
-		SKIP_REASON="/etc/multipath.conf already exists"
+		SKIP_REASONS+=("/etc/multipath.conf already exists")
 		return
 	fi
 }
diff --git a/tests/zbd/004 b/tests/zbd/004
index f09ee31a72d7..17c860e248d5 100755
--- a/tests/zbd/004
+++ b/tests/zbd/004
@@ -47,7 +47,7 @@ test_device() {
 	# Find target sequential required zones and reset write pointers
 	_get_blkzone_report "${TEST_DEV}" || return $?
 	if ! idx=$(_find_two_contiguous_seq_zones cap_eq_len); then
-		SKIP_REASON="No contiguous sequential write required zones"
+		SKIP_REASONS+=("No contiguous sequential write required zones")
 		_put_blkzone_report
 		return
 	fi
diff --git a/tests/zbd/rc b/tests/zbd/rc
index fea55d64ea37..77d3ec236607 100644
--- a/tests/zbd/rc
+++ b/tests/zbd/rc
@@ -19,7 +19,7 @@ group_requires() {
 
 group_device_requires() {
 	if ! _test_dev_is_zoned; then
-		SKIP_REASON="${TEST_DEV} is not a zoned block device"
+		SKIP_REASONS+=("${TEST_DEV} is not a zoned block device")
 		return
 	fi
 }
@@ -287,7 +287,7 @@ _test_dev_is_dm() {
 
 _require_test_dev_is_logical() {
 	if ! _test_dev_is_partition && ! _test_dev_is_dm; then
-		SKIP_REASON="$TEST_DEV is not a logical device"
+		SKIP_REASONS+=("$TEST_DEV is not a logical device")
 		return 1
 	fi
 	return 0
-- 
2.31.1

^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH blktests v2] common, tests: Support printing multiple skip reasons
  2022-07-12  8:21 [PATCH blktests v2] common, tests: Support printing multiple skip reasons lizhijian
@ 2022-07-12 21:26 ` Bart Van Assche
  2022-07-13 10:41 ` Shinichiro Kawasaki
  1 sibling, 0 replies; 4+ messages in thread
From: Bart Van Assche @ 2022-07-12 21:26 UTC (permalink / raw)
  To: lizhijian, linux-block, shinichiro.kawasaki

On 7/12/22 01:21, lizhijian@fujitsu.com wrote:
> Some test cases or test groups have rather large number of test
> run requirements and then they may have multiple skip reasons. However,
> blktests can report only single skip reason. To know all of the skip
> reasons, we need to repeat skip reason resolution and blktests run.
> This is a troublesome work.
> 
> In this patch, we add skip reasons to SKIP_REASONS array, then all of
> the skip reasons will be printed by iterating SKIP_REASONS at one shot
> run.

Reviewed-by: Bart Van Assche <bvanassche@acm.org>

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH blktests v2] common, tests: Support printing multiple skip reasons
  2022-07-12  8:21 [PATCH blktests v2] common, tests: Support printing multiple skip reasons lizhijian
  2022-07-12 21:26 ` Bart Van Assche
@ 2022-07-13 10:41 ` Shinichiro Kawasaki
  2022-07-13 12:07   ` lizhijian
  1 sibling, 1 reply; 4+ messages in thread
From: Shinichiro Kawasaki @ 2022-07-13 10:41 UTC (permalink / raw)
  To: lizhijian; +Cc: linux-block, bvanassche

Hi Zhijian, thanks for this v2 patch. I found a few minor nits in the changes
in the "new" script. Please find my comments in line. Also, the word
"Supporting" in the commit title is not so meaningful. It could be
"common, tests: Print multiple skip reasons".

Could you respin the patch one more time?

On Jul 12, 2022 / 08:21, lizhijian@fujitsu.com wrote:
> Some test cases or test groups have rather large number of test
> run requirements and then they may have multiple skip reasons. However,
> blktests can report only single skip reason. To know all of the skip
> reasons, we need to repeat skip reason resolution and blktests run.
> This is a troublesome work.
> 
> In this patch, we add skip reasons to SKIP_REASONS array, then all of
> the skip reasons will be printed by iterating SKIP_REASONS at one shot
> run.
> 
> Most of the works are done by following script:
> 
> sed -i 's/SKIP_REASON/SKIP_REASONS/' $(git ls-files)
> git grep -h 'SKIP_REASONS=' | awk -F'SKIP_REASONS=' '{print $2}' | sort | uniq |
> while read -r r
> do
> 	s="SKIP_REASONS=$r"
> 	f=$(git grep -l "$s")
> 	sed -i "s@$s@SKIP_REASONS+=($r)@" $f
> done
> 
> Signed-off-by: Li Zhijian <lizhijian@fujitsu.com>

[...]

> diff --git a/new b/new
> index 41f8b8d5468b..817656fac97b 100755
> --- a/new
> +++ b/new
> @@ -64,29 +64,31 @@ if [[ ! -e tests/${group} ]]; then
>  
>  # TODO: if this test group has any extra requirements, it should define a
>  # group_requires() function. If tests in this group cannot be run,
> -# group_requires() should set the \$SKIP_REASON variable.
> +# group_requires() should add strings to the \$SKIP_REASONS array which
> +# describe why the group cannot be run.
>  #
>  # Usually, group_requires() just needs to check that any necessary programs and
>  # kernel features are available using the _have_foo helpers. If
> -# group_requires() sets \$SKIP_REASON, all tests in this group will be skipped.
> +# group_requires() adds strings to \$SKIP_REASONS, all tests in this group will
> +# be skipped.
>  group_requires() {
>  	_have_root
>  }
>  
>  # TODO: if this test group has extra requirements for what devices it can be
>  # run on, it should define a group_device_requires() function. If tests in this
> -# group cannot be run on the test device, it should set the \$SKIP_REASON
> -# variable. \$TEST_DEV is the full path of the block device (e.g., /dev/nvme0n1
> -# or /dev/sda1), and \$TEST_DEV_SYSFS is the sysfs path of the disk (not the
> -# partition, e.g., /sys/block/nvme0n1 or /sys/block/sda). If the target device
> -# is a partition device, \$TEST_DEV_PART_SYSFS is the sysfs path of the
> -# partition device (e.g., /sys/block/nvme0n1/nvme0n1p1 or /sys/block/sda/sda1).
> -# Otherwise, \$TEST_DEV_PART_SYSFS is an empty string.
> +# group cannot be run on the test device, it should adds strings to

s/adds/add/

> +# \$SKIP_REASONS. \$TEST_DEV is the full path of the block device (e.g.,
> +# /dev/nvme0n1 # or /dev/sda1), and \$TEST_DEV_SYSFS is the sysfs path of the
> +# disk (not the # partition, e.g., /sys/block/nvme0n1 or /sys/block/sda). If

Two #s are left in the two lines above.

> +# the target device is a partition device, \$TEST_DEV_PART_SYSFS is the sysfs
> +# path of the partition device (e.g., /sys/block/nvme0n1/nvme0n1p1 or
> +# /sys/block/sda/sda1). Otherwise, \$TEST_DEV_PART_SYSFS is an empty string.
>  #
>  # Usually, group_device_requires() just needs to check that the test device is
>  # the right type of hardware or supports any necessary features using the
> -# _require_test_dev_foo helpers. If group_device_requires() sets \$SKIP_REASON,
> -# all tests in this group will be skipped on that device.
> +# _require_test_dev_foo helpers. If group_device_requires() adds strings to
> +# \$SKIP_REASONS, all tests in this group will be skipped on that device.
>  # group_device_requires() {
>  # 	_require_test_dev_is_foo && _require_test_dev_supports_bar
>  # }
> @@ -149,28 +151,28 @@ DESCRIPTION=""
>  # CAN_BE_ZONED=1
>  
>  # TODO: if this test has any extra requirements, it should define a requires()
> -# function. If the test cannot be run, requires() should set the \$SKIP_REASON
> -# variable. Usually, requires() just needs to check that any necessary programs
> -# and kernel features are available using the _have_foo helpers. If requires()
> -# sets \$SKIP_REASON, the test is skipped.
> +# function. If the test cannot be run, requires() should add strings to
> +# \$SKIP_REASONS. Usually, requires() just needs to check that any necessary
> +# programs and kernel features are available using the  _have_foo helpers.

Double spaces in the live above.

> +# If requires() adds strings to \$SKIP_REASONS, the test is skipped.
>  # requires() {
>  # 	_have_foo
>  # }

[...]

-- 
Shin'ichiro Kawasaki

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH blktests v2] common, tests: Support printing multiple skip reasons
  2022-07-13 10:41 ` Shinichiro Kawasaki
@ 2022-07-13 12:07   ` lizhijian
  0 siblings, 0 replies; 4+ messages in thread
From: lizhijian @ 2022-07-13 12:07 UTC (permalink / raw)
  To: Shinichiro Kawasaki; +Cc: linux-block, bvanassche


on 7/13/2022 6:41 PM, Shinichiro Kawasaki wrote:
> Hi Zhijian, thanks for this v2 patch. I found a few minor nits in the changes
> in the "new" script. Please find my comments in line. Also, the word
> "Supporting" in the commit title is not so meaningful. It could be
> "common, tests: Print multiple skip reasons".
>
> Could you respin the patch one more time?

No problem :)



>
> On Jul 12, 2022 / 08:21, lizhijian@fujitsu.com wrote:
>> Some test cases or test groups have rather large number of test
>> run requirements and then they may have multiple skip reasons. However,
>> blktests can report only single skip reason. To know all of the skip
>> reasons, we need to repeat skip reason resolution and blktests run.
>> This is a troublesome work.
>>
>> In this patch, we add skip reasons to SKIP_REASONS array, then all of
>> the skip reasons will be printed by iterating SKIP_REASONS at one shot
>> run.
>>
>> Most of the works are done by following script:
>>
>> sed -i 's/SKIP_REASON/SKIP_REASONS/' $(git ls-files)
>> git grep -h 'SKIP_REASONS=' | awk -F'SKIP_REASONS=' '{print $2}' | sort | uniq |
>> while read -r r
>> do
>> 	s="SKIP_REASONS=$r"
>> 	f=$(git grep -l "$s")
>> 	sed -i "s@$s@SKIP_REASONS+=($r)@" $f
>> done
>>
>> Signed-off-by: Li Zhijian <lizhijian@fujitsu.com>
> [...]
>
>> diff --git a/new b/new
>> index 41f8b8d5468b..817656fac97b 100755
>> --- a/new
>> +++ b/new
>> @@ -64,29 +64,31 @@ if [[ ! -e tests/${group} ]]; then
>>   
>>   # TODO: if this test group has any extra requirements, it should define a
>>   # group_requires() function. If tests in this group cannot be run,
>> -# group_requires() should set the \$SKIP_REASON variable.
>> +# group_requires() should add strings to the \$SKIP_REASONS array which
>> +# describe why the group cannot be run.
>>   #
>>   # Usually, group_requires() just needs to check that any necessary programs and
>>   # kernel features are available using the _have_foo helpers. If
>> -# group_requires() sets \$SKIP_REASON, all tests in this group will be skipped.
>> +# group_requires() adds strings to \$SKIP_REASONS, all tests in this group will
>> +# be skipped.
>>   group_requires() {
>>   	_have_root
>>   }
>>   
>>   # TODO: if this test group has extra requirements for what devices it can be
>>   # run on, it should define a group_device_requires() function. If tests in this
>> -# group cannot be run on the test device, it should set the \$SKIP_REASON
>> -# variable. \$TEST_DEV is the full path of the block device (e.g., /dev/nvme0n1
>> -# or /dev/sda1), and \$TEST_DEV_SYSFS is the sysfs path of the disk (not the
>> -# partition, e.g., /sys/block/nvme0n1 or /sys/block/sda). If the target device
>> -# is a partition device, \$TEST_DEV_PART_SYSFS is the sysfs path of the
>> -# partition device (e.g., /sys/block/nvme0n1/nvme0n1p1 or /sys/block/sda/sda1).
>> -# Otherwise, \$TEST_DEV_PART_SYSFS is an empty string.
>> +# group cannot be run on the test device, it should adds strings to
> s/adds/add/
>
>> +# \$SKIP_REASONS. \$TEST_DEV is the full path of the block device (e.g.,
>> +# /dev/nvme0n1 # or /dev/sda1), and \$TEST_DEV_SYSFS is the sysfs path of the
>> +# disk (not the # partition, e.g., /sys/block/nvme0n1 or /sys/block/sda). If
> Two #s are left in the two lines above.
>
>> +# the target device is a partition device, \$TEST_DEV_PART_SYSFS is the sysfs
>> +# path of the partition device (e.g., /sys/block/nvme0n1/nvme0n1p1 or
>> +# /sys/block/sda/sda1). Otherwise, \$TEST_DEV_PART_SYSFS is an empty string.
>>   #
>>   # Usually, group_device_requires() just needs to check that the test device is
>>   # the right type of hardware or supports any necessary features using the
>> -# _require_test_dev_foo helpers. If group_device_requires() sets \$SKIP_REASON,
>> -# all tests in this group will be skipped on that device.
>> +# _require_test_dev_foo helpers. If group_device_requires() adds strings to
>> +# \$SKIP_REASONS, all tests in this group will be skipped on that device.
>>   # group_device_requires() {
>>   # 	_require_test_dev_is_foo && _require_test_dev_supports_bar
>>   # }
>> @@ -149,28 +151,28 @@ DESCRIPTION=""
>>   # CAN_BE_ZONED=1
>>   
>>   # TODO: if this test has any extra requirements, it should define a requires()
>> -# function. If the test cannot be run, requires() should set the \$SKIP_REASON
>> -# variable. Usually, requires() just needs to check that any necessary programs
>> -# and kernel features are available using the _have_foo helpers. If requires()
>> -# sets \$SKIP_REASON, the test is skipped.
>> +# function. If the test cannot be run, requires() should add strings to
>> +# \$SKIP_REASONS. Usually, requires() just needs to check that any necessary
>> +# programs and kernel features are available using the  _have_foo helpers.
> Double spaces in the live above.
>
>> +# If requires() adds strings to \$SKIP_REASONS, the test is skipped.
>>   # requires() {
>>   # 	_have_foo
>>   # }
> [...]
>

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2022-07-13 12:08 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-07-12  8:21 [PATCH blktests v2] common, tests: Support printing multiple skip reasons lizhijian
2022-07-12 21:26 ` Bart Van Assche
2022-07-13 10:41 ` Shinichiro Kawasaki
2022-07-13 12:07   ` lizhijian

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.