[PATCH i-g-t] igt-runner fact checking

[Lucas De Marchi]

+Ryszard to know if the CI part would be doable.

On Thu, Nov 07, 2024 at 08:18:52AM +0100, Peter Senna Tschudin wrote:
>>> +    scan_kernel_loaded_kmods(list, last_test);
>>
>>
>> So... igt_facts() hardcodes what is the info collected. Maybe we
>> should rather plug this into the hooks framework as part of
>> "built-in hooks".
>
>I do not follow what you are trying to communicate here. Also, please see what Zbigniew said here*. Facts are not test requirements, but the line is blurry. In the context here a fact is something in the environment that can change but that should not. An example is the disappearing GPU.

$ ./build/runner/igt_runner --help | grep -A1 hook
   --hook HOOK_STR
                         Forward HOOK_STR to the --hook option of each test.
   --help-hook
                         Show detailed usage information for --hook.

So... today we already have "generic support" for running arbitrary
things before/after each test/subtest/dynamic-subtest.

	$ cat << EOF > testlist.txt
	igt at xe_exec_basic@once-basic
	EOF
	$ chmod +x lspci.sh
	$ sudo ./build/runner/igt_runner \
		--hook 'post-subtest:lspci -vv -d 8086:*:03xx' \
		--test-list testlist.txt build/tests/ results/
	$ head -n4 results/0/out.txt
	03:00.0 VGA compatible controller: Intel Corporation DG2 [Arc A580] (rev 08) (prog-if 00 [VGA controller])
         Subsystem: Intel Corporation Device 1425
         Control: I/O+ Mem+ BusMaster+ SpecCycle- MemWINV- VGASnoop- ParErr- Stepping- SERR- FastB2B- DisINTx-
         Status: Cap+ 66MHz- UDF- FastB2B- ParErr- DEVSEL=fast >TAbort- <TAbort- <MAbort- >SERR- <PERR- INTx-

So, instead of creating a separate thing called "igt_facts", what I'm
proposing is that we build this on top of the hooks and these "facts"
are simply built-in hooks that are commonly used. We may even keep them
as scripts rather than translate to C.

What I'd like is something liked this (with whatever name we decide):

	sudo ./build/runner/igt_runner --builtin-hooks lspci,drm-cards

which is a shorthand or equivalent to:

	sudo ./build/runner/igt_runner \
		--hook post-subtest:$PWD/scripts/built-in-hooks/lspci.sh \
		--hook post-subtest:$PWD/scripts/built-in-hooks/drm-cards.sh \
		--test-list testlist.txt build/tests/ results/

The cherry on top would be for CI to parse the cover letter and
enable those ondemand, so we only do that in CI when we want to:

/ci-built-in-hooks: lspci,drm-cards

Could also be:

/ci-hook: 'post-subtest:lspci -vv -d 8086:*:03xx'

(note that the current way we have is "Test-with:", but that is
problematic as it's then parsed as a git-trailer and added to the
patches by some tools)

Lucas De Marchi

>
>So I suggest we start with the hardcoded simple code to assess the potential value of these facts. If the facts turns out to be useful in helping us debug corner cases, then we evaluate how to extend it.
>
>Any other fact you want to add? Asking because I miss this code in our CI to debug two issues that are on my queue...
>
>>
>> This way it's easier to control what and when info is collected,
>> allowing developers (and CI) to pick and choose what gets executed.
>
>The original idea was to have static and dynamic facts. The dynamic idea suggested a convenient mechanism for each test to define facts as needed. Zbigniew did not appreciate the potential to bloating.
>
>>
>> +Gustavo
>>
>> Lucas De Marchi
>
>* - https://patchwork.freedesktop.org/patch/621849/?series=140566&rev=1
>