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....@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 <nfrapr...@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-163b9cd7b...@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
