Re: [PATCH] selftests: lib: Skip tests on missing test modules

[Shuah Khan]

On 06/16/2017 09:10 AM, Shuah Khan wrote:
> Hi Sumit,
> 
> On 06/15/2017 11:29 PM, Sumit Semwal wrote:
>> With older kernels, printf.sh and bitmap.sh fail because they can't find
>> the respective test modules they are looking for.
>>
>> Add the skip portion on missing the respective test_XXX module. Error out
>> the same way as prime_numbers.sh.
>>
>> Signed-off-by: Sumit Semwal <sumit.semwal@xxxxxxxxxx>
>> ---
>>  tools/testing/selftests/lib/bitmap.sh | 5 +++++
>>  tools/testing/selftests/lib/printf.sh | 5 +++++
>>  2 files changed, 10 insertions(+)
>>
>> diff --git a/tools/testing/selftests/lib/bitmap.sh b/tools/testing/selftests/lib/bitmap.sh
>> index 2da187b6ddad..85294b4a0861 100755
>> --- a/tools/testing/selftests/lib/bitmap.sh
>> +++ b/tools/testing/selftests/lib/bitmap.sh
>> @@ -1,6 +1,11 @@
>>  #!/bin/sh
>>  # Runs bitmap infrastructure tests using test_bitmap kernel module
>>  
>> +if ! /sbin/modprobe -q test_bitmap; then
>> +        echo "bitmap: [SKIP]"
>> +	exit 77
>> +fi
> 
> Please add this logic below in the else clause instead of
> adding duplicate block.
> 
> I think it is would be good to differentiate when a modules
> isn't present vs. modprobe failed keying off of
> 
> "Module test_printf not found" message.
> 
> The second one can be separate patch.

Please make the change to detect module not present vs.
module failed load in the same patch. Thinking about more,
this change will always report SKIP which is worse than
what we had before that it always reports FAIL.

The reason being, reporting SKIP when module is present and
failed load due to some error is worse than reporting FAIL
when module isn't present.

> 
>> +
>>  if /sbin/modprobe -q test_bitmap; then
>>  	/sbin/modprobe -q -r test_bitmap
>>  	echo "bitmap: ok"
>> diff --git a/tools/testing/selftests/lib/printf.sh b/tools/testing/selftests/lib/printf.sh
>> index 4fdc70fe6980..024e749a83d4 100755
>> --- a/tools/testing/selftests/lib/printf.sh
>> +++ b/tools/testing/selftests/lib/printf.sh
>> @@ -1,6 +1,11 @@
>>  #!/bin/sh
>>  # Runs printf infrastructure using test_printf kernel module
>>  
>> +if ! /sbin/modprobe -q test_printf; then
>> +        echo "printf: [SKIP]"
>> +	exit 77
>> +fi
> 
> Please add this logic below in the else clause instead of
> adding duplicate block.
> 
> I think it is would be good to differentiate when a modules
> isn't present vs. modprobe failed keying off of
> 
> "Module test_printf not found" message.
> 
> The second one can be separate patch.

Same commnet as above.

> 
>> +
>>  if /sbin/modprobe -q test_printf; then
>>  	/sbin/modprobe -q -r test_printf
>>  	echo "printf: ok"
>>
> 
> thanks,
> -- Shuah
> 
> 

thanks,
-- Shuah