Re: [PATCH RFC] kselftest: devices: Add test to detect missing devices

On 8/1/24 15:03, Nícolas F. R. A. Prado wrote:
> On Thu, Aug 01, 2024 at 02:13:05PM -0600, Shuah Khan wrote:
>> On 8/1/24 13:15, Nícolas F. R. A. Prado wrote:
>>> On Wed, Jul 31, 2024 at 05:19:45PM -0600, Shuah Khan wrote:
>>>> On 7/24/24 15:40, Nícolas F. R. A. Prado wrote:
>>>>> Introduce a new test to identify regressions causing devices to go
>>>>> missing on the system.
>>>>>
>>>>> For each bus and class on the system the test checks the number of
>>>>> devices present against a reference file, which needs to have been
>>>>> generated by the program at a previous point on a known-good kernel, and
>>>>> if there are missing devices they are reported.
>>>>
>>>> Can you elaborate on how to generate reference file? It isn't clear.
>>>
>>> Indeed, I'll make that information clearer in future versions.
>>>
>>> The reference file is generated by passing the --generate-reference flag to the
>>> test:
>>>
>>> ./exist.py --generate-reference
>>>
>>> It will be printed as standard output.
>>
>> How about adding an option to generate file taking filename?
>> Makes it easier to use.
> 
> Sure, we can do that. Another option would be to write it to the filename that
> would be looked for by default. So for your machine just calling
> 
>    ./exist.py --generate-reference
> 
> could write the reference to ./LENOVO,20XH005JUS.yaml.

You could. Do mention this as the default option and to the
help message.

>
>>>
>>> No, that repository would just be a place where people could find pre-generated
>>> reference files (which we'll be using when running this test in KernelCI), but
>>> anyone can always generate their own reference files and store them wherever
>>> they want.
>>>
>>
>> Thanks for the clarification. This might be good addition to the document.
>> I think this test could benefit from a README or howto
> 
> Sure, I can add a README in the next revision.
> 
>>
>>>>
>>>> This is what I see when I run the test on my system:
>>>>
>>>> make -C tools/testing/selftests/devices/exist/ run_tests
>>>> make: Entering directory '/linux/linux_6.11/tools/testing/selftests/devices/exist'
>>>> TAP version 13
>>>> 1..1
>>>> # timeout set to 45
>>>> # selftests: devices/exist: exist.py
>>>> # TAP version 13
>>>> # # No matching reference file found (tried './LENOVO,20XH005JUS.yaml')
>>>
>>> First generate the reference file for your system like so:
>>>
>>> tools/testing/selftests/devices/exist/exist.py --generate-reference > tools/testing/selftests/devices/exist/LENOVO,20XH005JUS.yaml
>>>
>>
>> Worked - I see
>>
>> TAP version 13
>> # Using reference file: ./LENOVO,20XH005JUS.yaml
>> 1..76
>>
>> ---
>> # Totals: pass:76 fail:0 xfail:0 xpass:0 skip:0 error:0
>>
>>
>> Things to improve:
>>
>> - Have the script take a file instead of assuming that the reference file
>>    is in the current directory.
>>    e.g: exist.py -f reference_file
> 
> The script also has another parameter to specify a different directory to look
> for the reference file: --reference-dir
> 
> But the file name is currently fixed and determined from the system's ID (DMI or
> Devicetree compatible).
> 
> We can definitely have another flag to force a different file name if that's
> useful. In theory it shouldn't be needed given the machine name is used as
> filename, but might come in handy if there are machine name clashes or if you
> want to have references for different kernel stable versions for the same
> machine in the same directory.

Providing an option to force is good.

thanks,
-- Shuah