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@xxxxxxxxxxx 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@xxxxxxxxxxx>
> [...]
>
>> 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
>> # }
> [...]
>
