Re: [RFC i-g-t] kunit: Remove hard-coded test list from xe_live_ktest

[John Harrison <john.c.harrison@intel.com>]

[-- Attachment #1: Type: text/plain, Size: 5497 bytes --]

On 9/16/2024 01:17, Janusz Krzysztofik wrote:
> Hi John,
>
> On Saturday, 14 September 2024 02:20:32 GMT+2 John Harrison wrote:
>> On 8/29/2024 02:57, Janusz Krzysztofik wrote:
>>> Hi John,
>>>
>>> On Wednesday, 28 August 2024 20:04:06 GMT+2 John Harrison wrote:
>>>> On 8/26/2024 06:07, Janusz Krzysztofik wrote:
>>>>> Hi John, Kamil,
>>>>>
>>>>> On Monday, 26 August 2024 13:46:45 GMT+2 Kamil Konieczny wrote:
>>>>>> Hi John.C.Harrison,
>>>>>> On 2024-08-23 at 11:24:18 -0700,John.C.Harrison@Intel.com  wrote:
>>>>>>> From: johnharr<johnharr@invalid-email.com>
>>>>>> ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
>>>>>> Again invalid e-mail here.
>>>>>>
>>>>>>> The list of supported kunit tests is currently hard coded.
>>>>> That pattern originates from i915_selftest, where there are 3 stable
> subtests,
>>>>> "mock", "live" and "perf", each of them executing a (possibly long) list
> of
>>>>> dynamic sub-subtests actually provided by the module for each category.
> Also,
>>>>> IIRC there was a separate module for each Xe test suite before, each
> mapped to
>>>>> a separate IGT test, later merged into one module with multiple suites
> and one
>>>>> test with multiple corresponding subtests.
>>>> Not sure if you are just explaining the history of the test or making a
>>>> suggestion as to how it should evolve next?
>>> Both, I think.  Maybe not the history, but origin of ideas standing behind
> the
>>> implementation, and how tests are expected to use it (and maybe evolve if
> now
>>> doing that in a different way).
>>>
>>>>>>> Which means
>>>>>>> that adding a new test to the kernel also requires patches to IGT as
>>>>>>> well.
>>>>>>>
>>>>>>> The list of available kunit tests is already exported by the kernel.
>>>>>>> So there is no need to bake a list into the IGT source code. So, add
>>>>>>> support for querying the test list dynamically.
>>>>>>>
>>>>>>> NB: Currently, the test list can only be queried by root but the IGT
>>>>>>> build system queries all tests at compile time. In theory this should
>>>>>>> not be a problem. However, the kunit helper code is all written to run
>>>>>>> inside a test and not inside the prep code, which means that when it
>>>>>>> fails to open the root only interfaces, it calls 'skip'. And skip is
>>>>>>> not allowed outside of running a test. Hence the build fails with:
>>>>>>>      skipping is allowed only in fixtures, subtests or igt_simple_main
>>>>>> Looks like we should fix it, move out skips from kunit libs.
>>>>> I suggest you consider a different approach: for a module, call
> igt_kunit()
>>>>> only once, with NULL suite argument.  As a result, you'll get results
> from one
>>>>> IGT subtest called "<module_name>" with a bunch of IGT dynamic sub-
> subtests
>>>>> called "<test_suite>-<test_case>", one for each test case provided by
> the
>>>>> module.
>>>> I'm not following. This is what my patch does, isn't it?
>>> No, your patch introduces a runtime determined list of subtests --
> something
>>> not existent in IGT.
>>>
>>> An IGT test may consist of one or more statically defined subtests with
> pre-
>>> defined names.  The term dynamic subtest is usually used in two meanings.
> It
>>> may mean a subtest of type igt_subtest_with_dynamic, still with a pre-
> defined
>>> name, but with a runtime determined list of sub-subtests, sometimes called
>>> dynamic sub-subtests, but often also called just dynamic subtests.  Names
> of
>>> dynamic sub-subtests are determined at runtime.
>>>
>>> My approach tries to address your need for a maintenance-free kunit IGT
> test
>>> source file in a different way.  I'm following the IGT standard of
> statically
>>> defined list of subtests with pre-defined names: one subtest of type
>>> igt_subtest_with_dynamic, named after the kunit test module name which you
>>> have to enter into the test code anyway, and providing all test cases from
>>> all test suites contained in that module reported as dynamic sub-subtests
>>> named <test_suite>-<test_case>.
>> Can you please prototype how to do this?  I can read your words but I
>> don't really get your meaning and I have no clue how to implement what
>> you are saying.
> The test code may look as simple as:
>
> igt_main
> {
>          igt_kunit("xe_live_test", NULL, NULL);
> }
>
> Then, igt_kunit() will use xe_live_test module and should report results from
> all its KUnit test cases as results from a set of IGT dynamic sub-subtests
> "<test_suite>-<test_case>" under a single IGT subtest "xe_live".
>
> Janusz
>
Doing that solves the problem of not generating a test list at compile 
time. But it creates the problem of not being able to generate a test 
list at all:

    0:28> sudo ./build/tests/xe_live_ktest --show-testlist
    igt@xe_live_ktest@xe_live
    0:29> sudo ./build/tests/xe_live_ktest --list-subtests
    xe_live
    0:30> sudo ./build/tests/xe_live_ktest --describe
    SUB xe_live ../lib/igt_kmod.c:1475:
       NO DOCUMENTATION!


And that last is clearly wrong because if I don't have a comment 
description of xe_live then it gives a build time error.

It is possible to run a specific sub-test, but you have to know the name 
already. E.g.:

    0:25> sudo ./build/tests/xe_live_ktest --dynamic-subtest xe_bo-xe_bo_evict_kunit
    IGT-Version: 1.28-g8df80ef3edc8 (x86_64) (Linux: 6.11.0-rc5-g2g x86_64)
    Using IGT_SRANDOM=1726523447 for randomisation
    Starting subtest: xe_live
    Starting dynamic subtest: xe_bo-xe_bo_evict_kunit
    ...


That seems less than ideal.

John.

[-- Attachment #2: Type: text/html, Size: 12343 bytes --]