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 >>>>>> ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ >>>>>> 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 "" with a bunch of IGT dynamic sub- > subtests >>>>> called "-", 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 -. >> 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 > "-" 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.