Re: [PATCH v2 0/1] lib: Convert UUID runtime test to KUnit

["André Almeida" <andrealmeid@collabora.com>]

Às 10:52 de 10/06/21, Andy Shevchenko escreveu:
> On Thu, Jun 10, 2021 at 10:08:30AM -0300, André Almeida wrote:
>> Às 09:39 de 10/06/21, Andy Shevchenko escreveu:
>>> On Thu, Jun 10, 2021 at 2:54 PM David Gow <davidgow@google.com> wrote:
>>>> On Thu, Jun 10, 2021 at 5:14 PM Andy Shevchenko
>>>> <andriy.shevchenko@linux.intel.com> wrote:
>>>>> On Wed, Jun 09, 2021 at 08:37:29PM -0300, André Almeida wrote:
> 
> ...
> 
>>>>> It's not your fault but I think we need to defer this until KUnit gains support
>>>>> of the run statistics. My guts telling me if we allow more and more conversions
>>>>> like this the point will vanish and nobody will care.
>>>>
>>>> Did the test statistics patch we sent out before meet your expectations?
>>>> https://patchwork.kernel.org/project/linux-kselftest/patch/20201211072319.533803-1-davidgow@google.com/
>>>
>>> Let me look at it at some point.
>>>
>>>> If so, we can tidy it up and try to push it through straight away, we
>>>> were just waiting for a review from someone who wanted the feature.
>>>>
>>>>
>>>>> I like the code, but I can give my tag after KUnit prints some kind of this:
>>>>>
>>>>>  * This is how the current output looks like in success:
>>>>>
>>>>>    test_uuid: all 18 tests passed
>>>>>
>>>>>  * And when it fails:
>>>>>
>>>>>    test_uuid: failed 18 out of 18 tests
>>>>>
>>>>
>>>> There are some small restrictions on the exact format KUnit can use
>>>> for this if we want to continue to match the (K)TAP specification
>>>> which is being adopted by kselftest. The patch linked above should
>>>> give something formatted like:
>>>>
>>>> # test_uuid: (0 / 4) tests failed (0 / 12 test parameters)
>>>>
>>>> Would that work for you?
>>>
>>> Can you decode it for me, please?
>>>
>>> (Assuming that the above question arisen, perhaps some rephrasing is
>>> needed. The idea that user should have clear understanding on how many
>>> test cases were run and how many of them successfully finished or
>>> failed. According to this thread I have to see the cumulative number
>>> of 18 (either as one number or sum over test cases or how you call
>>> them, I see 4 here).
>>
>> In the original code, each `if(uuid/guid_parse/equal)` was considered as
>> a test, so there were 4 tests for the 3 correct inputs and 2 tests for
>> the 3 wrong inputs: 4 * 3 + 2 * 3 = 18 tests.
>>
>> In my patch, I've organized in a different way, with 4 test cases:
>>
>> - A test case for guid_parse and guid_equal for correct inputs
>> - A test case for uuid_parse and uuid_equal for correct inputs
>> - A test case for guid_parse for incorrect inputs
>> - A test case for uuid_parse for incorrect inputs
>>
>> So now we have 4 test cases, instead of the 6 test cases in the original
>> code, because I've united _parse and _equal in a single test case. Given
>> that each test has 3 parameters, this is why we see 12 test parameters
>> and that's why there's no "18 tests" around anymore.
> 
> I see, is it mentioned in the commit message? If no, please add this
> explanation.
> 
> Let's assume 12 now is the correct number, then the output can be somehow
> rephrased, but again, it's not in your patch anyway :-)
> 

Sure! I'll add this for v3 :)