Re: [PATCH] selftests: ksft: Track skipped tests when finishing the test suite

[Laura Nao <laura.nao@collabora.com>]

On 7/22/24 20:51, Nícolas F. R. A. Prado wrote:
> On Mon, Jul 22, 2024 at 11:32:35AM -0600, Shuah Khan wrote:
>> On 7/22/24 09:43, Laura Nao wrote:
>>> Consider skipped tests in addition to passed tests when evaluating the
>>> overall result of the test suite in the finished() helper.
>>>
>>> Signed-off-by: Laura Nao <laura.nao@collabora.com>
>>> ---
>>>    tools/testing/selftests/kselftest/ksft.py | 2 +-
>>>    1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/tools/testing/selftests/kselftest/ksft.py b/tools/testing/selftests/kselftest/ksft.py
>>> index cd89fb2bc10e..bf215790a89d 100644
>>> --- a/tools/testing/selftests/kselftest/ksft.py
>>> +++ b/tools/testing/selftests/kselftest/ksft.py
>>> @@ -70,7 +70,7 @@ def test_result(condition, description=""):
>>>    def finished():
>>> -    if ksft_cnt["pass"] == ksft_num_tests:
>>> +    if ksft_cnt["pass"] + ksft_cnt["skip"] == ksft_num_tests:
>>
>> Please don't. Counting skips in pass or fail isn't accurate
>> reporting. skips need to be reported as skips.
> 
> Hi Shuah,
> 
> this won't change the skip count, just allow a test suite that has a mix of pass
> and skip results to exit with code 0. That's the same behavior as the C
> ksft_finished() helper in kselftest.h:
> 
> #define ksft_finished()			\
> 	ksft_exit(ksft_plan ==		\
> 		  ksft_cnt.ksft_pass +	\
> 		  ksft_cnt.ksft_xfail +	\
> 		  ksft_cnt.ksft_xskip)
> 

Correct, this patch aligns the behavior of the python helper with both 
the existing C and bash counterparts (ksft_finished() and 
ktap_finished() respectively).

> It was my oversight to not do the same in the python helper.
> 
> Laura,
> 
> I consider this fixing an incorrect behavior, so I'd add this tag:
> 
> Fixes: dacf1d7a78bf ("kselftest: Add test to verify probe of devices from discoverable buses")
> 
> I think the message is good as is, but maybe it could have mentioned that this
> matches the behavior of the C helper, just to make the point above clearer.
> 
> Reviewed-by: Nícolas F. R. A. Prado <nfraprado@collabora.com>
> 
> And just a note for the maintainers, this patch depends on "kselftest: Move ksft
> helper module to common directory"
> https://lore.kernel.org/all/20240705-dev-err-log-selftest-v2-2-163b9cd7b3c1@collabora.com/
> which was picked through the usb tree but is queued for 6.11-rc1.
> 

Right, thanks!

Shuah, please let me know if you have any other concerns with this patch.

Best,

Laura